parse_strategy :sync for properties where no object exists yet

[JP Slavinsky]

Howdy all -

I like parse_strategy: :sync -- I use it in a number of places in my representers.  But I just ran into a case where it didn't do what I needed, so I thought I'd post my issue and solution here to see if there's a better way.  

I also see the recent conversation b/w Nick and Josh Cohen along with a related GH issue (https://github.com/apotonick/roar/issues/85). That discussion seems a little more related to collections than properties but maybe there are some parallels with this post.

Up to today, whenever I had used parse_strategy: :sync on a property, the model referred to by the property already existed -- since it existed, the sync strategy was great because it updated that existing model.

Today I had a case where I had a property that could start out as null / empty. Once the property is set, I want to use the :sync strategy so that future changes update the property's model instance.  But before it is created, I want roar / representable to make a new object (which it won't do if the parse_strategy is :sync).  When :sync is used and there is no existing object, I get errors about not being able to set values on a nil class.  

My solution is to set a "getter" on my property that initializes my property to a new instance of the model if it doesn't yet exist.  

class ModelA < ActiveRecord::Base
  belongs_to :model_b 

 

  # side note: even though this says belongs_to, ModelA in my system really "contains" ModelB
  # can't have the belongs_to on ModelB as ModelB can be contained by many other 
  # models, and I have reasons for not using a polymorphic belongs_to
end 

 

class ModelB < ActiveRecord::Base; end 

 

class ModelARepresenter < Roar::Decorator
  include Roar::Representer::JSON
    property :model_b,
                 class: ModelB,
                 decorator: ModelBRepresenter,
                 getter: lambda { |*| @model_b || ModelB.new },   
                 parse_strategy: :sync 
end

If I don't have the getter but do have sync, the code blows up if model_b doesn't exist yet.  If I don't have sync, future interactions create a new model_b instead of updating the existing.

The above approach seems to work, but...

1. Is there something else I should be doing instead?
2. As parse_strategy is being worked on, can Roar / Representable do the ModelB.new step for me if needed?

Thanks!

JP

[JP Slavinsky]

I realized this "getter" approach isn't that great -- when I do a GET of the JSON for a ModelA instance, when model_b is null the getter above gives me a blank ModelB JSON representation instead of null.  

Looking in the code I see there is actually a TODO that looks connected to this issue (line 36 of lib/representable/deserializer.rb).  Might the solution be as easy as changing that block of code to be:

35      if @binding.sync?

36        # TODO: this is also done when instance: { nil }

37        @object = @object.call || @binding.create_object(fragment) # call Bi...

38      else

39        @object = @binding.create_object(fragment)

40      end

?

[JP Slavinsky]

I went ahead and made a fork and a pull request to implement the suggestion I just posted: https://github.com/apotonick/representable/pull/70

[Nick Sutterer]

Hey JP,

thanks for you PR. Looks like a good idea. Anyway, I wanna go an implement different strategies that ship with representable. This is why I originally introduced the parse_strategy option.

Yours would go to something like :sync_or_create - what else do we need?

[JP Slavinsky]

I think :sync_or_create would be the biggie.  I see the "a stable API is a good API" value of keeping :sync, but if :sync_or_create is added do you have a feeling for when people would want to continue using plain old :sync?

If :sync stays as is, it'd be good to put in a note in the readme that warns people about what happens when there is nothing to sync with.

Re what else do we need, thinking back to the palm pilot days there was always the "device overrides computer" and "computer overrides device" settings when synchronizing data.  Wonder if this motivates letting parse_strategy be a lambda that can decide what to do on a case-by-case basis using the incoming data, or even both the incoming and existing data (e.g. the lambda could compare "updated_at" fields to keep the most recent data).