post processing validation #557
Conversation
Fixes #533 sh:datatype, sh:hasValue, and sh:nodeKind constraints were only being enforced during transactions, and not when referenced by a "graph crawling" shape. This change ensures these constraints are always applied.
There was a problem hiding this comment.
With this PR, is it the case that there will be instances where constraints are checked twice? IIRC you said offline that you'd be doing some followup for optimizations, does that include potentially getting rid of the mid-processing versions of these to avoid duplicate checking?
src/fluree/db/json_ld/shacl.cljc
Outdated
| [false (str "sh:in: value " val " must be one of " in)])) | ||
| has-value-results (when has-value | ||
| (if (some #(= (flake/o %) has-value) p-flakes) | ||
| [true (str "sh:not sh:hasValue: at least one value must not be " has-value)] |
There was a problem hiding this comment.
I tend to get kind of mixed up with the sh:not cases, but I'm not sure this is the right message here. I thought it would be be that none of the values can have the has-value?
| (is (util/exception? invalid-person)) | ||
| (is (= "SHACL PropertyShape exception - sh:hasValue: at least one value must be true." | ||
| (ex-message invalid-person))))) | ||
| (testing "extended path contstraints" |
There was a problem hiding this comment.
typo in "constraints" here
| "type" "ex:User" | ||
| "ex:friend" {"@id" "ex:Bob"}})] | ||
| (is (util/exception? db-forbidden-friend)) | ||
| (is (= "SHACL PropertyShape exception - sh:datatype: every datatype must be 1." |
There was a problem hiding this comment.
I don't think this matches the error message you'd get with the mid-transaction-processing version of this constraint check. If we're going to keep those versions around, I think the error messages should match -- from the user's perspective, it's the same constraint, so different error messages for post/mid processing wouldn't make sense.
There was a problem hiding this comment.
This is an interesting one, when I try a naive mid-processing datatype validation I actually get a coercion error instead of a validation error, which makes sense as coercion happens before validation.
|
Regarding the optimizations, what I'm working on right now segregates property shapes into "eligible for mid-processing" and "everything else" buckets, where the main differentiator is whether any graph crawling is necessary. So, it aims to hit that "correct, but fast" sweet spot when its available but will only do the work once, either during or after processing. |
Also fix a typo
Adds validation for sh:nodeKind, sh:hasValue, and sh:datatype when they are not explicitly part of the transacted flakes.
Fixes #533