Is RelationshipType defined properly in CALM?

[LeighFinegold]

Support Question

For a lot of tooling e.g. visualiser, generator we are defining a model to work with.

Whilst experimenting with json schema to pojo generator as an additional replacement for the java generator module (#702) , a question arose around the relationship type modelling

Option 1

export interface Relationship {
  "unique-id": string;
  description?: string;
  "relationship-type": {
    interacts?: InteractsType;
    connects?: ConnectsType;
    "deployed-in"?: DeployedInType;
    "composed-of"?: ComposedOfType;
    [k: string]: unknown;
  };

represented by this json schema definition

 "relationship": {
      "type": "object",
      "properties": {
        "unique-id": {
          "type": "string"
        },
        "description": {
          "type": "string"
        },
        "relationship-type": {
          "type": "object",
          "properties": {
            "interacts": {
              "$ref": "#/defs/interacts-type"
            },
            "connects": {
              "$ref": "#/defs/connects-type"
            },
            "deployed-in": {
              "$ref": "#/defs/deployed-in-type"
            },
            "composed-of": {
              "$ref": "#/defs/composed-of-type"
            }
          },
          "oneOf": [
            {
              "required": [
                "deployed-in"
              ]
            },
            {
              "required": [
                "composed-of"
              ]
            },
            {
              "required": [
                "interacts"
              ]
            },
            {
              "required": [
                "connects"
              ]
            }
          ]
        }
}

Even if we keep with this option, in all json schema examples, I don't think the property section should exist

Option 2

export interface Relationship {
  "unique-id": string;
  description?: string;
  "relationship-type": ComposedOfType | InteractsType | ConnectsType | DeployedInType;

which is represented

"relationship": {
      "type": "object",
      "properties": {
        "unique-id": {
          "type": "string"
        },
        "description": {
          "type": "string"
        },
        "relationship-type": {
          "oneOf": [
            { "$ref": "#/defs/composed-of-type" },
            { "$ref": "#/defs/interacts-type" },
            { "$ref": "#/defs/connects-type" },
            { "$ref": "#/defs/deployed-in-type" }
          ]
        }
}

This seems more inline with Device Type example on https://json-schema.org/learn/json-schema-examples

[rocketstack-matt]

Option 2 certainly seems cleaner if it's valid.

[jpgough-ms]

I think this is a good example of #818

I agree with @LeighFinegold and @rocketstack-matt that option 2 is better, but it does loosen the JSON Schema validation of a relationship. This might not be an issue, if we agree that our JSON schema isn't perfect but pragmatic.

Current Implementation:

Enforces a stricter validation logic since the relationship-type must be an object with defined properties and exactly one property from the oneOf list.

Proposed Implementation:

Looser validation since relationship-type can directly resolve to one of the four $ref types without enforcing additional structure.

Summary:

• If the intent is to have relationship-type always be an object with exactly one of the specified properties (interacts, connects, etc.), we need what we have.
• If the intent is for relationship-type to be one of several types (not necessarily an object with a specific property). However, if we say that we're happy to loosen the constraint here and have spectral assert that it is valid CALM - then that would be fine.

[aidanm3341]

Just to add some support for option 2 as well, I actually already modeled it this way in the manual TypeScript types that I wrote for CALM a while back https://github.com/finos/architecture-as-code/blob/main/shared/src/types.ts#L19

I think having the weaker JSON Schema validation is a worthwhile trade off.

[rocketstack-matt]

I'd missed the subtlety . . . but is it looser or just different? Isn't the difference that in Option 2 it simply can't have additional properties, but is still strict in the sense that it must be one of the declared relationship types?

[jpgough-ms]

I think the outcome of this discussion is that yes, it is defined correctly in CALM, however there is a question about whether we change the schema for a slightly looser definition and use validation etc going forward.

@LeighFinegold I think we should close this issue and then put it through @rocketstack-matt new Schema Proposal flow after we get v1 out. We can leave it open until that happens - I'll assign you and tag as won't fix

[markscott-ms]

I believe Option 2 could not distinguish between a composed-of and deployed-in relationship, as both types have two properties - container and nodes. There is no other required distinguishing property.

Option 3

Directly aligned with the Device Type example on https://json-schema.org/learn/json-schema-examples#device-type, using the tagged union concept.

"relationship": {
   "type": "object",
   "properties": {
     "unique-id": {
       "type": "string"
     },
     "description": {
       "type": "string"
     },
     "relationship-type": {
       "type": "string"
     }
   },
   "oneOf": [
      { "properties": { "relationship-type" : { "const" : "composed-of" } }, "$ref": "#/defs/composed-of-type" },
      { "properties": { "relationship-type" : { "const" : "interacts" } }, "$ref": "#/defs/interacts-type" },
      { "properties": { "relationship-type" : { "const" : "connects" } }, "$ref": "#/defs/connects-type" },
      { "properties": { "relationship-type" : { "const" : "deployed-in" } }, "$ref": "#/defs/deployed-in-type" },
   ]
}

which would give a much flatter relationship object to look like:

{ 
   "unique-id" : "my-id",
   "description": "widget containment",
   "relationship-type": "deployed-in",
   "container": "node1",
   "nodes" : [ "node2", "node3", "node4" ]
}

[jpgough-ms]

As mentioned, this needs to go through the new new schema proposal workflow if we feel that this is still needed