POST/PATCH must advertise constraints if triples not persisted #808

whikloj · 2015-06-02T13:58:50Z

ajs6f · 2015-06-02T14:05:02Z

fcrepo-kernel-impl/src/main/java/org/fcrepo/kernel/impl/FedoraResourceImpl.java

@@ -493,6 +494,8 @@ public void replaceProperties(final IdentifierConverter<Resource, FedoraResource
        try {
            new RdfRemover(idTranslator, getSession(), replacementStream
                    .withThisContext(differencer)).consume();
+        } catch (final ConstraintViolationException e) {


Do we want to immediately rethrow a ConstraintViolationException or do we want to accumulate them to give a more full report?

You mean like we are doing directly below this with the MalformedRdfException?
I was a little concerned about how we are reporting if we violate multiple constraints. In which case maybe our constraints should be a single file and the exception message would have to identify which constraint? Not sure...

Yeah, that's a very good point. I think you've got it right now. Most folks are not going to throw in RDF with many many violations, anyway.

ajs6f · 2015-06-02T14:10:38Z

On the whole this looks really nice.

@whikloj ++

Just a few comments in-line, and I do want to make sure I understand how we are publishing the RDF/HTML and if we are doing it efficiently.

ajs6f · 2015-06-02T14:12:15Z

fcrepo-kernel/src/main/java/org/fcrepo/kernel/exception/ConstraintViolationException.java

+     * Generates a link header for a constraint based on UriInfo and the name of the Exception class.
+     * @return Link
+     */
+    public static Link buildConstraintLink(final ConstraintViolationException e, final UriInfo uriInfo) {


Since the expected clients of this method are actually subclasses of this class, you could just make it an instance method, which would shorten the code a bit.

I seem to have to pass the uriInfo into this method regardless. If I don't, then it becomes null. Is this an aspect of @context or am I doing something wrong?

I am a moron, I had @context in all the ExceptionMappers and not in the Exceptions. I think I got this.

Creating OutOfDomainSubjectException subclass of ConstraintViolationException Moving ServerManagedPropertyException to subclass of ConstraintViolationException Added OutOfDomainSubjectExceptionMapper, ConstraintViolationExceptionMapper Changed some Exceptions thrown at JcrPropertyStatementListener

Propagate out of PersistingRdfStreamConsumer and FedoraResourceImpl

@context

Tried to limit @context from bleeding over sub-classes, ended up passing it into the method.

…Consumer

awoods · 2015-06-22T20:01:47Z

fcrepo-kernel/src/main/java/org/fcrepo/kernel/exception/ConstraintViolationException.java

+     * Generates a link header for a constraint based on UriInfo and the name of the Exception class.
+     * @return Link
+     */
+    public Link buildConstraintLink(final UriInfo uriInfo) {


Please add a unit test for this class/method.

How do you mock up a URI? I keep getting a target invocation error. Everything is working great then it executes the buildConstraintLink twice and tosses some exception to the JUnit.

See: https://github.com/fcrepo4/fcrepo4/blob/master/fcrepo-http-api/src/test/java/org/fcrepo/http/api/FedoraFixityTest.java#L70-L71

awoods · 2015-06-22T20:04:56Z

Please add a new integration test method to FedoraLdpIT that creates an error case and verifies the created link header.

awoods · 2015-06-22T20:14:18Z

It would be helpful to have an integration test that verifies:
** the existence of the static RDF pages are available in fcrepo-webapp (see: https://github.com/fcrepo4/fcrepo4/blob/master/fcrepo-webapp/src/test/java/org/fcrepo/integration/SanityCheckIT.java)
** the existence of the static RDF pages in the codebase, with the proper name, that correlates to the respective Exception classes.

Update ExceptionMappers to use new superclass. Add unit test. Add integration test.

whikloj · 2015-06-24T21:09:08Z

Regarding this SanityCheck, I think that it:

Would need the issue about Grizzly not serving static pages to be resolved first.
Because there are RDF files for a subset of Exceptions (subclasses of ConstraintViolationException), I'm not sure how you would locate those Exceptions.

As always, if you want to give me some Java education on a way to do this, I can add it to the PR. 😉

ajs6f · 2015-06-25T13:27:06Z

...t/java/org/fcrepo/http/commons/exceptionhandlers/ConstraintViolationExceptionMapperTest.java

+
+    @Before
+    public void setUp() throws RepositoryException, URISyntaxException {
+        initMocks(this);


You might want to use @RunWith(MockitoJUnitRunner.class), it's a bit crisper to my eye.

Related to: https://jira.duraspace.org/browse/FCREPO-1418

awoods · 2015-06-25T15:33:39Z

See: whikloj#2
...will look at the SanityIT.java later.

Move string format into OutOfDomainException constructor, update calls to Exception.

Add test to ensure Constraints RDF exists

Related to: https://jira.duraspace.org/browse/FCREPO-1418

awoods · 2015-06-25T20:35:19Z

See: whikloj#3 for the SanityIT.

Add test to verify Constraint Link header

awoods · 2015-06-28T22:24:26Z

fcrepo-kernel/src/main/java/org/fcrepo/kernel/exception/ConstraintViolationException.java

+ */
+package org.fcrepo.kernel.exception;
+
+import javax.ws.rs.core.UriInfo;


We do not want a javax.ws.rs dependency in fcrepo-kernel. You should be able to remove the following method:
https://github.com/fcrepo4/fcrepo4/pull/808/files#diff-c14ecfb343036db02d3070d59bd47b0eR64
...and perform all of the buildConstraintLink logic in ConstraintExceptionMapper, no?

Removed unused constructor in ConstraintViolationException and sub-classes.

awoods · 2015-06-29T17:09:22Z

Resolved with: 447e56a

ajs6f reviewed Jun 2, 2015
View reviewed changes

whikloj added 5 commits June 18, 2015 22:14

Update FedoraLdpIT test

1ea2a10

Propagate out of PersistingRdfStreamConsumer and FedoraResourceImpl

Changed method from static to instance

ab085ea

Tried to limit @context from bleeding over sub-classes, ended up passing it into the method.

Added ManagedTypeException, Mapper and throw from PersistingRdfStream…

30e3693

…Consumer

Rename ManagedTypeException to ServerManagedTypeException

12379c3

whikloj force-pushed the FCREPO-1418 branch from 1238fe6 to 12379c3 Compare June 19, 2015 03:19

Forgot to rename the constraint RDF.

5b5ffb8

awoods reviewed Jun 22, 2015
View reviewed changes

whikloj added 2 commits June 24, 2015 15:48

Make ConstraintExceptionMapper superclass.

7749a49

Update ExceptionMappers to use new superclass. Add unit test. Add integration test.

Update owl2html.xsl

e78cc9e

ajs6f reviewed Jun 25, 2015
View reviewed changes

Add test to ensure Constraints RDF exists

540af69

Related to: https://jira.duraspace.org/browse/FCREPO-1418

whikloj and others added 3 commits June 25, 2015 12:57

Switch to MockitoJUnitRunner

5d30af3

Move string format into OutOfDomainException constructor, update calls to Exception.

Merge pull request #2 from awoods/fcrepo-1418

cab1ce6

Add test to ensure Constraints RDF exists

Add test to verify Constraint Link header

48f7326

Related to: https://jira.duraspace.org/browse/FCREPO-1418

Merge pull request #3 from awoods/fcrepo-1418

8b78eda

Add test to verify Constraint Link header

awoods reviewed Jun 28, 2015
View reviewed changes

whikloj added 2 commits June 29, 2015 09:43

Moved all constrainedBy link building to ExceptionMapper.

e7ee80d

Removed unused constructor in ConstraintViolationException and sub-classes.

< is < than < so replace < with < where necessary

4d664bf

awoods closed this Jun 29, 2015

awoods deleted the FCREPO-1418 branch June 29, 2015 17:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

POST/PATCH must advertise constraints if triples not persisted #808

POST/PATCH must advertise constraints if triples not persisted #808

whikloj commented Jun 2, 2015

ajs6f Jun 2, 2015

whikloj Jun 2, 2015

ajs6f Jun 2, 2015

ajs6f commented Jun 2, 2015

ajs6f Jun 2, 2015

whikloj Jun 2, 2015

whikloj Jun 2, 2015

awoods Jun 22, 2015

whikloj Jun 23, 2015

awoods Jun 23, 2015

awoods commented Jun 22, 2015

awoods commented Jun 22, 2015

whikloj commented Jun 24, 2015

ajs6f Jun 25, 2015

awoods commented Jun 25, 2015

awoods commented Jun 25, 2015

awoods Jun 28, 2015

awoods commented Jun 29, 2015

POST/PATCH must advertise constraints if triples not persisted #808

POST/PATCH must advertise constraints if triples not persisted #808

Conversation

whikloj commented Jun 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajs6f commented Jun 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awoods commented Jun 22, 2015

awoods commented Jun 22, 2015

whikloj commented Jun 24, 2015

Choose a reason for hiding this comment

awoods commented Jun 25, 2015

awoods commented Jun 25, 2015

Choose a reason for hiding this comment

awoods commented Jun 29, 2015