Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doesn't support source & output files that are directly contained by the Fedora object #60

Closed
flyingzumwalt opened this issue Jun 5, 2015 · 13 comments
Assignees
Labels

Comments

@flyingzumwalt
Copy link
Member

This is shown in PR #59

@awead
Copy link
Contributor

awead commented Jun 5, 2015

@flyingzumwalt seems like the solution should be that hydra-derivatives accepts parameters for these. Is the gem currently making too many assumptions about the model?

@flyingzumwalt
Copy link
Member Author

@awead one solution would be to use .send instead of .attached_files, but I bet there is a reason that it didn't already do that?

Example

def source_file
    object.send(source_name.to_sym)
end

instead of

def source_file
    object.attached_files[source_name.to_s]
end

@flyingzumwalt
Copy link
Member Author

or maybe rely on a retrieve_attached_file method

def retrieve_attached_file(object, attachment_ref)
    object.send(attachment_ref.to_sym)
end

def source_file
    retrieve_attached_file(object, source_name)
end

def output_file(path)
        # first, check for a defined file
        output_file = if retrieve_attached_file(object, path)
          object.attached_files[path]
        else
          ActiveFedora::File.new("#{object.uri}/#{path}").tap do |file|
            object.attach_file(file, path)
          end
        end
      end

@flyingzumwalt
Copy link
Member Author

I would love it if hydra-derivatives didn't assume that you were using fedora at all. Let the object know how persistence happens, and let the calls to transform_file tell hydra-derivatives which methods to call on that object in order to retrieve the source and return the output.

@flyingzumwalt
Copy link
Member Author

Then hydra-derivatives would become the first hydra gem that's truly useful outside of the hydra/fedora sphere.

@awead
Copy link
Contributor

awead commented Jun 5, 2015

👍 to that.

@awead
Copy link
Contributor

awead commented Jun 5, 2015

@flyingzumwalt specify a default retrieve_attached_file method that is the current process using attached_files, but the implementor overrides that? Or make it explicit:

class Hydra::Derivatives::Processor::Base

  def retrieve_attached_file
    raise NotImplementedError, "You must specify a method for retrieving the file's content"
  end

end

@flyingzumwalt
Copy link
Member Author

That doesn't work. That would mean all of your Processors have to choose
which attached_file strategy to assume. I should be able to use the Image
processor, etc. regardless of how files are attached to my object.

@awead
Copy link
Contributor

awead commented Jun 5, 2015

fair enough!

@cjcolvar
Copy link
Member

cjcolvar commented Jun 5, 2015

👍 to this work, hydra-derivatives assumptions about the model keep us from adopting it.

@flyingzumwalt
Copy link
Member Author

Solving this by refactoring Processors to use a RetrieveSourceFile service for retrieving source file and a PersistOutputFile service for persisting output file. These services can be configured in the initializer and/or in your calls to transform_file.

@kevinreiss
Copy link
Contributor

See f48c2e5, 946f581.

@tampakis
Copy link
Member

tampakis commented Aug 3, 2015

Fixed in #61. Test coverage in #66.

@tampakis tampakis closed this as completed Aug 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants