feat: Support blank nodes in attribute editor for fixed and default values (RDFA-321)#22
feat: Support blank nodes in attribute editor for fixed and default values (RDFA-321)#22spah-soptim wants to merge 6 commits intomainfrom
Conversation
e463e38 to
95ced91
Compare
d3a4163 to
f5b701b
Compare
Fixed and default attribute values can be persisted in two equivalent shapes in the source RDF: as a direct literal triple, or as a blank-node wrapper (subject -> predicate -> _:blank ; _:blank -> rdfs:Literal -> literal). Until now the model layer collapsed both shapes into a plain literal, so editing an attribute backed by a blank-node value silently dropped that blank node on write. Introduced a shared `AttributeValueNode` base for `CIMSIsFixed` and `CIMSIsDefault` carrying a `blankNode` flag, and a `ValueNodeParser` utility that turns an `RDFNode` into a `ParsedValue(value, dataType, blankNode)` while validating the blank-node shape (exactly one statement with `rdfs:Literal` predicate and a literal object). `CIMQuerySolutionParser` takes an optional `Model` so it can re-resolve blank-node properties from the source dataset; `CIMObjectFactory.createCIMAttributeList` and `CIMObjectFetcher.fetchCIMAttributeList` thread that model through. `CIMUpdates.appendValueNode` emits either a direct literal or a fresh blank-node wrapper depending on `blankNode`. Because removing an attribute via `?attr ?p ?o` does not reach the inner triples of an attached blank-node value, `replaceAttribute`/`replaceAttributes` now return an `UpdateRequest` composed of a separate blank-node cleanup, the SPO delete and the insert; `deleteClass` performs the same cleanup before deleting its attributes. `InMemorySparqlExecutor` gains an `executeSingleUpdate(UpdateRequest)` overload to run those compound requests in a single write transaction.
Adds the resolver that decides, for every attribute being created or replaced, whether its fixed/default value should be persisted as a direct literal or as a blank-node wrapper: - if the attribute already exists in the graph, the existing shape wins (a blank-node value stays a blank-node value, a literal stays a literal), - otherwise the new `attributes.newValuesBlankNode` config flag decides; defaults to `false` so behaviour is unchanged for fresh installs. shape as `AssociationsService` — a single `resolve()` followed by a `AttributeFixedDefaultResolver` is a `@Service` with the config injected via `@Value` constructor parameter. It self-manages a READ transaction when none is active and reads inline when called from inside an existing WRITE transaction (e.g. `UpdateClassService.replaceClass`), so callers don't need transaction boilerplate. `AttributesService` keeps the same single `executeSingleUpdate()`. `AttributeMapper` now resolves the canonical XSD datatype URI (`xsd:string` instead of `xsd:String`) for fixed/default values via `XSDDatatypeMapper`; the resolver re-resolves it from the graph for non-primitive attribute datatypes. Frontend: `mapAttributeDtoToReactiveAttribute` passes `fixedValue` and `defaultValue` through so they are available on the reactive object. Signed-off-by: Jan-Hendrik Spahn <jan-hendrik.spahn@soptim.de>
…te insert Signed-off-by: Jan-Hendrik Spahn <jan-hendrik.spahn@soptim.de>
…TOCTOU
- CIMUpdates.deleteAttributeValueNodes{,ForClass} now build their SPARQL
via UpdateBuilder + ExprFactory.isBlank instead of raw string concat,
so user-controlled classUUID and graphURI flow through Jena's node
coercion and can no longer be used to inject SPARQL.
- InMemorySparqlExecutor gains an executeSingleUpdate overload that
takes a Function<Graph, UpdateRequest> and runs build + execute in a
single WRITE transaction. AttributesService uses it so the
fixed/default shape resolution and the resulting update happen
atomically — the previous read-then-write split could see another
writer change the existing value's shape between the two
transactions.
- AttributeFixedDefaultResolver no longer self-manages a READ
transaction; the caller's open transaction is required (and is now
always a WRITE).
- ValueNodeParser.parseLiteral takes a Literal directly so the contract
is enforced at the call site.
- Add a comment explaining why insertAttribute uses BIND to force the
WHERE clause to match exactly once.
Signed-off-by: Jan-Hendrik Spahn <jan-hendrik.spahn@soptim.de>
f5b701b to
d8ac3d3
Compare
Signed-off-by: Jan-Hendrik Spahn <jan-hendrik.spahn@soptim.de>
| graph.begin(TxnType.WRITE); | ||
| var cimClass = classMapper.toCIMObject(newClass); | ||
| // resolver inherits the open WRITE txn for its read-only attribute lookup | ||
| fixedDefaultResolver.resolve(graph, cimClass.getAttributes()); |
There was a problem hiding this comment.
I still find this call questionable. It suggests that either classMapper.toCIMObject or CIMUpdates.replaceClass isn't working properly and needs an external fix.
| public void executeSingleUpdate( | ||
| GraphRewindableWithUUIDs graph, UpdateRequest update, String graphUri) { | ||
| executeSingleUpdate(graph, graphUri, g -> update); | ||
| } | ||
|
|
There was a problem hiding this comment.
Does this method make sense, if it's only used in unit tests?
| public void executeSingleUpdate( | ||
| GraphRewindableWithUUIDs graph, | ||
| String graphUri, | ||
| Function<GraphRewindableWithUUIDs, UpdateRequest> updateBuilder) { |
There was a problem hiding this comment.
I don't really see the need to use a lamba. Why not just prepare the argument beforehand and then pass it? And if you need to access the graph beforehand then just don't use the executeSingleUpdate, which makes this overly complicated.
| String classUUID, | ||
| List<CIMAttribute> attributes) { | ||
| var baseUpdate = CIMUpdates.deleteAttributes(prefixMapping, graphURI, classUUID); | ||
| var updateRequest = deleteAttributeValueNodesForClass(graphURI, classUUID); |
There was a problem hiding this comment.
Doesn't it make more sense to just enhance the delete Attribute Query instead of doing this separately, since value nodes are part of an attribute.
| PrefixMapping prefixMapping, String graphURI, CIMAttribute attribute) { | ||
| var baseUpdate = deleteAttribute(prefixMapping, graphURI, attribute.getUuid()); | ||
| return appendInsertAttribute(baseUpdate, attribute); | ||
| var updateRequest = deleteAttributeValueNodes(graphURI, attribute.getUuid()); |
There was a problem hiding this comment.
Same here. this should not be its own method, but part of delete Attribute.
| * the properties of an attribute fixed/default value that was returned from the query as a | ||
| * blank node. May be {@code null} when blank-node values are not expected. | ||
| */ | ||
| private final Model valueNodeModel; |
There was a problem hiding this comment.
The CIMQuerySolutionParser should not have a model. Its job is to parse the result of a query, not query the model itself. Instead the query should be extended. For attributes that would be the query created in CIMQueries.getAttributesQuery.
| @@ -0,0 +1,128 @@ | |||
| /* | |||
There was a problem hiding this comment.
If you just parse a query result in CIMQuerySolutionParser this file is either obsolete or heavily refactored to parse a query result instead of searching the model itself
| graph, | ||
| graphUri, | ||
| g -> { | ||
| fixedDefaultResolver.resolve(g, cimAttribute); |
There was a problem hiding this comment.
should either inside CIMUpdates.insertAttribute or attributeMapper.toCIMObject
| graph, | ||
| graphUri, | ||
| g -> { | ||
| fixedDefaultResolver.resolve(g, cimAttribute); |
| graph, | ||
| graphUri, | ||
| g -> { | ||
| fixedDefaultResolver.resolve(g, attributeCIMObjects); |
Description
Added RDF blank-node support for attribute
cims:isFixed/cims:isDefaultvalues, in addition to literals.CIMSIsFixed/CIMSIsDefault) carriesvalue,dataType, and ablankNodeflag.ValueNodeParserreplaces the previous literal/blank-node extraction inCIMQuerySolutionParser. The fetcher now passes the datasetModelthrough so blank-node properties can be enumerated.AttributeFixedDefaultResolverdecides per-attribute whether the new value should be literal vs. blank-node - preserves existing shape, otherwise defaults toattributes.newValuesBlankNode(default false).CIMUpdatesnow emits a multi-statementUpdateRequestthat first cleans orphan blank-node triples (since?attribute SPOdoes not reach the inner triples of nested resources), then deletes the attribute, then inserts the new one.AttributeMapper.buildXsdDatatypenow goes throughXSDDatatypeMapper(now for examplexsd:stringnotxsd:String).fixedValue/defaultValue.Test Checklist
General Behavior
Global MenuBar
Welcome Page
Editor - MenuBar
Editor - Navigation
Editor - Package View
Editor - Class Editor
Prefixes Page
Changelog Page
Compare Page