code4lib / ruby-oai

a Ruby library for building OAI-PMH clients and servers
MIT License
62 stars 43 forks source link

Question about "sets" for ActiveRecordWrapper #9

Closed seandmccarthy closed 12 years ago

seandmccarthy commented 12 years ago

ActiveRecordWrapper is a wrapper for model.

The documentation states that your ActiveRecord model should implement a class method "sets":

# A model class is expected to provide a method Model.sets that
# returns all the sets the model supports.  See the 
# activerecord_provider tests for an example.

However, the find method basically composes a query condition that, if the OAI request contains a "set" parameter, looks like:

AND set = '<set_name>'

So it is assuming your ActiveRecord model has an attribute (and hence a database column) "set".

Is this what we want? This amounts to per-record granularity- you need to have the "set" value set per record that you want returned. Conceptually I think of a model as being a set, and hence all it's record belong.

I think we should remove this. I'm happy to supply a pull request if no one has an argument in favour of keeping it.

tjdett commented 12 years ago

@seandmccarthy I'm looking at this issue right now, as I have I case where records could conceptually belong to more than one set.

My intention is provide an additional method for defining sets that uses ActiveRecord associations. Something along the lines of:

class Record < ActiveRecord::Base
  has_many :set_memberships, :class_name => 'RecordSetMembership'
  has_many :sets, :through => :set_memberships

  def self.sets
    Set.all
  end

end

class RecordSetMembership < ActiveRecord::Base
  belongs_to :record
  belongs_to :set
end

class Set < ActiveRecord::Base
  has_many :record_memberships, :class_name => 'RecordSetMembership'
  has_many :records, :through => :record_memberships
end

It shouldn't be too hard to check if the results returned by model.sets are ActiveRecord models, use reflection to find the association back to model (eg. set.records) and then use it to perform find.

tjdett commented 12 years ago

Pull-request #18 splits out this behaviour into two variants:

In model-based sets, the sets are ActiveRecord models which have an association back to records. They preserve the example DCSet behaviour.

In attribute-based sets, the set attribute to determine set membership (and the class method simply needs to provide a list of those sets somehow).