Skip to content

Commit

Permalink
#353 correctly instantiate boolean properties and improve spectral ru…
Browse files Browse the repository at this point in the history
…les (#500)

* #353 properly instantiate boolean properties

* #353 add spectral rule to report boolean placeholder values

* #353 add pattern rule to warn against using booleans

---------

Co-authored-by: Matthew Bain <[email protected]>
  • Loading branch information
willosborne and rocketstack-matt authored Oct 18, 2024
1 parent e149101 commit 82d375b
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 1 deletion.
4 changes: 4 additions & 0 deletions cli/src/commands/generate/components/metadata.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ describe('instantiateMetadataObject', () => {
'integer-prop': {
'type': 'integer'
},
'boolean-prop': {
'type': 'boolean'
},
'const-prop': {
'const': 'constant'
}
Expand All @@ -42,6 +45,7 @@ describe('instantiateMetadataObject', () => {
{
'string-prop': '{{ STRING_PROP }}',
'integer-prop': -1,
'boolean-prop': '{{ BOOLEAN_BOOLEAN_PROP }}',
'const-prop': 'constant'
},
);
Expand Down
15 changes: 15 additions & 0 deletions cli/src/commands/generate/components/node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,21 @@ describe('instantiateNodes', () => {
]);
});

it('return instantiated node with boolean property', () => {
const pattern = getSamplePatternNode({
'property-name': {
'type': 'boolean'
}
});

expect(instantiateNodes(pattern, mockSchemaDir, false, true))
.toEqual([
{
'property-name': '{{ BOOLEAN_PROPERTY_NAME }}'
}
]);
});

it('only instantiate required properties when instantiateAll set to false', () => {
const pattern = getSamplePatternNode({
'property-name': {
Expand Down
7 changes: 7 additions & 0 deletions cli/src/commands/generate/components/property.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,11 @@ describe('getPropertyValue', () => {
}))
.toBe('{{ REF_KEY_NAME }}');
});

it('generates boolean placeholder from variable', () => {
expect(getPropertyValue('key-name', {
'type': 'boolean'
}))
.toBe('{{ BOOLEAN_KEY_NAME }}');
});
});
9 changes: 8 additions & 1 deletion cli/src/commands/generate/components/property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ export function getRefPlaceholder(name: string): string {
return '{{ REF_' + name.toUpperCase().replaceAll('-', '_') + ' }}';
}

export function getBooleanPlaceholder(name: string): string {
return '{{ BOOLEAN_' + name.toUpperCase().replaceAll('-', '_') + ' }}';
}

interface Detail {
const?: string | object,
type?: 'string' | 'integer' | 'number' | 'array',
type?: 'string' | 'integer' | 'number' | 'array' | 'boolean',
$ref?: string
}

Expand All @@ -26,6 +30,9 @@ export function getPropertyValue(keyName: string, detail: Detail): string | stri
if (propertyType === 'string') {
return getStringPlaceholder(keyName);
}
if (propertyType === 'boolean') {
return getBooleanPlaceholder(keyName);
}
if (propertyType === 'integer' || propertyType === 'number') {
return -1;
}
Expand Down
15 changes: 15 additions & 0 deletions cli/src/commands/generate/components/relationship.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,21 @@ describe('instantiateRelationships', () => {
}
]);
});

it('return instantiated relationship with boolean property', () => {
const pattern = getSamplePatternRelationship({
'property-name': {
type: 'boolean'
}
});

expect(instantiateRelationships(pattern, mockSchemaDir, false, true))
.toEqual([
{
'property-name': '{{ BOOLEAN_PROPERTY_NAME }}'
}
]);
});

it('return instantiated relationship with const property', () => {
const pattern = getSamplePatternRelationship({
Expand Down
10 changes: 10 additions & 0 deletions spectral/instantiation/validation-rules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ rules:
function: pattern
functionOptions:
notMatch: "^{{\\s*[A-Z_]+\\s*}}$"

instantiation-has-no-placeholder-properties-boolean:
description: Should not contain placeholder values with pattern {{ BOOLEAN_[property name] }}
message: Boolean placeholder detected in instantiated pattern.
severity: warn
given: "$..*"
then:
function: pattern
functionOptions:
notMatch: "^{{\\s*BOOLEAN_[A-Z_]+\\s*}}$"

relationship-references-existing-nodes-in-instantiation:
description: Relationships must reference existing nodes
Expand Down
10 changes: 10 additions & 0 deletions spectral/pattern/validation-rules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ rules:
# only container and actor are in scope, since source/destination use interfaces now
function: node-id-exists

avoid-boolean-properties:
description: Boolean property detected. Booleans should ideally be avoided as they are closed for extension; have you considered using an enum instead?
severity: warn
message: Boolean property detected. Booleans should ideally be avoided as they are closed for extension; have you considered using an enum instead?
given: "$..type"
then:
function: pattern
functionOptions:
notMatch: /boolean/

relationship-references-existing-nodes-in-pattern:
description: Relationships with const properties must reference existing nodes
severity: error
Expand Down

0 comments on commit 82d375b

Please sign in to comment.