-
Notifications
You must be signed in to change notification settings - Fork 235
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
Feat/776 circuit inputs parser #778
Changes from 11 commits
e11a0ed
2da298b
cd0b612
e886d95
290cf66
652d07c
d3366cd
ef44d4c
d6cfdac
8b81617
29fba71
5762fc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,259 @@ | ||
import { DeepMockProxy, mockDeep } from 'jest-mock-extended'; | ||
import { LoggingService } from '../../../../../shared/logging/logging.service'; | ||
import { CircuitInputsParserService } from './circuitInputParser.service'; | ||
|
||
let cips: CircuitInputsParserService; | ||
const loggingServiceMock: DeepMockProxy<LoggingService> = | ||
mockDeep<LoggingService>(); | ||
|
||
beforeAll(async () => { | ||
cips = new CircuitInputsParserService(loggingServiceMock); | ||
}); | ||
|
||
describe('CircuitInputsParserService', () => { | ||
it('Should return null if empty CircuitInputsMapping passed in', () => { | ||
// Arrange | ||
const payload = 'test'; | ||
const schema = {} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toBeNull(); | ||
}); | ||
|
||
it('Should generate a single circuit input param with charcode sum based on a single string param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"supplierInvoiceID": "INV123" | ||
}`; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceID', | ||
dataType: 'string', | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toStrictEqual({ dascircuitinput: 387 }); | ||
}); | ||
|
||
it('Should generate a single circuit input param with charcode sum based on a default value for a missing string param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"somethingElse": "" | ||
}`; | ||
|
||
const defaultParamValue = '555333'; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceID', | ||
dataType: 'string', | ||
defaultValue: defaultParamValue, | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toStrictEqual({ dascircuitinput: 312 }); | ||
}); | ||
|
||
it('Should return null based on a missing default value for a missing string param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"somethingElse": "" | ||
}`; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceID', | ||
dataType: 'string', | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toBeNull(); | ||
}); | ||
|
||
it('Should generate a single circuit input param with charcode sum based on a single string param at root + 1 level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"supplierInvoice": { | ||
"ID": "INV123" | ||
} | ||
}`; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoice.ID', | ||
dataType: 'string', | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toStrictEqual({ dascircuitinput: 387 }); | ||
}); | ||
|
||
it('Should generate a single circuit input param based on a single integer param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"supplierInvoiceID": 123 | ||
}`; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceID', | ||
dataType: 'integer', | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toStrictEqual({ dascircuitinput: 123 }); | ||
}); | ||
|
||
it('Should generate a single circuit input param based on a default value for a missing integer param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"somethingElse": "" | ||
}`; | ||
|
||
const defaultValue = 555333; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceID', | ||
dataType: 'integer', | ||
defaultValue: defaultValue, | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toStrictEqual({ dascircuitinput: defaultValue }); | ||
}); | ||
|
||
it('Should return null based on a missing default value for a missing integer param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"somethingElse": "" | ||
}`; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceID', | ||
dataType: 'integer', | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toBeNull(); | ||
}); | ||
|
||
it('Should generate a single circuit input param based on a single integer array param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"supplierInvoiceIDs": [ | ||
123, | ||
321, | ||
454 | ||
] | ||
}`; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceIDs', | ||
dataType: 'array', | ||
arrayType: 'integer', | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toStrictEqual({ dascircuitinput: [123, 321, 454] }); | ||
}); | ||
|
||
it('Should generate a single circuit input param with charcode sums based on a single string array param at root level', () => { | ||
// Arrange | ||
const payload = `{ | ||
"supplierInvoiceIDs": [ | ||
"INV123", | ||
"INV124", | ||
"INV125" | ||
] | ||
}`; | ||
|
||
const schema = { | ||
mapping: [ | ||
{ | ||
circuitInput: 'dascircuitinput', | ||
description: 'desc', | ||
payloadJsonPath: 'supplierInvoiceIDs', | ||
dataType: 'array', | ||
arrayType: 'string', | ||
} as CircuitInputMapping, | ||
], | ||
} as CircuitInputsMapping; | ||
|
||
// Act | ||
const circuitInputs = cips.applyMappingToJSONPayload(payload, schema); | ||
|
||
// Assert | ||
expect(circuitInputs).toStrictEqual({ dascircuitinput: [387, 388, 389] }); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
import { Injectable } from '@nestjs/common'; | ||
import { LoggingService } from '../../../../../shared/logging/logging.service'; | ||
|
||
@Injectable() | ||
export class CircuitInputsParserService { | ||
constructor(private readonly logger: LoggingService) {} | ||
|
||
public applyMappingToJSONPayload(payload: string, cim: CircuitInputsMapping) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ognjenkurtic don't we just need a json schema validator for typscript. Why not use something like this https://github.com/ThomasAribart/json-schema-to-ts That way we just need to have a json schema file for the circuit stored in a path and then can validate the provided input against that. Is that not easier than explicity writing doen all the types etc.? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Therecanbeonlyone1969 The lib above is written for a different use-case: solving code duplication when you do static and runtime type checking. From their readme: Our use-case is different than that. We are not only validating that the tx payload is of a specific type, we are also applying specific business rules (translations) based on the type definitions for specific circuit inputs. Introduction of a lib like this would not help as we would still need to act based on the type of the property in the payload. I would vote we start with what i did. We have a set of unit tests we ll continue to extend, so we ll have good infra in place to refactor if\whenever the need arises. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ognjenkurtic I do not like what you did. How about we use this? ... https://github.com/grantila/suretype ... looks easy to implement and very flexible when the user provides the schema. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreement is not to use the library per the last SRI sync call. Will address other comments if any and submit for rereview |
||
const result: any = {}; | ||
|
||
try { | ||
const jsonPayload = JSON.parse(payload); | ||
|
||
for (let mapping of cim.mapping) { | ||
const value = this.getJsonValueByPath( | ||
jsonPayload, | ||
mapping.payloadJsonPath, | ||
); | ||
|
||
if (!value && !mapping.defaultValue) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we check value === undefined, if value can be empty string? since we are checking ===undefined in method itself |
||
this.logger.logError( | ||
`Missing value and default value for mapping ${cim.mapping} while mapping circuit inputs for payload ${payload}`, | ||
); | ||
return null; | ||
} | ||
|
||
switch (mapping.dataType) { | ||
case 'string': | ||
result[mapping.circuitInput] = value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this be simplified, like result... = this.calc(value || default) |
||
? this.calculateStringCharCodeSum(value) | ||
: this.calculateStringCharCodeSum(mapping.defaultValue); | ||
break; | ||
|
||
case 'integer': | ||
result[mapping.circuitInput] = value ?? mapping.defaultValue; | ||
break; | ||
|
||
case 'array': | ||
if (mapping.arrayType === 'string') { | ||
result[mapping.circuitInput] = value | ||
? value.map((val) => this.calculateStringCharCodeSum(val)) | ||
: mapping.defaultValue.map((val) => | ||
this.calculateStringCharCodeSum(val), | ||
); | ||
} | ||
|
||
if (mapping.arrayType === 'integer') { | ||
result[mapping.circuitInput] = value ?? mapping.defaultValue; | ||
} | ||
|
||
if (mapping.arrayType === 'object') { | ||
// TODO | ||
} | ||
break; | ||
default: | ||
this.logger.logError( | ||
`Unknown datatype '${mapping.dataType}' while mapping circuit inputs for payload ${payload}`, | ||
); | ||
return null; | ||
} | ||
} | ||
} catch (error) { | ||
this.logger.logError( | ||
`Error '${error}' while mapping circuit inputs for payload ${payload}`, | ||
); | ||
return null; | ||
} | ||
|
||
return result; | ||
} | ||
|
||
private getJsonValueByPath(json: any, path: string) { | ||
const parts = path.split('.'); | ||
let currentValue = json; | ||
|
||
for (const part of parts) { | ||
if (currentValue[part] === undefined) { | ||
return undefined; | ||
} | ||
currentValue = currentValue[part]; | ||
} | ||
|
||
return currentValue; | ||
} | ||
|
||
private calculateStringCharCodeSum(text: string): number { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe this method can be more readable with usage of but not super important There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i like the good old for loop here and there |
||
let sum = 0; | ||
|
||
for (let i = 0; i < text.length; i++) { | ||
sum += text.charCodeAt(i); | ||
} | ||
|
||
return sum; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
type CircuitInputsMapping = { | ||
mapping: CircuitInputMapping[]; | ||
}; | ||
|
||
type CircuitInputMapping = { | ||
circuitInput: string; | ||
description: string; | ||
payloadJsonPath: string; | ||
dataType: string; | ||
defaultValue?: any; | ||
arrayType?: string; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need both array and arrayType? can arrayType be calculated using
typeof array[0]
? cantypeof value
be used in other places too to simplify mapping logic and remove some fields here?i feel more validation of this object is needed, for example if dataType is not array, we should not specifiy arrayType and so on, but it seems like some simplification can be done first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want the participants to strictly define the types that are expected in the payload, so that we can do a type check before we even try to invoke the circuit. there is still definitely room for improvement, both for simplification and additional validation, but let's do it in small iterations as we move forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree. the
circuitInput
for example will be an object that has a schema itself depending on the type of input parameters.