Raven24 / diaspora-federation

MIT License
18 stars 5 forks source link

Broken Fabricate methods #5

Open JannikStreek opened 10 years ago

JannikStreek commented 10 years ago

While I was testing the new poll entities I realized that most of the fabricate methods are broken (e.g. build and the normal fabricate). The reason is basically that fabricate first instantiates an object and after sets all the parameters. However, the implementation of diaspora-federation requires to set the properties during the instantiation. This leads to the broken methods. The only exception is the attributes_for method, which is used in the project, because it does not really instantiate the object.

I think it's not too hard to fix this (the method def initialize(args) in the entity class is the problem), but I'm not sure if this was an intended design decision or something? At least it is confusing for starters in this project.

Raven24 commented 10 years ago

Ok, I'm not entirely sure what you mean exactly. Could you add some minimal code example to make yourself clear, please?

JannikStreek commented 10 years ago

Sure. Lets play this through:

You call Fabricate.build(:message). This fails with initialize': wrong number of arguments (0 for 1) (ArgumentError). Why does it fail?

In the Fabricate gem code, this happens:

def build_instance
    self._instance = _klass.new
    set_attributes
  end

This is calling the entity class in the federation project:

def initialize(args)
      raise ArgumentError, "expected a Hash" unless args.is_a?(Hash)
      missing_props = self.class.missing_props(args)
      unless missing_props.empty?
        raise ArgumentError, "missing required properties: #{missing_props.join(', ')}"
      end

      self.class.default_props.merge(args).each do |k,v|
        instance_variable_set("@#{k}", v) if setable?(k, v)
      end
      freeze
    end

But this method requires an argument, namely the attributes for the entity. However, as the code above shows, the attributes are set after the instantiation. This leads to problem, that you can't really use many of the Fabricate gem methods. The only method which works is the attributes_for method, because it does not need to build the instance. Hope that this was easier to understand?

Raven24 commented 10 years ago

oh, that's what you mean :) yes, actually, that behaviour is by design. the idea behind it is that in the context of federation (which this gem is all about) an entity should be immutable once it has been instantiated. that's the reason you can only set properties during initialization and afterwards the instance is frozen. this makes the entities (as used in the context of this gem) plain data containers with some extra logic for de-/serialization, that are guaranteed not to change, you know, since that data can be signed and what not.

But I'm open to suggestions, if you know a different way to solve this. ;)