active_fedora icon indicating copy to clipboard operation
active_fedora copied to clipboard

ActiveFedora::Versionable should allow you to set/read `.versionable` on instances rather than class

Open flyingzumwalt opened this issue 10 years ago • 17 comments

The implementation of ActiveFedora::Versionable is a bit odd. I think this is because the way versioning is done in Fedora has changed and the API & API docs have not changed to reflect it.

How versioning has changed in Fedora 4

In Fedora 3, when you set a Datastream as versionable, it was internally represented as VersionSet and Fedora would automatically create new entries in that VersionSet whenever you updated the Datastream. This way, all you had to do was set versionable to true or false on the Datastream and the rest happened automatically.

In Fedora 4, versions of a resource are basically snapshots of the resource. The resource itself is always represented as its 'latest' version. If it is versionable, it can have any number of versions in its VersionGraph. Big difference To create a new entry in the version graph, you update the resource in Fedora and then tell Fedora to create a new entry in the VersionGraph based on the resource's current state. That's quite different, especially because it relies on the consumer (rather than Fedora) to decide when a new version should be added to the VersionGraph.

You can set a #versionable flag on resources in Fedora. I'm not sure what that does.

The way it is now

It appears that the only way to set the #versionable flag on a record is to define a Versionable class, but it's not clear what that does.

class ActiveFedora::File
    include ActiveFedora::Versionable   # ActiveFedora::File already includes the Versionable module, but does not mark Files as versionable
end
class VersionableFile < ActiveFedora::File
     has_many_versions   # this class method from ActiveFedora::Versionable sets the `versionable` _class attribute_ which, presumably, tells the class to mark instances as versionable
end

It's not clear what changes when I load a File from Fedora using VersionableFile instead of ActiveFedora::File, but this is what the code is encouraging. Presumably it ensures that instances of VersionableFile will have the versionable flag set in Fedora and will return true to #versionable?

I think this code, like Fedora, relies on the consumer to decide when to create new entries in the VersionGraph. Here's an Example of code that creates version histories: (from https://github.com/projecthydra-labs/hydra-works/blob/2e69e0fce4fe4e78132f6ef922f95226022b6652/lib/hydra/works/services/generic_file/add_file.rb#L34-L41)

      if current_file.versionable?
        if current_file.new_record?
          object.save  # this persists current_file and its membership in object.files container
        else
          current_file.save # if we updated an existing file's content we need to save the file explicitly
        end
        object.reload     # this forces the object to see the updated file
        current_file.create_version # Create version _after_ saving because ActiveFedora::Versionable#create_version tells Fedora to create a version that is a snapshot of the object's current state within Fedora
      else
        object.save
      end

It's not clear if I even have to set .versionable to true in order to be able to create and use a VersionGraph.

So it's all a bit confusing.

How it should work

Definitely do this

  • Setting .versionable = true on an object marks it as versionable in Fedora.
  • The response to .versionable? is based on the #versionable flag on the resource in Fedora
  • Calling #has_many_versions at Class level ensures that versionable is set to true on all instances of that class (but you don't have to use a class with has_many_versions set in order to set objects as versionable. You can always set it as a one-off on specific objects. That way, I can do things like this
file1 = ActiveFedora::File.create                 # file1 is not versionable
file2 = ActiveFedora::File.create
file2.versionable = true                          # now file2 is versionable
file2.save                                         # now the fedora resource for file2 is flagged as versionable
reloaded_file2 = ActiveFedora::File.find(file2.uri) # reloaded_file2 is also versionable because the resource in Fedora is flagged as versionable

Option 1: objects marked as versionable automattically save versions on update

  • If .versionable? is true on an object, it automatically creates new entries in the VersionGraph whenever you update the object

Option 2: version_on_update is something you optionally set, separately from versionable

Maybe there are cases where you want versionable to be true on the resource in fedora but don't want it to automatically create new versions when you update? This is where we get into a fuzzy zone around the versionable flag in Fedora and what it means. Can you add versions to a resource without marking it with the versionable flag? If so, can you retrieve and/or restore versions?

If we want to support that case, you could do something like this:

  • Setting .versionable on objects just marks them as versionable but does not trigger version_on_update
  • Setting something like .version_on_update on instances causes them to always add an entry in the version graph when saving changes
  • Setting #version_on_update at Class level sets .version_on_update to true on any instances

flyingzumwalt avatar Jun 24 '15 17:06 flyingzumwalt

:arrow_up: @awead

mjgiarlo avatar Jun 24 '15 17:06 mjgiarlo

:clap: @flyingzumwalt

awead avatar Jun 24 '15 17:06 awead

This is a really important issue. Any update? @flyingzumwalt @awead @mjgiarlo @jcoyne

dchandekstark avatar Dec 08 '15 15:12 dchandekstark

@dchandekstark I don't think anyone is working on this. I don't understand why AF would need a versionable accessor.

jcoyne avatar Dec 08 '15 15:12 jcoyne

@dchandekstark do you have a preference for how it should be changed? Currently, you have to call .create_version on your resources, but you can set .versionable to true so your application would know if it needs to or not. My preference would be for option 1, which would kind of restore the old Fedora 3 behavior, but this has implications and would require ActiveFedora 10 because it's a breaking change. Unless we have another flag, like autoversion which automatically creates new version after each update.

awead avatar Dec 08 '15 15:12 awead

@jcoyne users need the ability to indicate if their resources should be versioned so that they can create versions for themselves. Yes, it's a bit weird, but I do see the use for it.

awead avatar Dec 08 '15 15:12 awead

@awead can't they create versions for themselves by calling .create_version?

jcoyne avatar Dec 08 '15 15:12 jcoyne

@jcoyne @awead What's wrong with this:

class Book < ActiveFedora::Base
  contains "content", versionable: true
end

such that create_version is called after save?

dchandekstark avatar Dec 08 '15 15:12 dchandekstark

@dchandekstark looks good to me. If this was implemented, we need to tell folks that if they use this they'd need to remove any calls to .create_version otherwise they'd have twice as many versions for everything.

awead avatar Dec 08 '15 16:12 awead

Note: sometimes you want to do more than just call create_version on a file. Also, just because a file is versionable does not mean you always want to create a new version on every save. Those decisions should be up to the implementer. I thought that was why AF just let you assert the :versionable flag but does not create versions for you -- intercepting calls to .save and creating a version when you want it is easier than preventing AF from automatically creating the version for you.

Also be careful not to break this code: https://github.com/projecthydra-labs/hydra-works/blob/8715f129c06a3daeac73490519f78e00144c80b0/lib/hydra/works/services/add_file_to_file_set.rb#L17-L19

On Tue, Dec 8, 2015 at 4:02 PM, Adam Wead [email protected] wrote:

@dchandekstark https://github.com/dchandekstark looks good to me. If this was implemented, we need to tell folks that if they use this they'd need to remove any calls to .create_version otherwise they'd have twice as many versions for everything.

— Reply to this email directly or view it on GitHub https://github.com/projecthydra/active_fedora/issues/821#issuecomment-162928183 .

flyingzumwalt avatar Dec 08 '15 16:12 flyingzumwalt

Which kind of brings us back to square one. @dchandekstark is having to call .create_version on your resources prior to saving not a good solution for your use case?

awead avatar Dec 08 '15 16:12 awead

@awead @flyingzumwalt @jcoyne Well, I feel that the issue is maintaining a certain consistency in the API, given that FCR4 does not carry forward the versionable configuration for a datastream in FCR3. As you recall, with AF < 9 and FCR3 we did this:

has_file_datastream "content", versionable: true

If I can't do a similar thing in AF9, then I have to hand-craft a solution (e.g., because I don't want every file to be "auto-versioned"), which I don't think is good for the community. If you don't want to use the config option, fine, you can always roll your own, but this seems basic functionality to me.

dchandekstark avatar Dec 08 '15 16:12 dchandekstark

@awead I would be OK with adding an autoversion flag if that sits better.

dchandekstark avatar Dec 08 '15 16:12 dchandekstark

@dchandekstark you could create a module:

def save
  self.create_version if versionable?
  super
end

include that on all your files and then set self.versionable as appropriate. You'll also need to pass a version label, which is just a string. Sufia uses "version1", "version2", etc.

awead avatar Dec 08 '15 16:12 awead

@awead I assume I could use a before_save callback?

dchandekstark avatar Dec 08 '15 16:12 dchandekstark

Since there is a divergence of opinion about versionable I think I should sketch out an alternate option for autoversioning.

dchandekstark avatar Dec 08 '15 17:12 dchandekstark

@dchandekstark :+1:

awead avatar Dec 08 '15 17:12 awead