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

Filtering out server-managed triples about hash URI fragments #936

Merged
merged 1 commit into from Nov 3, 2015

Conversation

escowles
Copy link
Contributor

@escowles escowles commented Nov 2, 2015

@ajs6f
Copy link
Contributor

ajs6f commented Nov 2, 2015

Can you explain a little about why this is the fix, @escowles ? (Actually, maybe an in-line comment or two would be good...?)

@escowles
Copy link
Contributor Author

escowles commented Nov 2, 2015

@ajs6f The predicates for testing whether triples are server-managed, and the logic for including various kinds of triples based on Prefer headers is already in ContentExposingResource, so it seemed like the logical place to implement the decision that server-managed triples should never be included for hash URI resources. I could definitely add a comment that we never want server-managed triples for hash URI resources, even if we want them for containers.

The other approach I considered was doing the filtering in the triple generating machinery. But I'm wary of over-complicating that already somewhat baroque code, and the logic of the decision seemed to fit better in the HTTP layer.

@ajs6f
Copy link
Contributor

ajs6f commented Nov 2, 2015

No, I'm sorry, @escowles , I meant at the micro-level: why does -rdfStream.concat(filter(getTriples(HashRdfContext.class), tripleFilter::test)); +rdfStream.concat(filter(getTriples(HashRdfContext.class), IS_MANAGED_TRIPLE.negate()::test));

do the job?

@escowles
Copy link
Contributor Author

escowles commented Nov 2, 2015

Oh, I see: we are avoiding the logic surrounding the server-managed triple Prefer header and always rejecting them. I'll add a comment to that effect.

@ajs6f
Copy link
Contributor

ajs6f commented Nov 2, 2015

Right, right. Thanks!

@escowles escowles force-pushed the fcrepo-1800-suppress-fragment-servermanaged-triples branch from b8276af to 8d15050 Compare November 2, 2015 18:52
@escowles
Copy link
Contributor Author

escowles commented Nov 3, 2015

@ajs6f Anything else, or is this ready to merge?

@ajs6f
Copy link
Contributor

ajs6f commented Nov 3, 2015

No, I'm grruvy. Want me to merge?

@escowles
Copy link
Contributor Author

escowles commented Nov 3, 2015

Yes, please!

ajs6f added a commit that referenced this pull request Nov 3, 2015
…rvermanaged-triples

Filtering out server-managed triples about hash URI fragments
@ajs6f ajs6f merged commit fbde4f7 into master Nov 3, 2015
@acoburn acoburn deleted the fcrepo-1800-suppress-fragment-servermanaged-triples branch November 4, 2015 04:11
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.

None yet

2 participants