samvera / hyrax

Hyrax is a Ruby on Rails Engine built by the Samvera community. Hyrax provides a foundation for creating many different digital repository applications.
http://hyrax.samvera.org/
Apache License 2.0
185 stars 124 forks source link

Objects with ids less than 8 characters long create bad data within fedora when added to collections #2546

Open cjcolvar opened 6 years ago

cjcolvar commented 6 years ago

Descriptive summary

Objects that have ids less than 8 characters long (or the length of the noid configuration if it is smaller) cause bad data to be stored in fedora when added to a collection. This same problem could potentially manifest in other ways. This came up when objects were created with specific ids in the spec tests.

Expected behavior

Objects added to collections will get a ldp:contains member_of_collections IndirectContainer that contains a ActiveFedora::Aggregation::Proxy for the collection object.

Actual behavior

Objects added to collections will cause amember_of_collections IndirectContainer to be created at the root (/rest/test/me/mb/er/_o/member_of_collections) instead of hanging off of the object. The object does not get a ldp:contains for that node. The IndirectContainer correctly assigns the ldp:membershipResource. If another object with a less than 8 character id is added to a collection then the membershipResource is overwritten but the ldp:contains is appended to making the new object be a member of two collections instead of one as expected.

Root Problem

The cause of the incorrect id for the indirect container is the translate_uri_to_id method that Hyrax uses: https://github.com/samvera/hyrax/blob/master/lib/hyrax/configuration.rb#L442-L447

The ActiveFedora code that calls this is https://github.com/samvera/active_fedora/blob/master/lib/active_fedora/associations/indirectly_contains_association.rb#L51

Example tests

The first test passes but the second fails.

  describe '#translate_uri_to_id' do
    let(:short_id) { 'abc123'}
    let(:short_uri) { described_class.new.translate_id_to_uri.call(short_id) }
    let(:contained_id) { short_id + '/member_of_collections' }
    let(:contained_uri) { described_class.new.translate_id_to_uri.call(contained_id) }

    subject { described_class.new.translate_uri_to_id }

    it 'handles objects with short ids' do
      expect(subject.call(short_uri)).to eq short_id
    end
    it 'handles directly contained objects whose parents have short ids' do
      expect(subject.call(contained_uri)).to eq contained_id
    end
  end
      expected: "abc123/member_of_collections"
            got: "member_of_collections"

Related work

Link to related tickets or prior related work here. https://github.com/samvera/hyrax/issues/2521

jlhardes commented 3 years ago

This appears to be something that doesn't happen within Hyrax when it is functioning as an application and creating objects but it can occur when supplying id's for spec tests. Backwards compatibility seems to be a concern for any fix. Providing warnings in the spec test route and/or stopping object creation in the spec test route seems reasonable to resolve this issue. Developer testing is needed to review this issue and verify any fixes.