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

https://jira.duraspace.org/browse/FCREPO-1411 #836

Closed
wants to merge 4 commits into from

Conversation

Projects
None yet
4 participants
@ajs6f
Copy link
Contributor

commented Jul 13, 2015

return input.getPredicate().equals(RDF.type.asNode())
&& isManagedNamespace.apply(input.getObject().getNameSpace());
}
}));
}.negate().and(not(isManagedTriple)::apply);

This comment has been minimized.

Copy link
@acoburn

acoburn Jul 13, 2015

Contributor

why not use Predicate::or(isManagedTriple).negate() here?

This comment has been minimized.

Copy link
@ajs6f

ajs6f Jul 13, 2015

Author Contributor

Because I stared at this too long to understand it or care any more. Send me a PR that seems clearer to you and I will happily merge it.

This comment has been minimized.

Copy link
@ajs6f

ajs6f Jul 13, 2015

Author Contributor

The whole mess of "isManagedFoot" and "isManagedEar" and specific type-tests needs to have the stuffing kicked out of it and beaten into something that makes some kind of sense, here and in the kernel.

final RdfStream resourceTriples)
throws MalformedRdfException, AccessDeniedException {
if (resource instanceof NonRdfSourceDescription) {
// update the description instead

This comment has been minimized.

Copy link
@awoods

awoods Jul 13, 2015

Member

This comment seems misleading. // update the description instead
The logic here is updating the described resource, i.e. the binary, not the description.
According to your intent, is the comment wrong or the logic?

This comment has been minimized.

Copy link
@ajs6f

ajs6f Jul 14, 2015

Author Contributor

You're quite right-- that was from an earlier version. I'll fix the comment.

This comment has been minimized.

Copy link
@awoods

awoods Jul 14, 2015

Member

If we are putting all of the description on the binary, then what is the use of the NonRdfSourceDescription? just a URI to GET/PATCH on that under the covers actually interacts with the binary resource properties?
Also, this seems to be in conflict with the following ticket, which is moving in the direction of making the NonRdfSourceDescription a regular container: https://jira.duraspace.org/browse/FCREPO-1590

This comment has been minimized.

Copy link
@ajs6f

ajs6f Jul 14, 2015

Author Contributor

I agree that fcr:metadata makes little sense and I have argued in the past to get rid of it. I don't see that there is any conflict with that ticket. You're welcome to offer a different design.

This comment has been minimized.

Copy link
@awoods

awoods Jul 14, 2015

Member

We need to make sure that if we are going with the design in this ticket, that it reconcile with:
https://jira.duraspace.org/browse/FCREPO-1540 and
https://jira.duraspace.org/browse/FCREPO-1590

This comment has been minimized.

Copy link
@escowles

escowles Jul 14, 2015

Contributor

I think there are a couple of different priorities here, and different people care about different things, leading to some of this always-shifting discussion. Some of the priorities are:

  1. Full-featured descriptions of binaries (e.g., Skolemized bnodes, hash URI resources, child containers, etc.)
  2. Binary descriptions tightly bound to the binary (e.g., deleted when the binary is deleted).
  3. Request URI == RDF subject URI of the triples returned

My understanding was that the way we were going to handle these competing priorities was to keep fcr:metadata, make sure all binary properties used it as the RDF subject, and then add container/bnode/hashURI features to it.

This comment has been minimized.

Copy link
@awoods

awoods Jul 14, 2015

Member

Thanks, @escowles. I share your understanding.

  1. Given the priorities as you listed, do you see an architectural conflict with the approach presented in this PR?
  2. @ajs6f, do you agree with the above noted priorities? and do you see an architectural conflict between the priorities and the approach presented in this PR?
  3. Would storing properties on the binary vs. description resource be a more constructive approach?

This comment has been minimized.

Copy link
@ajs6f

ajs6f Jul 14, 2015

Author Contributor

We need to take this to a better forum than comments on an outdated diff.
Otherwise, your understanding is not supported by any of the conversation so far. For example, neither of the tickets about subjects of triples in fcr:metadata (1540 or 1635) demand that the subjects of the triples in question be fcr:metadata, as @escowles describes. They both demand that the subjects in question be the NonRDFSource, instead.
In other words, we don't have a coherent policy here that we can implement and we need to discuss this more publicly to get one.

This comment has been minimized.

Copy link
@awoods

awoods Jul 14, 2015

Member

+1 to public forum.
Yes, I misread @escowles comment: My understanding was that the way we were going to handle these competing priorities was to keep fcr:metadata, make sure all binary properties used it as the RDF subject.
My understanding was that the properties were to be stored on fcr:metadata but have the subject of the binary.

This comment has been minimized.

Copy link
@ajs6f

ajs6f Jul 14, 2015

Author Contributor

Right, but that's not what any of the issues say and it's not what I did here. Whether it's what we want to do or not, let's find out in a larger discussion.

@@ -592,7 +593,7 @@ public void testPatchBinaryDescription() throws Exception {
patch.addHeader("Content-Type", "application/sparql-update");
final BasicHttpEntity e = new BasicHttpEntity();
e.setContent(new ByteArrayInputStream(
("INSERT { <" + location +
("INSERT { <" + serverAddress + pid + "/x" +

This comment has been minimized.

Copy link
@awoods

awoods Jul 13, 2015

Member

This integration test update would indicate that the logic:
https://github.com/fcrepo4/fcrepo4/pull/836/files#diff-d372035f89c8d0846920e20bd13e0145R591

...Should be if (resource instanceof NonRdfSource)

This comment has been minimized.

Copy link
@ajs6f

ajs6f Jul 14, 2015

Author Contributor

No. Obviously, the test passes, so that can't be the case.

The simple sense is that descriptions contain triples about the described thing, so only triples with the described thing as their subject are legal.

@ajs6f ajs6f force-pushed the FCREPO-1411 branch from 44354d4 to dd84a23 Jul 14, 2015

@awoods

This comment has been minimized.

Copy link
Member

commented Jul 16, 2015

Resolved with: 6e755b2

@awoods awoods closed this Jul 16, 2015

@awoods awoods deleted the FCREPO-1411 branch Jul 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.