New centralised model for CALM (#790) #902

LeighFinegold · 2025-02-14T08:47:09Z

🔍 Summary

This PR introduces a centralized and structured data model for CALM. Per discussion on #899, The new model provides well-defined TypeScript classes and enums for core CALM components, ensuring seamless integration and validation for cli and visualizer componenets.

✨ Key Changes

Added types that match the current schema files.
Introduced CalmCore as the centralized entry point for nodes, relationships, metadata, controls, and flows.
Standardized structure for CalmNode, CalmRelationship, CalmFlow, CalmControl, and CalmMetadata.
Each model includes a fromJson() factory method to enable seamless conversion from JSON.
Ensures consistent handling of nested objects, such as relationships and interfaces.
Added unit tests for all models to ensure stability when we start to amend them to support potential new concepts like visitor pattern etc.

❌ Out of Scope for this PR

Not updating the existing codebase to move away from types.ts yet, as this will wait for the completion of SchemaDirectory changes as per Restructure SchemaDirectory to use document loading strategy #897
Once SchemaDirectory changes are finalised, all existing tooling will be updated to leverage this model and expose it via CLI.
This update will then enable additional template-bundles related to Infrastructure as Code and Solution Architecture document summaries into Code as a follow up PR to First version of calm template for early review (#890) #901

Observations around CALM Specification

We should look to move out metadata into separate json file as definition duplicated in flows.json
This PR corrects a spelling mistake on data-asset node-type
The current core.json doesn't set any requirements on any of the attributes e.g. nodes, relationships. Should it also specify type: object as part of the specification. (Note: generators have required that to be defined)
Should core.json actually have a collection of flowUrls instead of collection of flows? In the same way control detail has a requirementUrl which we expected to be linked to control-requirement.json

aidanm3341 · 2025-02-14T13:22:39Z

shared/src/model/control-requirement.ts

@@ -0,0 +1,17 @@
+/* eslint-disable  @typescript-eslint/no-explicit-any */


hmm, it would be good to not have this linting suppression. What if we use unknown on line 10 instead of any.

Ah yes - that was a topic wanted to raise but forgot. In the end I went with status quo in other classes so let's reiterate this on next meet-up. Will go with unknown

Discussed this further with @aidanm3341 further and we agreed it would have been better to go with object than unknown. However, on further thought, I decided that actually it might be better to define types in the schema structures after all and have added this on latest commit.

aidanm3341 · 2025-02-14T13:23:54Z

shared/src/model/control-requirement.spec.ts

+        expect(control.name).toBe('JSON Control');
+        expect(control.description).toBe('Description from JSON');
+    });
+});


I reckon these data classes be responsible for basic validation that the fields in the input data exist before using them. E.g. if name was not provided, an error should be thrown here

Think its a good question - certainly on #790 we discussed the generator and visualizer would assume the document is validated already. The quesiton is how will the validator work with the model

As long as we're consistent either way, e.g. I noticed that in the flow model you are doing some basic validations "// Ensure value is an object". If we're assuming that the input is valid, then we don't need that validation check there

shared/src/model/relationship.ts

aidanm3341 · 2025-02-14T13:29:57Z

shared/src/model/node.ts

+        public dataClassification?: CalmDataClassification,
+        public runAs?: string,
+        public instance?: string,
+        public interfaces?: CalmInterface[],


to make using these model objects a little nicer to use, we could get rid of the optional aspect of these lists and instead default them to an empty list. That would mean that any client code using these models doesn't have to worry about checking for undefined before iterating over it.

public interfaces: CalmInterface[] = []

Yeah thinkng about it, the optionality I put in because it matched the json-schema btu given we are parsing I like the idea of avoiding such checks.

shared/src/model/node.ts

aidanm3341 · 2025-02-14T13:32:42Z

shared/src/model/node.ts

+export class CalmNodeDetails {
+    'detailed-architecture'?: string;
+    'required-pattern'?: string;
+


also this class doesn't have a constructor like the others? Not sure if I'm missing something with this particular class

Yeah this class clearly got neglected :D

LeighFinegold · 2025-02-16T01:13:18Z

shared/src/types/core-types.ts

+    relationships?: CalmRelationshipSchema[];
+    metadata?: CalmMetadataSchema;
+    controls?: CalmControlsSchema;
+    flows?: CalmFlowSchema[];


One thing I considered was stating that this could be an array of CalmFlowSchema[] | String[]. For now I kept it as this, but it requires the calm doc to be de-referenced before parsing which left as a TODO as requires @willosborne PR work on SchemaDirectory.

LeighFinegold · 2025-02-16T01:20:19Z

shared/src/parser/parser.ts

+        const logger = CalmParser.logger;
+        try {
+            const data = fs.readFileSync(coreCalmFilePath, 'utf8');
+            const dereferencedData: CalmCoreSchema = JSON.parse(data); // TODO: this needs to use SchemaDirectory to parse the other documents e.g. flows.


I don't think this needs to block the PR to get the models in, but we really do need to ensure that the document is dereferenced prior to trying to parse into a model. If we decide we don't want to do that then likely CalmCore.fromJson doesn't get called. I think the individual Calm method fromJson e.g. CalmFlow.fromJson would though.

A similar thing applies to the fact that a CalmControlDetail has reference to a requirementUrl but there is no official linkage to load the CalmRequirement. Added that to description of PR to consider CALM change to have a linkage of flowUrls.

aidanm3341 · 2025-02-17T10:17:18Z

shared/src/types/core-types.ts

+//TODO: There is no required section.
+export type CalmCoreSchema = {
+    nodes?: CalmNodeSchema[];
+    relationships?: CalmRelationshipSchema[];


aren't nodes and relationships required?

Not atm on core.json which is why I put that an observation on this PR description

willosborne · 2025-02-17T13:11:32Z

shared/src/model/interface.ts

+    constructor(public uniqueId: string) {}
+
+    static fromJson(data: CalmInterfaceTypeSchema): CalmInterface {
+        if ('host' in data && 'port' in data) {


This bit is the only part I'm a little iffy about. Happy to proceed with this approach for now but we might need some more thought on this later

willosborne · 2025-02-17T13:11:51Z

shared/src/model/metadata.ts

+
+    static fromJson(data: CalmMetadataSchema): CalmMetadata {
+        const flattenedData = data.reduce((acc, curr) => {
+            return { ...acc, ...curr };


Very nifty.

willosborne · 2025-02-17T13:12:47Z

shared/src/model/metadata.ts

+    constructor(public data: Record<string, unknown>) {}
+
+    static fromJson(data: CalmMetadataSchema): CalmMetadata {
+        const flattenedData = data.reduce((acc, curr) => {


Does this flatten only the top level? We probably don't want it to recursively flatten all the nested objects

Should be only first level, but can add a test case to confirm on subsequent commit.

LeighFinegold requested review from aidanm3341, lbulanti-ms, willosborne, grahampacker-ms, jpgough-ms, rocketstack-matt, Thels and a team as code owners February 14, 2025 08:47

github-actions bot added calm shared labels Feb 14, 2025

aidanm3341 reviewed Feb 14, 2025

View reviewed changes

shared/src/model/relationship.ts Outdated Show resolved Hide resolved

aidanm3341 reviewed Feb 14, 2025

View reviewed changes

shared/src/model/node.ts Outdated Show resolved Hide resolved

aidanm3341 reviewed Feb 14, 2025

View reviewed changes

LeighFinegold force-pushed the template_manual_model branch 2 times, most recently from 0c4d0cf to 94e0ad9 Compare February 16, 2025 01:00

LeighFinegold commented Feb 16, 2025

View reviewed changes

LeighFinegold force-pushed the template_manual_model branch from 94e0ad9 to 8d352ca Compare February 16, 2025 01:21

New centralised model (finos#790)

3725906

LeighFinegold force-pushed the template_manual_model branch from 8d352ca to 3725906 Compare February 16, 2025 01:31

aidanm3341 reviewed Feb 17, 2025

View reviewed changes

willosborne reviewed Feb 17, 2025

View reviewed changes

willosborne approved these changes Feb 17, 2025

View reviewed changes

LeighFinegold merged commit 9db5c1c into finos:main Feb 17, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New centralised model for CALM (#790) #902

New centralised model for CALM (#790) #902

LeighFinegold commented Feb 14, 2025 •

edited

Loading

aidanm3341 Feb 14, 2025

LeighFinegold Feb 14, 2025

LeighFinegold Feb 16, 2025

aidanm3341 Feb 14, 2025 •

edited

Loading

LeighFinegold Feb 14, 2025

aidanm3341 Feb 14, 2025

aidanm3341 Feb 14, 2025

LeighFinegold Feb 14, 2025

aidanm3341 Feb 14, 2025

LeighFinegold Feb 14, 2025

LeighFinegold Feb 16, 2025 •

edited

Loading

LeighFinegold Feb 16, 2025 •

edited

Loading

aidanm3341 Feb 17, 2025

LeighFinegold Feb 17, 2025

willosborne Feb 17, 2025

willosborne Feb 17, 2025

willosborne Feb 17, 2025

LeighFinegold Feb 17, 2025

		@@ -0,0 +1,17 @@
		/* eslint-disable @typescript-eslint/no-explicit-any */

New centralised model for CALM (#790) #902

New centralised model for CALM (#790) #902

Conversation

LeighFinegold commented Feb 14, 2025 • edited Loading

🔍 Summary

✨ Key Changes

❌ Out of Scope for this PR

Observations around CALM Specification

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aidanm3341 Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeighFinegold Feb 16, 2025 • edited Loading

Choose a reason for hiding this comment

LeighFinegold Feb 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeighFinegold commented Feb 14, 2025 •

edited

Loading

aidanm3341 Feb 14, 2025 •

edited

Loading

LeighFinegold Feb 16, 2025 •

edited

Loading

LeighFinegold Feb 16, 2025 •

edited

Loading