Skip to content

Commit 522526b

Browse files
jpgough-msjpgough-ms
authored
fix(calm-hub): align deployment decorator target prefix and surface malformed JSON errors (finos#2669)
Route Drawer and DiagramSection through a shared fetchDeploymentDecoratorsForArchitecture so both use the canonical /api/calm prefix with numeric-id resolution. Add extractIdFromJsonStrict so control POST handlers return a precise 400 on malformed JSON instead of falling through to a dead catch. Closes finos#2656
1 parent 2b6d587 commit 522526b

10 files changed

Lines changed: 178 additions & 88 deletions

File tree

‎calm-hub-ui/src/hub/components/diagram-section/DiagramSection.test.tsx‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { beforeEach, describe, it, expect, vi } from 'vitest';
66
import { Data } from '../../../model/calm.js';
77

88
const calmServiceMock = {
9-
fetchDecoratorValues: vi.fn().mockResolvedValue([]),
9+
fetchDeploymentDecoratorsForArchitecture: vi.fn().mockResolvedValue([]),
1010
fetchVersionsByCustomId: vi.fn().mockResolvedValue(['1.0.0', '2.0.0']),
1111
fetchArchitectureTimeline: vi.fn().mockRejectedValue(new Error('no timeline')),
1212
fetchArchitectureSummaries: vi
@@ -62,7 +62,7 @@ vi.mock('./timeline/TimelineBar.js', () => ({
6262

6363
vi.mock('../../../service/calm-service.js', () => ({
6464
CalmService: vi.fn().mockImplementation(function () { return {
65-
fetchDecoratorValues: calmServiceMock.fetchDecoratorValues,
65+
fetchDeploymentDecoratorsForArchitecture: calmServiceMock.fetchDeploymentDecoratorsForArchitecture,
6666
fetchVersionsByCustomId: calmServiceMock.fetchVersionsByCustomId,
6767
fetchArchitectureTimeline: calmServiceMock.fetchArchitectureTimeline,
6868
fetchArchitectureSummaries: calmServiceMock.fetchArchitectureSummaries,
@@ -89,7 +89,7 @@ const patternData: Data & { calmType: 'Patterns' } = {
8989
describe('DiagramSection', () => {
9090
beforeEach(() => {
9191
vi.clearAllMocks();
92-
calmServiceMock.fetchDecoratorValues.mockResolvedValue([]);
92+
calmServiceMock.fetchDeploymentDecoratorsForArchitecture.mockResolvedValue([]);
9393
calmServiceMock.fetchVersionsByCustomId.mockResolvedValue(['1.0.0', '2.0.0']);
9494
calmServiceMock.fetchArchitectureTimeline.mockRejectedValue(new Error('no timeline'));
9595
});
@@ -122,7 +122,7 @@ describe('DiagramSection', () => {
122122
expect(screen.getByTestId('drawer')).toHaveTextContent('Drawer for test-arch');
123123
});
124124

125-
it('uses selected architecture id in deployment decorator target', async () => {
125+
it('fetches deployment decorators via the shared service method with namespace, id and version', async () => {
126126
render(
127127
<MemoryRouter>
128128
<DiagramSection data={architectureData} />
@@ -131,10 +131,10 @@ describe('DiagramSection', () => {
131131

132132
await screen.findByTestId('drawer');
133133

134-
expect(calmServiceMock.fetchDecoratorValues).toHaveBeenCalledWith(
134+
expect(calmServiceMock.fetchDeploymentDecoratorsForArchitecture).toHaveBeenCalledWith(
135135
'arch-namespace',
136-
'/calm/namespaces/arch-namespace/architectures/test-arch/versions/1-0-0',
137-
'deployment'
136+
'test-arch',
137+
'1.0.0'
138138
);
139139
});
140140

‎calm-hub-ui/src/hub/components/diagram-section/DiagramSection.tsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,14 @@ export function DiagramSection({ data, onItemSelect, hasDetailsPanel }: DiagramS
220220
setDecorators([]);
221221
return;
222222
}
223-
const versionPath = data.version.replace(/\./g, '-');
224-
const target = `/calm/namespaces/${data.name}/architectures/${data.id}/versions/${versionPath}`;
225-
calmService.fetchDecoratorValues(data.name, target, 'deployment').then((values) => setDecorators(values as DeploymentDecorator[]));
223+
let cancelled = false;
224+
225+
calmService
226+
.fetchDeploymentDecoratorsForArchitecture(data.name, data.id, data.version)
227+
.then((values) => { if (!cancelled) setDecorators(values as DeploymentDecorator[]); })
228+
.catch(() => { if (!cancelled) setDecorators([]); });
229+
230+
return () => { cancelled = true; };
226231
}, [data, isArchitecture, calmService]);
227232

228233
// When an architecture has an explicit timeline, landing on the latest

‎calm-hub-ui/src/service/calm-service.test.tsx‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,4 +461,64 @@ describe('CalmService', () => {
461461
await expect(calmService.fetchResourceByCustomId(namespace, 'api-gateway', '1.0.0', 'Patterns')).rejects.toThrowError();
462462
});
463463
});
464+
465+
describe('fetchDeploymentDecoratorsForArchitecture', () => {
466+
const decorators = [
467+
{
468+
uniqueId: 'dec-1',
469+
type: 'deployment',
470+
target: ['/api/calm/namespaces/test-namespace/architectures/1/versions/1-0-0'],
471+
appliesTo: ['node-a'],
472+
data: { status: 'completed' },
473+
},
474+
];
475+
476+
it('should fetch decorators using the numeric id directly without calling mappings', async () => {
477+
const target = `/api/calm/namespaces/${namespace}/architectures/1/versions/1-0-0`;
478+
mock.onGet(
479+
`/api/calm/namespaces/${namespace}/decorators/values?target=${encodeURIComponent(target)}&type=deployment`
480+
).reply(200, { values: decorators });
481+
482+
const actual = await calmService.fetchDeploymentDecoratorsForArchitecture(namespace, '1', '1.0.0');
483+
484+
expect(actual).toEqual(decorators);
485+
// Verify no mapping call was made (numeric id needs no resolution)
486+
expect(mock.history.get.filter((r) => r.url?.includes('/calm/namespaces/') && r.url?.includes('/architectures') && !r.url?.includes('/decorators'))).toHaveLength(0);
487+
});
488+
489+
it('should resolve a slug id to its numeric id via mappings before fetching decorators', async () => {
490+
const mappings = [
491+
{ namespace, customId: 'my-arch', resourceType: 'ARCHITECTURE', numericId: 42 },
492+
];
493+
mock.onGet(`/calm/namespaces/${namespace}/architectures`).reply(200, { values: mappings });
494+
495+
const target = `/api/calm/namespaces/${namespace}/architectures/42/versions/1-0-0`;
496+
mock.onGet(
497+
`/api/calm/namespaces/${namespace}/decorators/values?target=${encodeURIComponent(target)}&type=deployment`
498+
).reply(200, { values: decorators });
499+
500+
const actual = await calmService.fetchDeploymentDecoratorsForArchitecture(namespace, 'my-arch', '1.0.0');
501+
502+
expect(actual).toEqual(decorators);
503+
});
504+
505+
it('should return empty array when the slug cannot be resolved in the mappings', async () => {
506+
mock.onGet(`/calm/namespaces/${namespace}/architectures`).reply(200, { values: [] });
507+
508+
const actual = await calmService.fetchDeploymentDecoratorsForArchitecture(namespace, 'unknown-slug', '1.0.0');
509+
510+
expect(actual).toEqual([]);
511+
});
512+
513+
it('should replace dots in the version with hyphens in the target path', async () => {
514+
const target = `/api/calm/namespaces/${namespace}/architectures/5/versions/2-3-1`;
515+
mock.onGet(
516+
`/api/calm/namespaces/${namespace}/decorators/values?target=${encodeURIComponent(target)}&type=deployment`
517+
).reply(200, { values: [] });
518+
519+
const actual = await calmService.fetchDeploymentDecoratorsForArchitecture(namespace, '5', '2.3.1');
520+
521+
expect(actual).toEqual([]);
522+
});
523+
});
464524
});

‎calm-hub-ui/src/service/calm-service.tsx‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { AxiosInstance } from 'axios';
22
import type { CalmTimelineSchema } from '@finos/calm-models/types';
3-
import { Data, ResourceSummary, ResourceMapping } from '../model/calm.js';
3+
import { Data, ResourceSummary, ResourceMapping, isSlug } from '../model/calm.js';
44
import { getAuthHeaders } from '../authService.js';
55
import { Decorator } from '../visualizer/contracts/decorator-contracts.js';
66
import { apiClient } from './utils/api-client.js';
@@ -297,6 +297,30 @@ export class CalmService {
297297
});
298298
}
299299

300+
/**
301+
* Fetches deployment decorators for a given architecture version.
302+
*
303+
* Resolves slug identifiers to their numeric ID via the mapping API before
304+
* building the exact-match target string used for decorator filtering.
305+
* Returns an empty array when the slug cannot be resolved or the fetch fails.
306+
*/
307+
public async fetchDeploymentDecoratorsForArchitecture(
308+
namespace: string,
309+
id: string,
310+
version: string
311+
): Promise<Decorator[]> {
312+
const versionPath = version.replace(/\./g, '-');
313+
let numericId = id;
314+
if (isSlug(id)) {
315+
const mappings = await this.fetchMappings(namespace, 'Architectures');
316+
const match = mappings.find((m) => m.customId === id);
317+
if (!match) return []; // cannot resolve slug — skip decorator fetch
318+
numericId = String(match.numericId);
319+
}
320+
const target = `/api/calm/namespaces/${namespace}/architectures/${numericId}/versions/${versionPath}`;
321+
return this.fetchDecoratorValues(namespace, target, 'deployment');
322+
}
323+
300324
// --- Front Controller API (name-based / slug-based access) ---
301325

302326
/** Maps a calmType (e.g. 'Patterns') or a resource-type enum value (e.g. 'PATTERN')

‎calm-hub-ui/src/visualizer/components/drawer/Drawer.test.tsx‎

Lines changed: 14 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,11 @@ import { Data } from '../../../model/calm.js';
55
import type { ReactFlowVisualizerProps } from '../../contracts/contracts.js';
66
import { DropzoneOptions } from 'react-dropzone';
77

8-
const mockFetchDecoratorValues = vi.fn().mockResolvedValue([]);
9-
const mockFetchMappings = vi.fn().mockResolvedValue([]);
8+
const mockFetchDeploymentDecoratorsForArchitecture = vi.fn().mockResolvedValue([]);
109

1110
vi.mock('../../../service/calm-service.js', () => ({
1211
CalmService: vi.fn().mockImplementation(function () { return {
13-
fetchDecoratorValues: (...args: unknown[]) => mockFetchDecoratorValues(...args),
14-
fetchMappings: (...args: unknown[]) => mockFetchMappings(...args),
12+
fetchDeploymentDecoratorsForArchitecture: (...args: unknown[]) => mockFetchDeploymentDecoratorsForArchitecture(...args),
1513
}; }),
1614
}));
1715

@@ -134,76 +132,37 @@ describe('Drawer', () => {
134132

135133
describe('Drawer — decorator fetching', () => {
136134
beforeEach(() => {
137-
mockFetchDecoratorValues.mockReset();
138-
mockFetchDecoratorValues.mockResolvedValue([]);
139-
mockFetchMappings.mockReset();
140-
mockFetchMappings.mockResolvedValue([]);
135+
mockFetchDeploymentDecoratorsForArchitecture.mockReset();
136+
mockFetchDeploymentDecoratorsForArchitecture.mockResolvedValue([]);
141137
});
142138

143-
it('fetches decorator values using numeric URL when architecture data has a slug ID', async () => {
144-
mockFetchMappings.mockResolvedValue([
145-
{ namespace: 'my-namespace', customId: 'my-arch', resourceType: 'architectures', numericId: 99 }
146-
]);
147-
148-
render(<Drawer data={architectureData} />);
149-
150-
await waitFor(() => {
151-
expect(mockFetchDecoratorValues).toHaveBeenCalledWith(
152-
'my-namespace',
153-
'/api/calm/namespaces/my-namespace/architectures/99/versions/1-0-0',
154-
'deployment'
155-
);
156-
});
157-
});
158-
159-
it('skips decorator fetch when slug cannot be resolved to a numeric ID', async () => {
160-
mockFetchMappings.mockResolvedValue([]); // no mapping found
161-
139+
it('delegates decorator fetching to the shared service method with namespace, id and version', async () => {
162140
render(<Drawer data={architectureData} />);
163141

164-
await new Promise((r) => setTimeout(r, 50));
165-
expect(mockFetchDecoratorValues).not.toHaveBeenCalled();
166-
});
167-
168-
it('fetches decorator values using numeric URL when architecture data has a numeric ID', async () => {
169-
const numericArchData: Data = {
170-
name: 'my-namespace',
171-
calmType: 'Architectures',
172-
id: '42',
173-
version: '1.0.0',
174-
data: { nodes: [], relationships: [] },
175-
};
176-
render(<Drawer data={numericArchData} />);
177-
178142
await waitFor(() => {
179-
expect(mockFetchDecoratorValues).toHaveBeenCalledWith(
143+
expect(mockFetchDeploymentDecoratorsForArchitecture).toHaveBeenCalledWith(
180144
'my-namespace',
181-
'/api/calm/namespaces/my-namespace/architectures/42/versions/1-0-0',
182-
'deployment'
145+
'my-arch',
146+
'1.0.0'
183147
);
184148
});
185149
});
186150

187151
it('does not fetch decorator values when decorators prop is provided', async () => {
188152
render(<Drawer data={architectureData} decorators={[]} />);
189153

190-
await waitFor(() => {
191-
expect(mockFetchDecoratorValues).not.toHaveBeenCalled();
192-
});
154+
await new Promise((r) => setTimeout(r, 50));
155+
expect(mockFetchDeploymentDecoratorsForArchitecture).not.toHaveBeenCalled();
193156
});
194157

195158
it('does not fetch decorator values for pattern data', async () => {
196159
render(<Drawer data={patternData} />);
197160

198-
await waitFor(() => {
199-
expect(mockFetchDecoratorValues).not.toHaveBeenCalled();
200-
});
161+
await new Promise((r) => setTimeout(r, 50));
162+
expect(mockFetchDeploymentDecoratorsForArchitecture).not.toHaveBeenCalled();
201163
});
202164

203165
it('passes fetched decorators to MetadataPanel', async () => {
204-
mockFetchMappings.mockResolvedValue([
205-
{ namespace: 'my-namespace', customId: 'my-arch', resourceType: 'architectures', numericId: 99 }
206-
]);
207166
const decorators = [{
208167
schema: 'https://calm.finos.org/draft/2026-03/standards/deployment/deployment.decorator.standard.json',
209168
uniqueId: 'dec-1',
@@ -216,7 +175,7 @@ describe('Drawer — decorator fetching', () => {
216175
'end-time': '2024-01-01T10:05:00Z',
217176
},
218177
}];
219-
mockFetchDecoratorValues.mockResolvedValue(decorators);
178+
mockFetchDeploymentDecoratorsForArchitecture.mockResolvedValue(decorators);
220179

221180
render(<Drawer data={architectureData} />);
222181

@@ -244,6 +203,6 @@ describe('Drawer — decorator fetching', () => {
244203
await waitFor(() => {
245204
expect(screen.getByTestId('metadata-panel')).toHaveTextContent('decorators:1');
246205
});
247-
expect(mockFetchDecoratorValues).not.toHaveBeenCalled();
206+
expect(mockFetchDeploymentDecoratorsForArchitecture).not.toHaveBeenCalled();
248207
});
249208
});

‎calm-hub-ui/src/visualizer/components/drawer/Drawer.tsx‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { MetadataPanel } from '../reactflow/MetadataPanel.js';
77
import { toSidebarNodeData, toSidebarEdgeData } from '../reactflow/utils/patternClickHandlers.js';
88
import { CalmService } from '../../../service/calm-service.js';
99
import type { DrawerProps, Flow, Control, Decorator } from '../../contracts/contracts.js';
10-
import { isSlug } from '../../../model/calm.js';
1110

1211
/**
1312
* Detect whether JSON data is a CALM pattern (JSON Schema) or an architecture instance.
@@ -67,25 +66,12 @@ export function Drawer({ data, onItemSelect, decorators: decoratorsProp }: Drawe
6766
return;
6867
}
6968
let cancelled = false;
70-
const versionPath = data.version.replace(/\./g, '-');
71-
const namespace = data.name;
72-
const rawId = data.id;
7369

74-
async function fetchDecorators() {
75-
let numericId: string = rawId;
76-
if (isSlug(rawId)) {
77-
const mappings = await calmService.fetchMappings(namespace, 'Architectures');
78-
const match = mappings.find((m) => m.customId === rawId);
79-
if (!match) return; // cannot resolve slug — skip decorator fetch
80-
numericId = String(match.numericId);
81-
}
82-
if (cancelled) return;
83-
const target = `/api/calm/namespaces/${namespace}/architectures/${numericId}/versions/${versionPath}`;
84-
const values = await calmService.fetchDecoratorValues(namespace, target, 'deployment');
85-
if (!cancelled) setDecoratorsState(values);
86-
}
70+
calmService
71+
.fetchDeploymentDecoratorsForArchitecture(data.name, data.id, data.version)
72+
.then((values) => { if (!cancelled) setDecoratorsState(values); })
73+
.catch(() => { if (!cancelled) setDecoratorsState([]); });
8774

88-
fetchDecorators().catch(() => { if (!cancelled) setDecoratorsState([]); });
8975
return () => { cancelled = true; };
9076
}, [data, fileInstance, decoratorsProp, calmService]);
9177

‎calm-hub/src/main/java/org/finos/calm/resources/CalmDocumentParser.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,25 @@ public String extractIdFromJson(String json) {
122122
return null;
123123
}
124124

125+
/**
126+
* Extracts the raw {@code $id} string from a JSON document, throwing
127+
* {@link JsonProcessingException} if the input is not valid JSON.
128+
* Returns {@code null} if the JSON is valid but {@code $id} is absent or non-string.
129+
*
130+
* <p>Unlike {@link #extractIdFromJson}, this method surfaces parse failures so
131+
* callers can distinguish "malformed JSON" from "valid JSON without an {@code $id}".</p>
132+
*/
133+
public String extractIdFromJsonStrict(String json) throws JsonProcessingException {
134+
JsonNode tree = OBJECT_MAPPER.readTree(json);
135+
if (tree.isObject()) {
136+
JsonNode node = tree.get("$id");
137+
if (node != null && !node.isNull() && node.isTextual()) {
138+
return node.asText();
139+
}
140+
}
141+
return null;
142+
}
143+
125144
/**
126145
* Extracts a named string field from a JSON document.
127146
* Returns an empty string if the field is absent or cannot be parsed.

‎calm-hub/src/main/java/org/finos/calm/services/MappingControllerService.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ public Response handleControlRequirementPost(String domain, String controlName,
222222
+ "/controls/" + controlName + "/requirement/versions/" + version;
223223
String actualId;
224224
try {
225-
actualId = documentParser.extractIdFromJson(requestBody);
226-
} catch (Exception e) {
225+
actualId = documentParser.extractIdFromJsonStrict(requestBody);
226+
} catch (JsonProcessingException e) {
227227
return invalidJsonResponse("Cannot parse request body as JSON");
228228
}
229229
if (actualId == null || actualId.isBlank()) {
@@ -288,8 +288,8 @@ public Response handleControlConfigurationPost(String domain, String controlName
288288
+ "/controls/" + controlName + "/configurations/" + configName + "/versions/" + version;
289289
String actualId;
290290
try {
291-
actualId = documentParser.extractIdFromJson(requestBody);
292-
} catch (Exception e) {
291+
actualId = documentParser.extractIdFromJsonStrict(requestBody);
292+
} catch (JsonProcessingException e) {
293293
return invalidJsonResponse("Cannot parse request body as JSON");
294294
}
295295
if (actualId == null || actualId.isBlank()) {

0 commit comments

Comments
 (0)