Attributes/Persistence not friendly to noSQL embedding

[jsonapi-suite]

Attributes/Persistence not friendly to noSQL embedding

[richmolj]

@Startouf I ended up needing something similar for a different use case. The default case is still

def create(attrs)
end

But you can now optionally do

def create(attrs, parent = nil)
end

This applies to update and destroy as well.

You can use these lower-level methods as well if needed:

def associate(parent, child, association_name, association_type)
end

def disassociate(parent, child, association_name, association_type)
end

[Startouf]

In the long term, I believe Jsonapi-suite's persistence logic needs an overhaul to be able to suppor specific persistence behaviors and not just ActiveRecord's. I was thinking of a sideposting adapter like the sideloading adapter. I also don't like that x variable in the code of JsonapiCompliable/Resource as I never understand what it represents and have to keep digging (if I understand well it somewhat stores the relation configuration). I was thinking maybe a small DSL could let us handle those specific cases in a better manner.

register_persistence_behavior_for :embeds_many do 
  it_saves :with_parent
  it_needs_parent true
end

# For ActiveRecord `belongs_to` this would look like
register_persistence_behavior_for :belongs_to do 
  it_saves :after_parent 
  it_include_attributes do |parent, child_attributes, association_config|
    # include the foreign key like in #update_foreign_key
    child_attributes[association_config.foreign_key] = parent_object.send(association_config.primary_key)
  end
  it_needs_parent false
end

And then the `it_xxx` could map to the existing code of JsonapiCompliable/Persistence logic

In the short term, passing the parent_object to the nested resource #create and #update would be sufficient. However I see no way to do this "in a clean manner" and had to bubble my monkeypatches quite the way down, and the changes introduced are quire breaking. Here's a small summary of the methods that needed some change (If I have time I'll fork & branch so we can get a better diff)

class JsonapiCompliable::Util::Persistence
  def process_has_many(relationships)
    ...
        x[:object] = x[:sideload].resource
          .persist_with_relationships(x[:meta], x[:attributes], x[:relationships],
            parent_object: x[:parent_object])
  end

  def persist_object(method, attributes, parent_object: nil)
    if parent_object.present?
      return persist_embedded_object(method, attributes, parent_object)
    end
    ....

  def persist_embedded_object(method, attributes, parent_object)
    case method
    when :destroy
      @resource.destroy(attributes[:id], parent_object)
    when :disassociate, nil
      @resource.update(attributes, parent_object)
    else
      @resource.send(method, attributes, parent_object)
    end
  end

  # Add one more argument to initialize method to store parent_object
  def initialize(resource, meta, attributes, relationships, parent_object: {})
    @resource      = resource
    @meta          = meta
    @attributes    = attributes
    @relationships = relationships
    @parent_object = parent_object
  end

 def run
    ...
    children = process_has_many(@relationships) do |x|
      if x[:sideload].type.in?([:embeds_many, :embeds_one])
        x[:parent_object] = persisted
      else
        update_foreign_key(persisted, x[:attributes], x)
      end
    end
end

class JsonapiCompliable::Resource
  # - Add @parent_object argument when calling persist_object
  def persist_with_relationships(meta, attributes, relationships,
    parent_object: nil)
    persistence = JsonapiCompliable::Util::Persistence \
      .new(self, meta, attributes, relationships, parent_object: parent_object)
    persistence.run
  end
end

As a final note, I believe what is missing to clean the code (and get rid of that x variable) would be a specific resource_context class that would hold association metadata (pretty much like what ActiveRecord or Mongoid already have). This context would be build for each association and include relevant information (such as the parent_object, the foreign_key and the primary_key)

Those are just some suggestions, anyway I'm looking for hints in case there's a better way to integrate embedded sideposting with jsonapi-suite

[Startouf]

Here is a link to a compare view with my changes to make Mongoid work (I hope I didn't miss any)

https://github.com/jsonapi-suite/jsonapi_compliable/compare/master...Startouf:feature/support-mongoid-embedded?expand=1

[richmolj]

@Startouf thanks for digging in to this.

I agree the x variable needs improving. The premise is less to store information about the resource/model, and more to store information about the request payload in a more accessible manner (the resource information is a bit more "while we're here..."). We can probably improve the Deserializer class to make this a bit more obvious and helpful.

In the meantime, here's what I believe is a simpler fix for your use case.What if we took parents from like 33, and pass it to #persist_object. We can then introspect the arity of create, update etc resource methods here passing the parents when appropriate.

This still has the downside of your parent being saved first, when you want to save things all at once. This probably means refactoring the persistence logic to have some save boolean flag.

Talking through it, though, I'm not sure this is the best approach. If you have a compound document, I would think these are more akin to attributes than associations. After all, it's not possible to fetch the "inner" relationship contents without the "outer" embedded document, right? Part of the reason this becomes difficult is this seems like it's really one object, but we're trying to treat it as several different objects.

In other words, we could put all this logic in, but you'd still have a resource that cannot save itself without its parent. This makes it different than all other resources. If something cannot save itself without a parent, it's coupled to the parent and probably a better fit for attributes. That said, I'm not super familiar with MongoDB and could definitely be off base here.

In summary, even if we could easily get this API:

def create(attributes, parents)
  # ... create ...
end

I'm not sure we should.

A better option might be a new type of relationship - something like has_embedded - instead of using the existing relationships. So different concepts are handled differently.

Thoughts?