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

Fix ETag handling for Binaries and Binary descriptions #1024

Closed
wants to merge 12 commits into from
Closed

Fix ETag handling for Binaries and Binary descriptions #1024

wants to merge 12 commits into from

Conversation

acoburn
Copy link
Contributor

@acoburn acoburn commented Apr 18, 2016

Resolves: https://jira.duraspace.org/browse/FCREPO-1983
Resolves: https://jira.duraspace.org/browse/FCREPO-1754

This is an alternate resolution of this issue. See #1022 for a different approach.

The advantage of this PR (from my perspective) is that ETag and last-mod values on the fedora:Binary and associated description are decoupled.

Note: this PR also addresses the Strong/Weak ETag issue: Binaries produce strong ETags, everything else produces a weak ETag. So it also supersedes #991

See: http://irclogs.fcrepo.org/2016-04-18.html for further discussion of this.

Based on discussion, this approach is to be pursued instead of #1022

@awoods
Copy link

awoods commented Apr 18, 2016

To be clear, this current PR bases the:

  • etag and last-mod for NonRdfSources on the Description, and
  • etag and last-mod for Descriptions on the NonRdfSources.

A brief rationale would be helpful here.

@acoburn
Copy link
Contributor Author

acoburn commented Apr 18, 2016

@awoods I agree a rationale would be helpful here. Would you like that as an inline comment or written here in the github comments? In brief, though, it follows from the principle of the binary being the subject of the triples in the description.

@acoburn
Copy link
Contributor Author

acoburn commented Apr 19, 2016

@awoods as a side note, this is exactly why we need a specification. It is impossible to fix incorrect behavior if there is no clear document describing what correct behavior is.

@awoods
Copy link

awoods commented Apr 19, 2016

Maybe an inline comment would help folks who look at the code in the future understand what is going on.

FCREPO-1989: Minor cleanup of compiler warnings
@ajs6f
Copy link
Contributor

ajs6f commented Apr 19, 2016

Six kinds of amen.

On Monday, April 18, 2016, Aaron Coburn notifications@github.com wrote:

@awoods https://github.com/awoods as a side note, this is exactly why
we need a specification. It is impossible to fix incorrect behavior if
there is no clear document describing what correct behavior is.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#1024 (comment)

@ajs6f
Copy link
Contributor

ajs6f commented Apr 19, 2016

To be clear, my comment was in support of @acoburn's remark about specification.

@sprater
Copy link

sprater commented Apr 21, 2016

@acoburn: I pulled your fix into my fork, compiled, and tested in the web UI again the case I documented in FCREPO-1983. That problem appears to be fixed.

@acoburn
Copy link
Contributor Author

acoburn commented Apr 21, 2016

@sprater thanks for checking! I would like to add some additional integration tests before this is actually merged. If that happens next week (before the 4.5.1 release), I'll close this PR and re-target it at the DEV branch; otherwise, it will stay where it is. In any case, this ticket is certainly moving forward.

@sprater
Copy link

sprater commented Apr 21, 2016

@acoburn sounds good. I'll wait for the merge when it occurs, then test again, just to make sure.

@acoburn
Copy link
Contributor Author

acoburn commented Apr 25, 2016

Superseded by #1028

@acoburn acoburn closed this Apr 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants