Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ export const PROCESS_DEFINITION_ID = '@Process.DefinitionId' as const;

export const LOG_MESSAGES = {
PROCESS_NOT_STARTED: 'Not starting process as start condition(s) are not met.',
NO_PROCESS_INPUTS_DEFINED:
'No process start input annotations defined, fetching entire entity row for process start context.',
PROCESS_INPUTS_FROM_DEFINITION:
'No inputs annotation defined. Filtering entity fields by process definition inputs.',
PROCESS_NOT_SUSPENDED: 'Not suspending process as suspend condition(s) are not met.',
PROCESS_NOT_RESUMED: 'Not resuming process as resume condition(s) are not met.',
PROCESS_NOT_CANCELLED: 'Not canceling process as cancel condition(s) are not met.',
Expand Down
60 changes: 55 additions & 5 deletions lib/handlers/processStart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
PROCESS_START_INPUTS,
LOG_MESSAGES,
PROCESS_LOGGER_PREFIX,
PROCESS_PREFIX,
BUSINESS_KEY,
BUSINESS_KEY_MAX_LENGTH,
} from './../constants';
Expand Down Expand Up @@ -48,8 +49,13 @@ export function getColumnsForProcessStart(target: Target): (column_expr | string
const startSpecs = initStartSpecs(target);
startSpecs.inputs = parseInputToTree(target);
if (startSpecs.inputs.length === 0) {
LOG.debug(LOG_MESSAGES.NO_PROCESS_INPUTS_DEFINED);
return ['*'];
LOG.debug(LOG_MESSAGES.PROCESS_INPUTS_FROM_DEFINITION);

if (!startSpecs.id) {
LOG.warn('No process definition id found on target, falling back to wildcard.');
return [WILDCARD];
}
return resolveColumnsFromProcessDefinition(startSpecs.id, target);
} else {
return convertToColumnsExpr(startSpecs.inputs);
}
Expand All @@ -66,11 +72,16 @@ export async function handleProcessStart(req: cds.Request, data: EntityRow): Pro
const startSpecs = initStartSpecs(target);
startSpecs.inputs = parseInputToTree(target);

// if startSpecs.input = [] --> no input defined, fetch entire row
// if startSpecs.input = [] --> no input annotation defined, resolve from process definition
let columns: (column_expr | string)[];
if (startSpecs.inputs.length === 0) {
columns = [WILDCARD];
LOG.debug(LOG_MESSAGES.NO_PROCESS_INPUTS_DEFINED);
if (!startSpecs.id) {
LOG.warn('No process definition id found on target, falling back to wildcard.');
columns = [WILDCARD];
} else {
columns = resolveColumnsFromProcessDefinition(startSpecs.id, target);
}
LOG.debug(LOG_MESSAGES.PROCESS_INPUTS_FROM_DEFINITION);
} else {
columns = convertToColumnsExpr(startSpecs.inputs);
}
Expand Down Expand Up @@ -188,6 +199,45 @@ function parseInputToTree(target: Target): ProcessStartInput[] {
return buildInputTree(parsedEntries, runtimeContext);
}

function getProcessInputFieldNames(definitionId: string): string[] | undefined {
const definitions = cds.model?.definitions;
if (!definitions) return undefined;
let serviceName: string | undefined;
for (const name in definitions) {
if (Object.hasOwn(definitions, name)) {
const def = definitions[name] as unknown as Record<string, unknown>;
if (def[PROCESS_PREFIX] === definitionId) {
serviceName = name;
break;
}
}
}

if (!serviceName) return undefined;
Comment on lines +205 to +216
Copy link
Contributor

Choose a reason for hiding this comment

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

Logic Error: Non-unique definitionId lookup may return the wrong service's ProcessInputs

getProcessInputFieldNames iterates cds.model.definitions and stops at the first entry where def[PROCESS_PREFIX] === definitionId. The same definitionId string can be shared across multiple entities with different @bpm.process.start.id values (e.g. annotation-service.cds has both StartNoInput and StartNoInputZeroMatch annotated with id: 'startNoInputProcess'). More critically, the loop is looking for the service-level @bpm.process annotation, but two independent services could theoretically carry the same process id, or the iteration order of definitions could pick the wrong one first.

Additionally, the lookup associates the service name with the definition id, then constructs ${serviceName}.ProcessInputs. If two services share a definition id, the resolution is non-deterministic and the wrong ProcessInputs type may be used, leading to silently wrong column filtering.

Should either enforce that definition-id uniqueness is assumed at a higher level (and add a warning when duplicates are found), or re-think the lookup so it doesn't silently use the first match when duplicates exist.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful


const processInputsType = definitions[`${serviceName}.ProcessInputs`] as
| { elements?: Record<string, unknown> }
| undefined;

if (!processInputsType?.elements) return undefined;

return Object.keys(processInputsType.elements);
}

function resolveColumnsFromProcessDefinition(
definitionId: string,
target: Target,
): (column_expr | string)[] {
const processFields = getProcessInputFieldNames(definitionId);
if (processFields) {
const entityElements = Object.keys((target as cds.entity).elements ?? {});
const matchingFields = processFields.filter((f) => entityElements.includes(f));
return matchingFields;
}

return [WILDCARD];
}

function convertToColumnsExpr(array: ProcessStartInput[]): (column_expr | string)[] {
const result: (column_expr | string)[] = [];

Expand Down
4 changes: 4 additions & 0 deletions lib/handlers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ async function fetchEntity(
results = {};
}

if (columns.length === 0) {
return {};
}

const keyFields = getKeyFieldsForEntity(request.target as cds.entity);

// build where clause
Expand Down
8 changes: 8 additions & 0 deletions tests/bookshop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@
"eu12.bpm-horizon-walkme.sdshipmentprocessor.ShipmentHandlerService": {
"kind": "process-service",
"model": "srv/external/eu12.bpm-horizon-walkme.sdshipmentprocessor.shipmentHandler"
},
"StartNoInputProcessService": {
"kind": "process-service",
"model": "srv/external/startNoInputProcess"
},
"StartNoInputWithAssocProcessService": {
"kind": "process-service",
"model": "srv/external/startNoInputWithAssocProcess"
}
}
},
Expand Down
23 changes: 19 additions & 4 deletions tests/bookshop/srv/annotation-service.cds
Original file line number Diff line number Diff line change
Expand Up @@ -890,8 +890,8 @@ service AnnotationService {
// ============================================

// --------------------------------------------
// Test 1: No inputs specified
// All entity fields should be included in context
// Test 1: No inputs specified, but ProcessInputs type exists
// Only entity fields matching ProcessInputs should be included
// --------------------------------------------
@bpm.process.start: {
id: 'startNoInputProcess',
Expand Down Expand Up @@ -1440,8 +1440,8 @@ service AnnotationService {
key ID : UUID;
}

// Test 14: No inputs (all fields including Composition and Association)
// Without inputs array, all fields should be included
// Test 14: No inputs with Composition and Association
// ProcessInputs type matches all scalar fields, so all should be included
// --------------------------------------------
@bpm.process.start: {
id: 'startNoInputWithAssocProcess',
Expand Down Expand Up @@ -1488,6 +1488,21 @@ service AnnotationService {
name : String(100);
}

// Test 16: No inputs, ProcessInputs exists but zero entity fields match
// Should send empty context {}
// --------------------------------------------
@bpm.process.start: {
id: 'startNoInputProcess',
on: 'CREATE'
}
entity StartNoInputZeroMatch {
key ID : UUID;
shipmentDate : Date;
expectedDelivery : Date;
totalValue : Decimal(15, 2);
notes : String(1000);
}

// ============================================
// BUSINESS KEY LENGTH VALIDATION TESTS
// Testing businessKey max length (255 chars) on processStart
Expand Down
35 changes: 35 additions & 0 deletions tests/bookshop/srv/external/startNoInputProcess.cds
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* checksum : test-fixture-start-no-input */

/** Test fixture: Process definition for startNoInputProcess.
* Used to test that when no @bpm.process.start.inputs annotation is defined,
* only entity fields matching ProcessInputs element names are sent. */
@protocol : 'none'
@bpm.process : 'startNoInputProcess'
service StartNoInputProcessService {

type ProcessInputs {
status : String;
origin : String;
};

type ProcessOutputs {};

action start(
inputs : ProcessInputs not null
);

action suspend(
businessKey : String not null,
cascade : Boolean
);

action resume(
businessKey : String not null,
cascade : Boolean
);

action cancel(
businessKey : String not null,
cascade : Boolean
);
};
36 changes: 36 additions & 0 deletions tests/bookshop/srv/external/startNoInputWithAssocProcess.cds
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/* checksum : test-fixture-start-no-input-with-assoc */

/** Test fixture: Process definition for startNoInputWithAssocProcess.
* Used to test that when no @bpm.process.start.inputs annotation is defined,
* only entity fields matching ProcessInputs element names are sent. */
@protocol : 'none'
@bpm.process : 'startNoInputWithAssocProcess'
service StartNoInputWithAssocProcessService {

type ProcessInputs {
ID : UUID;
status : String;
author_ID : UUID;
};

type ProcessOutputs {};

action start(
inputs : ProcessInputs not null
);

action suspend(
businessKey : String not null,
cascade : Boolean
);

action resume(
businessKey : String not null,
cascade : Boolean
);

action cancel(
businessKey : String not null,
cascade : Boolean
);
};
49 changes: 40 additions & 9 deletions tests/integration/annotations/processStart-input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ describe('Integration tests for START annotation with inputs array', () => {
};

// ================================================
// Test 1: No inputs array specified
// All entity fields should be included in context
// Test 1: No inputs array specified, but ProcessInputs type exists
// Only entity fields matching ProcessInputs should be included
// ================================================
describe('Test 1: No inputs array (all fields included)', () => {
it('should include all entity fields in process context', async () => {
describe('Test 1: No inputs array (filtered by ProcessInputs)', () => {
it('should include only entity fields matching ProcessInputs element names', async () => {
const shipment = {
ID: '550e8400-e29b-41d4-a716-446655440000',
status: 'PENDING',
Expand All @@ -54,8 +54,11 @@ describe('Integration tests for START annotation with inputs array', () => {
const context = getStartContext();
expect(context).toBeDefined();

// All fields should be present
expect(context).toEqual({ ...shipment });
// Only fields matching ProcessInputs (status, origin) should be present
expect(context).toEqual({
status: shipment.status,
origin: shipment.origin,
});
});
});

Expand Down Expand Up @@ -604,10 +607,10 @@ describe('Integration tests for START annotation with inputs array', () => {

// ================================================
// Test 14: No inputs with Composition and Association
// Without inputs array, all fields should be included
// ProcessInputs type matches all scalar fields, so all should be included
// ================================================
describe('Test 14: No inputs with Composition and Association', () => {
it('should include all fields including compositions and associations', async () => {
it('should include all scalar fields matching ProcessInputs', async () => {
const author = {
ID: '550e8400-e29b-41d4-a716-446655440099',
};
Expand All @@ -629,7 +632,7 @@ describe('Integration tests for START annotation with inputs array', () => {
const context = getStartContext();
expect(context).toBeDefined();

// No inputs specified - should include all fields
// ProcessInputs has {ID, status, author_ID} — all match entity scalar fields
expect(context).toEqual({
ID: entity.ID,
status: entity.status,
Expand Down Expand Up @@ -680,4 +683,32 @@ describe('Integration tests for START annotation with inputs array', () => {
});
});
});

// ================================================
// Test 16: No inputs, ProcessInputs exists but zero entity fields match
// Should send empty context {}
// ================================================
describe('Test 16: No inputs, ProcessInputs exists but zero fields match', () => {
it('should send empty context when no entity fields match ProcessInputs', async () => {
const entity = {
ID: '550e8400-e29b-41d4-a716-44665544ff01',
shipmentDate: '2026-01-15',
expectedDelivery: '2026-01-25',
totalValue: 2500.0,
notes: 'Handle with care',
};

const response = await POST('/odata/v4/annotation/StartNoInputZeroMatch', entity);

expect(response.status).toBe(201);
expect(foundMessages.length).toBe(1);

const context = getStartContext();
expect(context).toBeDefined();

// ProcessInputs has {status, origin} but entity has {ID, shipmentDate, expectedDelivery, totalValue, notes}
// No field names match, so context should be empty
expect(context).toEqual({});
});
});
});
Loading