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

Dev/api init #4

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Dev/api init #4

wants to merge 4 commits into from

Conversation

dlamoris
Copy link
Contributor

initial api <-> backend prototype for discussion

Note this is still preliminary code for discussion
DId some minor refactoring, added "RdfService4jV1 - which does "triple level" updates as well as sparql updates. This can serve as an example of different ways to implement a back end.
Copy link
Contributor Author

@dlamoris dlamoris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notes and questions

* @property graphKey Key of graph holding these predicates.
* @property rdfService RdfService supplying utility functions
*/
public class PredicateSpec(val predicate:String, val schemaType:String, var valueType:String?, val graphKey:String, val rdfService:RdfService) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were discussions a few months ago regarding the same attribute names in different sysml classes and how they'll be represented in rdf - I think the consensus was for most and those with the same range it'll just be the name with no type prefix, but there'll be some where it's handled by implementation? Want to know if the schema classes would include lookups for these exceptions

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no exceptions required, the RDF is essentially overloaded - the same predicate is used. The OWL uses restrictions to contextualize it. In the RdfService code it uses predicateSpec for the differentiation - predicateSpec is based on both the container class and the json term.

logApiError("Error: No identifier for $sourceJson")
}

if (sourceJson["@type"] != null) this.typeName = sourceJson["@type"]?.jsonPrimitive?.content
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the openapi spec, the @type of a payload element is always required - this has major performance implications because if we can count on the @type of the payload always being available in the sourceJson, there's no need to get existing elements to construct the sparql updates (since the typeName and key is used to lookup the predicate)
Should we also be able to reject the request if @type is not provided, or just default to 'element' as is done here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type is required in payload but not for the project objects. But, since we can't differentiate a create from an edit (uggg), we have to either query or delete existing content - I can handle either approach but implemented deleseContent based on Blakes preference for flexo. Note that some of these records are huge - nothing "micro" about them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the delete we mentioned is actually a replace of the specific triple - since the payload may not be complete object deleting the entire element and readding what's from the json may actually delete data
something like this

delete {
            <urn:sysmlv2:uuid> sysml:predicate ?o
        } insert {
        	<urn:sysmlv2:uuid> sysml:predicate value
        } where {
        	optional {
        		<urn:sysmlv2:uuid> sysml:predicate ?o
        	}
        }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete or edit at the triple level (which you show), or the "element" level. Last week Blake suggested it best to delete the whole object then reconstruct. We can try it both ways, see what performs better. Keep in mind there can be thousands of these in a single commit.

Also right now each delete is an update fragment, it MAY be better to do all the deletes in a list using a single update - again, just not sure what would perform better - once we get to "working" we can optimize. I have seen some RDF-DB not do so well with expansive queries/updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right - i was just pointing out depending on whether you get the existing element (which itself can be expensive for many elements) affects which option of sparql update is viable - because the payload can have partial content, in order for the delete whole element and reconstruct to work you HAVE to get the existing element (as well as insert/delete triple literals), whereas the delete/insert/where doesn't have the requirement that the existing element is available

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind, i didn't realize the posted data are not partial objects

this.gc.metamodelSpec?.getPredicateSpec(this.typeName!!, key)
if (predicateSpec==null) {
//logApiError("Warning: Predicate <$key> not known in <$typeName> - defaults used")
predicateSpec = rdfService.makeDefaultPredicateSpec(this.typeName, key) // ToDo make value sensitive
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should reject or allow?
This can go down a rabbit hole about model validation also (if the input changes 'owner' of 1 element but not 'owned***' of its parent elements)
Punt down to future work and assume the client makes model consistent changes for now

inner class DataVersionObject(sourceJson: JsonObject, val isNewPayload:Boolean=false) : ApiObject(rdfService.gcProject, sourceJson, "DataVersion", isNew=true) {
val payload: JsonObject? = sourceJson["payload"]?.jsonObject
var identityId: String? = sourceJson["identity"]?.jsonObject?.get("@id")?.jsonPrimitive?.content // Identity of payload
val identity:String = if (identityId!=null) identityId!! else rdfService.getUniqueUUID()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The openapi spec indicates that only payload is required - seems identity should only be generated if both payload's @id and identity are missing (in the case where payload is null then identity would be required) (the spec actually says for new elements identity should be null)
Can't find if any of the specs say the client cannot include @id in payload if creating a new element
Would it be fair to assume that if we're generating a UUID that it's a new element
Also that if id is provided it can be an existing element OR a new element

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Payload is null for a delete, for edit/create there must be a payload and the identity comes from the IdentityObject, if the payload also has @id I check that they match. The spec seems a bit fuzzy about creating new UUIDs. Currently, I do create a new ID for a payload if one is not provided by either the IdentityObject or Payload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking at page 31 of the api services pdf and for createCommit mapping it said this
For creating new Data, CommitRequest.change should include a DataVersion where DataVersion.payload includes the Data being created and DataVersion.identity is not specified.
If the payload has an @id then i think later in the code the compare will fail because identity is being generated here and passed into payloadObject

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DataVersion has a different id than the payload or DataIdentity and needs to be persisted to the project's graph.
I know, this API is very strange in this area

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not the id of the DataVersion (actually the openapi.json doesn't even include the DataVersion id) the comparison failure happens if identity isn't specified (and gets generated here) and payload has an @id (which the spec doesn't really say whether it should be there for new elements)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DataVersion has a required ID in the "PIM" but none in the API - very confusing. we would want to make up an ID for the DataVersion (I would allow it to be supplied as this may change), if the payload has an @id then that is to be used, you don't make one up. What they were trying to do is create an API wrapper that did not impose an idea of identity on the model content.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind was looking at the wrong version spec

RdfService can be used to either call an external service or return a set of updates. A "Mock flexo" test shows an example for commit. Example for query still pending.
For use as a "layer", we may want to change to suspend functions.
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.

2 participants