Object resource #500

no-reply · 2014-10-22T18:08:13Z

This pushes AT to 0.3.0, in the process, it completes the removal of ObjectResource and makes it possible to pass an existing AT::Resource class to an RdfDatastream.

Removing the singleton_class.properties stuff was key to getting running on AT 0.3.x.

There is a test reverted to pending for discussion. AT's behavior has changed slightly with respect to how it handles property names conflicting with existing methods on Resource. This makes it impossible to override type in the relevant test. AT should handle this better (see: ActiveTriples/ActiveTriples#44), but we should also discuss to what degree this is a backwards compatibility issue.

ActiveFedora::Rdf::Persistence. The change allows any ActiveTriples::Resource descendant which includes the mixin to be used as the resource class for an ActiveFedora::RDFDatastream, rather than requiring an ObjectResource subclass.

no-reply · 2014-10-22T19:45:42Z

spec/integration/ntriples_datastream_spec.rb

+    #
+    # Behavior changed around how properties are handled for the type
+    # keyword in particular
+    xit "should write rdf with proper subjects" do


This is the relevant failing test. It fails with:

-<http://oregondigital.org/ns/99> <http://purl.org/dc/terms/type> "Frog" . +<http://oregondigital.org/ns/99> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> "Frog" .

Thoughts?

I would mark it as "pending" which, with the new version 3 behavior, RSpec will interpret its failure as a success. In other words, we know this is supposed to fail, so if it fails it's meeting our expectations. When it's finally patched, it will succeed and then RSpec will raise an issue.

I think we definitely need to communicate with adopters about this, I wouldn't be surprised if many people have 'type' properties with dc:type.

Is it acceptable (for the moment) that this fails so silently?

@no-reply raise an error if you're using a property called type with dc:type?
doh! just read the issue...

I think we definitely need to communicate with adopters about this, I wouldn't be surprised if many people have 'type' properties with dc:type.

Is it acceptable (for the moment) that this fails so silently?

We're an institution which would break if this goes in and we pull it.

It wouldn't be a problem if it informed us that we were doing something bad - instead this will start silently breaking our objects.

I have a patch on the AT side in the works: ActiveTriples/ActiveTriples#64

Once that is merged the question becomes how to handle ActiveFedora versioning. Does starting to throw errors on property definitions like :name and :type that once worked cause a problem?

awead · 2014-10-22T19:56:11Z

I'm 👍 fine with this, but I defer to @jcoyne. He may also want the commits squashed.

tpendragon · 2014-10-22T20:05:16Z

This seems like it works great, no notes from me. 👍

no-reply · 2014-10-22T20:08:30Z

I absolutely plan to squash prior to merge.

awead · 2014-10-22T20:09:16Z

👏 @no-reply

Tests pass without this line. It does not appear to be necessary.

This is as far as I’ve gotten trying to get AT 0.3 working with Fedora4. I cherry-picked the commits from PR #500 into the fedora-4 branch as a start. The secret is to apply the same changes in rdf_datastream.rb to fedora_attributes.rb.

no-reply · 2014-10-24T17:18:56Z

This is now squashed, pins to newly minted AT 0.4.0, and throws errors if you define unwelcome properties.

Several test cases are changed to reflect this.

From my perspective, this seems ready to go. I defer to others on how to handle communicating this change to AF7 RDF users and how to handle versioning on this end.

Introduces a better interface for using existing Resources to back a datastream. Removes ObjectResource and related singleton_class calls. Pins to ActiveTriples 0.4.0, which restricts property names for Resources & RdfDatastreams to those not already used by a method on the class (prevents overriding `class`, `graph`, `repository`, `name`, etc...).

jcoyne · 2014-10-25T13:39:59Z

While this is great work. It introduces some breaking changes. So I wonder if we need to make this change for ActiveFedora 7. Perhaps we should just leave this PR open indefinitely unless someone needs it. Does anyone have that situation? This will be helpful for crafting a patch for AF 8 though.

jcoyne · 2014-10-25T13:43:29Z

Another option is to merge this and then release ActiveFedora 8 which would target Fedora Commons 3. Then we make the fedora-4 branch be ActiveFedora 9. This would be relatively painless.

This is as far as I’ve gotten trying to get the latest AT working with Fedora4. I cherry-picked from PR #500 as a start. The secret is to apply the same changes in rdf_datastream.rb to fedora_attributes.rb.

no-reply · 2014-10-26T22:29:06Z

@jcoyne I'm with you about the need to bump versions. (Though there may be something to be said for what I take to be @terrellt's attitude that the 'breaking changes' here are actually just AF/AT catching what was always bad behavior. Maybe the property name restrictions are a bug fix?)

The downside of declining to push this to Fedora 3 users is that they are then stuck on the ActiveTriples 0.2.x branch. I don't anticipate being able to port bug fixes, etc... back to a pre-1.0 release.

I think @dchandekstark may also be wanting the ObjectResource changes originating in 4f9a7a6.

Maybe this reveals a need for further thought about the versioning strategy for Fedora3/Fedora4 on AF?

dchandekstark · 2014-10-27T14:18:01Z

@jcoyne @no-reply

Yes, I would like to have the update available for fc3.
If that means pushing the fc4-compatible version of AF, and as @jcoyne says, it's not a big deal, let's do that, and ...
Yes, I think we need to kick around the versioning strategy of AF, perhaps on hydra-tech and/or conference call.

dchandekstark · 2014-10-27T14:23:30Z

@jcoyne @no-reply Considering further, if we reintroduced ObjectResource (as a subclass of AT::Resource adding no behavior of its own) as the default class with a deprecation warning, would that solve the backward compat issue? Seems like it could ...

jcoyne · 2014-10-29T01:17:01Z

@dchandekstark I'm speaking mostly about the fact that ActiveTriples will no longer allow you to define properties that are already method names (e.g. type)

no-reply · 2014-10-31T19:04:18Z

Just bumping this. Do we have an idea what the way forward is? Is this AF8?

awead · 2014-10-31T19:13:03Z

@no-reply I think it depends on the outcome of the email conversation on hydra-tech as to whether AF with Fedora3 should be it's own gem, or rather if AF with Fedora4 should be its own gem.

Object resource. Merged into 8.0.0.pre1 as publicized on hydra-tech.

no-reply mentioned this pull request Oct 22, 2014

Factors out behavior from ActiveFedora::Rdf::ObjectResource to mixin #473

Closed

no-reply reviewed Oct 22, 2014
View reviewed changes

no-reply referenced this pull request in ActiveTriples/ActiveTriples Oct 23, 2014

Removing code that was not called

69e4bdd

Tests pass without this line. It does not appear to be necessary.

no-reply force-pushed the object-resource branch from 81b72ef to 2e981f7 Compare October 24, 2014 16:58

no-reply force-pushed the object-resource branch from 2e981f7 to 960ff31 Compare October 24, 2014 17:37

awead mentioned this pull request Oct 24, 2014

Integration with ActiveTriples 0.4.0 #491

Closed

2 tasks

jcoyne added this to the 8.0.0 milestone Nov 12, 2014

dchandekstark self-assigned this Nov 12, 2014

dchandekstark added a commit that referenced this pull request Nov 17, 2014

Merge pull request #500 from no-reply/object-resource

a3f8293

Object resource. Merged into 8.0.0.pre1 as publicized on hydra-tech.

dchandekstark merged commit a3f8293 into samvera:master Nov 17, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Object resource #500

Object resource #500

no-reply commented Oct 22, 2014

no-reply Oct 22, 2014

awead Oct 22, 2014

no-reply Oct 22, 2014

awead Oct 22, 2014

tpendragon Oct 22, 2014

no-reply Oct 22, 2014

awead commented Oct 22, 2014

tpendragon commented Oct 22, 2014

no-reply commented Oct 22, 2014

awead commented Oct 22, 2014

no-reply commented Oct 24, 2014

jcoyne commented Oct 25, 2014

jcoyne commented Oct 25, 2014

no-reply commented Oct 26, 2014

dchandekstark commented Oct 27, 2014

dchandekstark commented Oct 27, 2014

jcoyne commented Oct 29, 2014

no-reply commented Oct 31, 2014

awead commented Oct 31, 2014

Object resource #500

Object resource #500

Conversation

no-reply commented Oct 22, 2014

no-reply Oct 22, 2014

Choose a reason for hiding this comment

awead Oct 22, 2014

Choose a reason for hiding this comment

no-reply Oct 22, 2014

Choose a reason for hiding this comment

awead Oct 22, 2014

Choose a reason for hiding this comment

tpendragon Oct 22, 2014

Choose a reason for hiding this comment

no-reply Oct 22, 2014

Choose a reason for hiding this comment

awead commented Oct 22, 2014

tpendragon commented Oct 22, 2014

no-reply commented Oct 22, 2014

awead commented Oct 22, 2014

no-reply commented Oct 24, 2014

jcoyne commented Oct 25, 2014

jcoyne commented Oct 25, 2014

no-reply commented Oct 26, 2014

dchandekstark commented Oct 27, 2014

dchandekstark commented Oct 27, 2014

jcoyne commented Oct 29, 2014

no-reply commented Oct 31, 2014

awead commented Oct 31, 2014