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

Is RelationshipType defined properly in CALM? #826

Open
LeighFinegold opened this issue Jan 23, 2025 · 4 comments
Open

Is RelationshipType defined properly in CALM? #826

LeighFinegold opened this issue Jan 23, 2025 · 4 comments

Comments

@LeighFinegold
Copy link
Member

LeighFinegold commented Jan 23, 2025

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
Copy link
Member

Option 2 certainly seems cleaner if it's valid.

@jpgough-ms
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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?

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

No branches or pull requests

4 participants