Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix return null value for expression type null #2091

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

fix return null value for expression type null #2091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d69d904
fix return null value for expression type null
Jimmy-Joseph19 Jun 28, 2024
3866638
fix: feedback
Jimmy-Joseph19 Jul 1, 2024
15c24b6
fix: unit test
Jimmy-Joseph19 Jul 1, 2024
bf9a3ae
sonar maintainability issue
Jimmy-Joseph19 Jul 1, 2024
0ce8f3c
Merge remote-tracking branch 'origin/main' into fix/returnNullValueFo…
Jimmy-Joseph19 Jul 1, 2024
4175579
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 2, 2024
c0a8897
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 3, 2024
09fa980
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 4, 2024
e1efbf6
Merge remote-tracking branch 'origin/main' into fix/returnNullValueFo…
Jimmy-Joseph19 Jul 5, 2024
200d82b
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 8, 2024
092e041
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 8, 2024
385b071
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 9, 2024
a5d096f
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 9, 2024
e833daa
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 9, 2024
be7f108
Merge branch 'main' into fix/returnNullValueForTypeNull
voicis Jul 10, 2024
6e1d346
Merge branch 'main' into fix/returnNullValueForTypeNull
voicis Jul 10, 2024
39f8470
Merge branch 'main' into fix/returnNullValueForTypeNull
Jimmy-Joseph19 Jul 18, 2024
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
5 changes: 5 additions & 0 deletions .changeset/two-lions-explode.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sap-ux/fiori-annotation-api': patch
---

fix: return null value for expression type null
22 changes: 14 additions & 8 deletions packages/fiori-annotation-api/src/change-converter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,14 +580,7 @@ export class ChangeConverter {
};
this.annotationFileChanges.push(internalChange);
} else if (onlyChangeValue && node.name === valueType) {
// element notation
const internalChange: ReplaceElementContent = {
type: REPLACE_ELEMENT_CONTENT,
uri: file.uri,
pointer: pointer,
newValue: [createTextNode(newValue)]
};
this.annotationFileChanges.push(internalChange);
this.addReplacementContent(node, file.uri, pointer, newValue);
} else if (node.attributes[type]) {
// attribute notation
this.annotationFileChanges.push({
Expand Down Expand Up @@ -618,6 +611,19 @@ export class ChangeConverter {
this.convertUpdateExpressionForAttrributeType(file.uri, targetName, valueType, content, pointer, newValue);
}
}
private addReplacementContent(node: Element, fileUri: string, pointer: string, newValue: string): void {
// skip updating value for node name that are already null.
if (node.name !== Edm.Null) {
// element notation
const internalChange: ReplaceElementContent = {
type: REPLACE_ELEMENT_CONTENT,
uri: fileUri,
pointer: pointer,
newValue: [createTextNode(newValue)]
};
this.annotationFileChanges.push(internalChange);
}
}

private convertUpdateExpressionForAttrributeType(
fileUri: string,
Expand Down
158 changes: 109 additions & 49 deletions packages/fiori-annotation-api/test/unit/fiori-service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,41 +87,49 @@ interface EditTestCase<T extends Record<string, string>> {
getChanges: (files: T) => Change[];
fsEditor?: Editor;
log?: boolean;
expectUpdate?: boolean;
}

interface CustomTest<T extends Record<string, string>> {
(testCase: EditTestCase<T>, timeout?: number): void;
only: CustomTest<T>;
skip: CustomTest<T>;
}

const createEditTestCase = (<T extends Record<string, string>>(): CustomTest<T> => {
const run = (testCase: EditTestCase<T>, timeout?: number) => () => {
for (const model of testCase.projectTestModels) {
const { info, root, files, serviceName } = model;
const name = `${info.type} ${info.name} ${info.version}`;
if (SKIP_TARGETS.has(model)) {
test.skip(name, () => {});
} else {
test(
name,
async () => {
const text = await testEdit(
root,
testCase.getInitialChanges ? testCase.getInitialChanges(files) : [],
testCase.getChanges(files),
serviceName,
testCase.fsEditor,
testCase.log
);

expect(text).toMatchSnapshot();
},
timeout
);
const run =
({ expectUpdate = true, ...testCase }: EditTestCase<T>, timeout?: number) =>
() => {
for (const model of testCase.projectTestModels) {
const { info, root, files, serviceName } = model;
const name = `${info.type} ${info.name} ${info.version}`;
if (SKIP_TARGETS.has(model)) {
test.skip(name, () => {});
} else {
test(
name,
async () => {
const { changedText, totalChangedFiles } = await testEdit(
root,
testCase.getInitialChanges ? testCase.getInitialChanges(files) : [],
testCase.getChanges(files),
serviceName,
testCase.fsEditor,
testCase.log
);
if (totalChangedFiles > 0) {
expect(changedText).toMatchSnapshot();
}

if (!expectUpdate) {
expect(changedText).toBe('');
expect(totalChangedFiles).toEqual(0);
}
},
timeout
);
}
}
}
};
};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const runner: any = (testCase: EditTestCase<T>, timeout?: number): void => {
describe(testCase.name, run(testCase, timeout));
Expand All @@ -142,7 +150,7 @@ async function testEdit(
serviceName: string,
fsEditor?: Editor,
log?: boolean
): Promise<string> {
): Promise<{ changedText: string; totalChangedFiles: number }> {
const editor = fsEditor ?? (await createFsEditorForProject(root));
const project = await getProject(root);
const service = await FioriAnnotationService.createService(project, serviceName, '', editor, {
Expand All @@ -163,23 +171,30 @@ async function testEdit(
}

const changedFileUris = applyChanges(service, changes);
await service.save();

for (const uri of changedFileUris.values()) {
const path = pathFromUri(uri);
const original = await promises.readFile(path, { encoding: 'utf-8' });
const afterInitialChanges = initialChangeCache.get(uri);
const textAfterEdit = editor.read(path);
if (log) {
console.log('Original: \n', original);
if (afterInitialChanges !== undefined) {
console.log('After initial changes: \n', afterInitialChanges);
const changedFile = await service.save();

if (changedFile.files > 0) {
for (const uri of changedFileUris.values()) {
const path = pathFromUri(uri);
const original = await promises.readFile(path, { encoding: 'utf-8' });
const afterInitialChanges = initialChangeCache.get(uri);
const textAfterEdit = editor.read(path);
if (log) {
console.log('Original: \n', original);
if (afterInitialChanges !== undefined) {
console.log('After initial changes: \n', afterInitialChanges);
}
console.log('After test changes: \n', textAfterEdit);
}
console.log('After test changes: \n', textAfterEdit);
return (
{ changedText: textAfterEdit, totalChangedFiles: changedFile.files } ?? {
changedText: '',
totalChangedFiles: changedFile.files
}
);
}
return textAfterEdit ?? '';
}
return '';
return { changedText: '', totalChangedFiles: changedFile.files };
}

function applyChanges(service: FioriAnnotationService, changes: Change[]): Set<string> {
Expand Down Expand Up @@ -769,7 +784,7 @@ describe('fiori annotation service', () => {
`
);
fsEditor.write(path, updatedSchemaFile);
const text = await testEdit(
const { changedText: text } = await testEdit(
root,
[],
[
Expand Down Expand Up @@ -1357,6 +1372,51 @@ describe('fiori annotation service', () => {
}
]
});
createEditTestCase({
name: 'no update expected when value is already null',
projectTestModels: TEST_TARGETS,
getInitialChanges: (files) => [
{
kind: ChangeType.InsertAnnotation,
uri: files.annotations,
content: {
type: 'annotation',
target: targetName,
value: {
term: DATA_POINT,
record: {
propertyValues: [
{
name: 'Value',
value: {
type: 'Null'
}
}
]
}
}
}
}
],
getChanges: (files) => [
{
kind: ChangeType.Update,
reference: {
target: targetName,
term: DATA_POINT
},
uri: files.annotations,
pointer: 'record/propertyValues/0/value',
content: {
type: 'expression',
value: {
type: 'Null'
}
}
}
],
expectUpdate: false
});
createEditTestCase({
name: "update propertyValue value 'string' value",
projectTestModels: TEST_TARGETS,
Expand Down Expand Up @@ -2000,7 +2060,7 @@ describe('fiori annotation service', () => {
};
`;
fsEditor.write(path, testData);
const text = await testEdit(
const { changedText: text } = await testEdit(
root,
[],
[
Expand Down Expand Up @@ -3024,7 +3084,7 @@ describe('fiori annotation service', () => {
}
};`;
fsEditor.write(mdPath, mdTestData);
const text = await testEdit(
const { changedText: text } = await testEdit(
root,
[],
[
Expand Down Expand Up @@ -3213,7 +3273,7 @@ describe('fiori annotation service', () => {
},
);`;
fsEditor.write(path, testData);
const text = await testEdit(
const { changedText: text } = await testEdit(
root,
[],
[
Expand Down Expand Up @@ -3262,7 +3322,7 @@ describe('fiori annotation service', () => {
}
);`;
fsEditor.write(path, testData);
const text = await testEdit(
const { changedText } = await testEdit(
root,
[],
[
Expand Down Expand Up @@ -3299,7 +3359,7 @@ describe('fiori annotation service', () => {
false
);

expect(text).toMatchSnapshot();
expect(changedText).toMatchSnapshot();
});

test('delete common text and textArrangement', async () => {
Expand Down Expand Up @@ -3366,7 +3426,7 @@ describe('fiori annotation service', () => {
// UI.KPI.DataPoint.Responsible.nickname: record/0/record/1/record/0
// UI.KPI.DataPoint.Responsible.fn: record/0/record/1/record/1
fsEditor.write(path, testData);
const text = await testEdit(
const { changedText } = await testEdit(
root,
[],
[
Expand All @@ -3385,7 +3445,7 @@ describe('fiori annotation service', () => {
false
);

expect(text).toMatchSnapshot();
expect(changedText).toMatchSnapshot();
});
});
});
Expand Down
Loading