samvera / hydra-works

A ruby gem implementation of the PCDM Works domain model based on the Samvera software stack
Other
24 stars 14 forks source link

Relationship between ContainedFiles and AddFile Service is confusing and prevents original_file from being versionable #127

Closed flyingzumwalt closed 5 years ago

flyingzumwalt commented 9 years ago

The way Works::GenericFile::ContainedFiles and the Works::AddFileToGenericFile Service are currently implemented is really confusing and makes it unclear how to do things like add thumbnails to a GenericFile. It also prevents us from having a clear way to say "The original_file should be versioned, but the others should not be"

TODO

Read up on contains and directly_contains

Read up on Services

... Fix things as appropriate....

Example: maybe Works::GenericFile::ContainedFiles should look like this (probably requires changes in ActiveFedora to support this)


module Hydra::Works::GenericFile::ContainedFiles
  extend ActiveSupport::Concern

  # allows you to find the specific file from with in the files contained via PCDM::ObjectBehavior#files
  directly_contains_one :original_file, has_member_relation: RDFVocabularies::PCDMTerms.hasFile,
    class_name: "CurationConcerns::VersionableFile", type: ::RDF::URI("http://pcdm.org/OriginalFile")

  directly_contains_one :thumbnail, has_member_relation: RDFVocabularies::PCDMTerms.hasFile,
    class_name: "Hydra::PCDM::File", type: ::RDF::URI("http://pcdm.org/Thumbnail")

  directly_contains_one :extracted_text, has_member_relation: RDFVocabularies::PCDMTerms.hasFile,
    class_name: "Hydra::PCDM::File", type: ::RDF::URI("http://pcdm.org/ExtractedText")
end

Then:

Make original_file versioned. Possibly use pattern from Sufia master, where it sets class_name: “VersionableFile" in the call to contains

jcoyne commented 9 years ago

I think you're on the right track, but I'm not sure that original_file must be versionable in PCDM. I think the default should be Hydra::PCDM::File, but there certainly should be a way to override it.

flyingzumwalt commented 9 years ago

Yeah. We're going to make it versionable in CurationConcerns::GenericFile (previously Sufia::GenericFile), not down in the lower gems. @jcoyne can you offer hints to the answers to the questions about directly_contains, or do you have ideas about the answer to my question about whether Classes should provide getters & setters for things like .thumbnail instead of relying solely on services?

jcoyne commented 9 years ago

I don't think there's any business logic in a relationship getter/setter, so my preference is to add that directly to the model.

flyingzumwalt commented 9 years ago

I agree. :+1:

flyingzumwalt commented 9 years ago

Updated the code sample in the main description to reflect the need for a directly_contains_one method instead of making directly_contains accept an argument of singular: true

jrgriffiniii commented 5 years ago

https://github.com/samvera/hydra-works/commit/10a4c1979d42c62d4536b093537e34e45093b3e0 seems to have addressed this (at least partly). Further feature requests should be proposed for active_fedora.