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

Changed ActiveFedora::File#persisted_size to simply return 0 if we're lo... #570

Merged
merged 1 commit into from
Nov 6, 2014
Merged

Changed ActiveFedora::File#persisted_size to simply return 0 if we're lo... #570

merged 1 commit into from
Nov 6, 2014

Conversation

afred
Copy link
Contributor

@afred afred commented Nov 6, 2014

...oking at a new record, rather than attempt a head request on the Ldp::Resource::BinarySource object.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 5c43458 on afred:change-af-file-persisted-size into 2db7e03 on projecthydra:fedora-4.

@afred
Copy link
Contributor Author

afred commented Nov 6, 2014

@awead @jcoyne This should avoid an Ldp::NotFound error when trying to do a head request on the Ldp::Resource::BinarySource object in ActiveFedora::File#content when the ActiveFedora::File object is new.

There is still a case where #has_content? will be false for new ActiveFedora::File objects that have something in #content that does not respond to a :size method.

One solution, would be to have #dirty_size do more thorough duck typing on #content looking for more than just :size but any other method that could provide an accurate value. I don't know what those methods would be though. Do we have a list of all the types of object that we can expect ActiveFedora::File#content to be?

@afred
Copy link
Contributor Author

afred commented Nov 6, 2014

👍 for #persisted_size to return nil if the record is not persisted. I'll push the change here momentarily.

Do you have any comment on the last bit of my comment above?

@awead
Copy link
Contributor

awead commented Nov 6, 2014

@afred content should always respond to .size. If it doesn't, it should raise an error. Are you seeing a case where is doesn't?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2751b25 on afred:change-af-file-persisted-size into 2db7e03 on projecthydra:fedora-4.

@afred
Copy link
Contributor Author

afred commented Nov 6, 2014

@awead no case that i can think of. Just wanted to be sure.

@afred
Copy link
Contributor Author

afred commented Nov 6, 2014

oh damn... i forgot to change the test expectation methinks. Stand by...

… looking at a new record, rather than attempt a head request on the Ldp::Resource::BinarySource object.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e468379 on afred:change-af-file-persisted-size into 2db7e03 on projecthydra:fedora-4.

awead added a commit that referenced this pull request Nov 6, 2014
Changed ActiveFedora::File#persisted_size to simply return 0 if we're lo...
@awead awead merged commit 503f0c0 into samvera:fedora-4 Nov 6, 2014
@afred afred deleted the change-af-file-persisted-size branch November 6, 2014 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants