Skip to content

Commit 11f823a

Browse files
tobiasso85tobiasso85
authored
[FIX] manifestCreator: 'embeds' should list all components (#575)
Field embeds in created manifest.json file now contains all nested manifests, independent from the reverse field embeddedBy (embeddedBy points from an embedded manifest to the library's manifest). Remove unreachable code (in createSapUI5() -> getUI5Version() function) . Use path.dirname method to get directory name in findEmbeddedComponents() function. Use string for version value in manifest (in sectionVersion() function). e.g. "1.2.3" instead of the semver object's string representation. It reverts changes which were introduced with #555 Successor of #554
1 parent 0fc364d commit 11f823a

File tree

2 files changed

+68
-94
lines changed

2 files changed

+68
-94
lines changed

‎lib/processors/manifestCreator.js

Lines changed: 11 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -177,75 +177,31 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
177177
function sectionVersion(candidateVersion) {
178178
// _version property for nested sections became optional with AppDescriptor V5
179179
if ( descriptorVersion.compare(APP_DESCRIPTOR_V5) < 0 ) {
180-
return candidateVersion;
180+
return candidateVersion.toString();
181181
}
182182
// return undefined
183183
}
184184

185-
async function createSapApp() {
186-
async function isEmbeddedByLibrary(componentPath, libraryPathPrefix) {
185+
function createSapApp() {
186+
function hasManifest(componentPath, libraryPathPrefix) {
187187
const manifestPath = componentPath + "/manifest.json";
188188

189189
const manifestResource = libBundle.findResource(manifestPath.substring(libraryPathPrefix.length));
190190
if ( manifestResource == null ) {
191191
log.verbose(" component has no accompanying manifest.json, don't list it as 'embedded'");
192192
return false;
193193
}
194-
const manifestString = await manifestResource.getString();
195-
let manifest;
196-
try {
197-
manifest = JSON.parse(manifestString);
198-
} catch (err) {
199-
log.error(
200-
" component '%s': failed to read the component's manifest.json, " +
201-
"it won't be listed as 'embedded'.\nError details: %s", componentPath, err.stack);
202-
return false;
203-
}
204-
let embeddedBy;
205-
if (manifest && manifest["sap.app"]) {
206-
embeddedBy = manifest["sap.app"]["embeddedBy"];
207-
}
208-
if (typeof embeddedBy === "undefined") {
209-
log.verbose(" component doesn't declare 'sap.app/embeddedBy', don't list it as 'embedded'");
210-
return false;
211-
}
212-
if (typeof embeddedBy !== "string") {
213-
log.error(
214-
" component '%s': property 'sap.app/embeddedBy' is of type '%s' (expected 'string'), " +
215-
"it won't be listed as 'embedded'", componentPath, typeof embeddedBy
216-
);
217-
return false;
218-
}
219-
if ( !embeddedBy.length ) {
220-
log.error(
221-
" component '%s': property 'sap.app/embeddedBy' has an empty string value (which is invalid), " +
222-
"it won't be listed as 'embedded'", componentPath
223-
);
224-
return false;
225-
}
226-
let resolvedEmbeddedBy = posixPath.resolve(componentPath, embeddedBy);
227-
if ( resolvedEmbeddedBy && !resolvedEmbeddedBy.endsWith("/") ) {
228-
resolvedEmbeddedBy = resolvedEmbeddedBy + "/";
229-
}
230-
if ( libraryPathPrefix === resolvedEmbeddedBy ) {
231-
log.verbose(" component's 'sap.app/embeddedBy' property points to library, list it as 'embedded'");
232-
return true;
233-
} else {
234-
log.verbose(
235-
" component's 'sap.app/embeddedBy' points to '%s', don't list it as 'embedded'", resolvedEmbeddedBy
236-
);
237-
return false;
238-
}
194+
return true;
239195
}
240196

241-
async function findEmbeddedComponents() {
197+
function findEmbeddedComponents() {
242198
const result = [];
243-
const prefix = libraryResource.getPath().slice(0, - ".library".length);
199+
const prefix = posixPath.dirname(libraryResource.getPath()) + "/";
244200
const components = libBundle.getResources(/^\/(?:[^/]+\/)*Component\.js$/);
245201
for (const comp of components) {
246-
const componentPath = comp.getPath().slice(0, - "Component.js".length - 1);
202+
const componentPath = posixPath.dirname(comp.getPath());
247203
log.verbose("checking component at %s", componentPath);
248-
if ( componentPath.startsWith(prefix) && await isEmbeddedByLibrary(componentPath, prefix) ) {
204+
if ( componentPath.startsWith(prefix) && hasManifest(componentPath, prefix) ) {
249205
result.push( componentPath.substring(prefix.length) );
250206
} else if ( prefix === "/resources/sap/apf/" ) {
251207
log.verbose("Package %s contains both '*.library' and 'Component.js'. " +
@@ -358,7 +314,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
358314
_version: sectionVersion(APP_DESCRIPTOR_V3_SECTION_SAP_APP),
359315
id: library.getName(),
360316
type: "library",
361-
embeds: await findEmbeddedComponents(),
317+
embeds: findEmbeddedComponents(),
362318
i18n,
363319
applicationVersion: {
364320
version: isValid(library.getVersion()) ? library.getVersion() : getProjectVersion()
@@ -429,11 +385,6 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
429385

430386
function createSapUI5() {
431387
function getUI5Version() {
432-
let ui5Version;
433-
if ( ui5Version != null ) {
434-
return ui5Version;
435-
}
436-
437388
const dummy = new Dependency({
438389
libraryName: [{
439390
_: "sap.ui.core"
@@ -689,7 +640,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
689640

690641
return {
691642
"_version": descriptorVersion.toString(),
692-
"sap.app": await createSapApp(),
643+
"sap.app": createSapApp(),
693644
"sap.ui": createSapUi(),
694645
"sap.ui5": createSapUI5(),
695646
"sap.fiori": createSapFiori(),
@@ -701,7 +652,7 @@ async function createManifest(libraryResource, libBundle, descriptorVersion, _in
701652
module.exports = function({libraryResource, resources, options}) {
702653
// merge options with defaults
703654
options = Object.assign({
704-
descriptorVersion: APP_DESCRIPTOR_V22,
655+
descriptorVersion: APP_DESCRIPTOR_V22, // TODO change this to type string instead of a semver object
705656
include3rdParty: true,
706657
prettyPrint: true
707658
}, options);

‎test/lib/processors/manifestCreator.js

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,38 @@ test.serial("default manifest creation with special characters small app descrip
422422
t.is(await result.getString(), expectedManifestContentSmallVersionString, "Correct result returned");
423423
});
424424

425+
test.serial("default manifest creation with special characters very small app descriptor version", async (t) => {
426+
const {manifestCreator, errorLogStub} = t.context;
427+
const prefix = "/resources/sap/ui/mine/";
428+
const libraryResource = {
429+
getPath: () => {
430+
return prefix + ".library";
431+
},
432+
getString: async () => {
433+
return libraryContent;
434+
},
435+
_project: {
436+
dependencies: [{
437+
metadata: {
438+
name: "sap.ui.core"
439+
}
440+
}]
441+
}
442+
};
443+
t.is(errorLogStub.callCount, 0);
444+
445+
const options = {descriptorVersion: new Version("1.1.0")};
446+
const result = await manifestCreator({libraryResource, resources: [], options});
447+
const expectedManifestContentSmallVersion = expectedManifestContentObject();
448+
expectedManifestContentSmallVersion["_version"] = "1.1.0";
449+
expectedManifestContentSmallVersion["sap.app"]["_version"] = "1.2.0";
450+
expectedManifestContentSmallVersion["sap.ui"]["_version"] = "1.1.0";
451+
expectedManifestContentSmallVersion["sap.ui5"]["_version"] = "1.1.0";
452+
expectedManifestContentSmallVersion["sap.app"]["i18n"] = "i18n/i18n.properties";
453+
const sResult = await result.getString();
454+
t.deepEqual(JSON.parse(sResult), expectedManifestContentSmallVersion, "Correct result returned");
455+
});
456+
425457
test.serial("manifest creation for sap/apf", async (t) => {
426458
const {manifestCreator, errorLogStub, verboseLogStub} = t.context;
427459

@@ -696,7 +728,7 @@ test.serial("manifest creation with embedded component", async (t) => {
696728
"checking component at %s", "/resources/sap/lib1/component1"
697729
]);
698730
t.deepEqual(verboseLogStub.getCall(1).args, [
699-
" component's 'sap.app/embeddedBy' property points to library, list it as 'embedded'"
731+
" sap.app/id taken from .library: '%s'", "sap.lib1"
700732
]);
701733
});
702734

@@ -708,7 +740,9 @@ test.serial("manifest creation with embedded component (Missing 'embeddedBy')",
708740
"sap.app": {
709741
"id": "sap.lib1",
710742
"type": "library",
711-
"embeds": [],
743+
"embeds": [
744+
"component1"
745+
],
712746
"applicationVersion": {
713747
"version": "1.0.0"
714748
},
@@ -780,7 +814,7 @@ test.serial("manifest creation with embedded component (Missing 'embeddedBy')",
780814
"checking component at %s", "/resources/sap/lib1/component1"
781815
]);
782816
t.deepEqual(verboseLogStub.getCall(1).args, [
783-
" component doesn't declare 'sap.app/embeddedBy', don't list it as 'embedded'"
817+
" sap.app/id taken from .library: '%s'", "sap.lib1"
784818
]);
785819
});
786820

@@ -792,7 +826,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' doesn't poi
792826
"sap.app": {
793827
"id": "sap.lib1",
794828
"type": "library",
795-
"embeds": [],
829+
"embeds": [
830+
"component1"
831+
],
796832
"applicationVersion": {
797833
"version": "1.0.0"
798834
},
@@ -868,8 +904,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' doesn't poi
868904
"checking component at %s", "/resources/sap/lib1/component1"
869905
]);
870906
t.deepEqual(verboseLogStub.getCall(1).args, [
871-
" component's 'sap.app/embeddedBy' points to '%s', don't list it as 'embedded'",
872-
"/resources/sap/lib1/foo/"
907+
" sap.app/id taken from .library: '%s'", "sap.lib1"
873908
]);
874909
});
875910

@@ -881,7 +916,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' absolute pa
881916
"sap.app": {
882917
"id": "sap.lib1",
883918
"type": "library",
884-
"embeds": [],
919+
"embeds": [
920+
"component1"
921+
],
885922
"applicationVersion": {
886923
"version": "1.0.0"
887924
},
@@ -957,8 +994,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' absolute pa
957994
"checking component at %s", "/resources/sap/lib1/component1"
958995
]);
959996
t.deepEqual(verboseLogStub.getCall(1).args, [
960-
" component's 'sap.app/embeddedBy' points to '%s', don't list it as 'embedded'",
961-
"/"
997+
" sap.app/id taken from .library: '%s'", "sap.lib1"
962998
]);
963999
});
9641000

@@ -970,7 +1006,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' empty strin
9701006
"sap.app": {
9711007
"id": "sap.lib1",
9721008
"type": "library",
973-
"embeds": [],
1009+
"embeds": [
1010+
"component1"
1011+
],
9741012
"applicationVersion": {
9751013
"version": "1.0.0"
9761014
},
@@ -1039,12 +1077,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' empty strin
10391077
});
10401078
t.is(await result.getString(), expectedManifestContent, "Correct result returned");
10411079

1042-
t.is(errorLogStub.callCount, 1);
1043-
t.deepEqual(errorLogStub.getCall(0).args, [
1044-
" component '%s': property 'sap.app/embeddedBy' has an empty string value (which is invalid), " +
1045-
"it won't be listed as 'embedded'",
1046-
"/resources/sap/lib1/component1"
1047-
]);
1080+
t.is(errorLogStub.callCount, 0);
10481081
});
10491082

10501083
test.serial("manifest creation with embedded component ('embeddedBy' object)", async (t) => {
@@ -1055,7 +1088,9 @@ test.serial("manifest creation with embedded component ('embeddedBy' object)", a
10551088
"sap.app": {
10561089
"id": "sap.lib1",
10571090
"type": "library",
1058-
"embeds": [],
1091+
"embeds": [
1092+
"component1"
1093+
],
10591094
"applicationVersion": {
10601095
"version": "1.0.0"
10611096
},
@@ -1126,13 +1161,7 @@ test.serial("manifest creation with embedded component ('embeddedBy' object)", a
11261161
});
11271162
t.is(await result.getString(), expectedManifestContent, "Correct result returned");
11281163

1129-
t.is(errorLogStub.callCount, 1);
1130-
t.deepEqual(errorLogStub.getCall(0).args, [
1131-
" component '%s': property 'sap.app/embeddedBy' is of type '%s' (expected 'string'), " +
1132-
"it won't be listed as 'embedded'",
1133-
"/resources/sap/lib1/component1",
1134-
"object"
1135-
]);
1164+
t.is(errorLogStub.callCount, 0);
11361165
});
11371166

11381167
test.serial("manifest creation with embedded component (no manifest.json)", async (t) => {
@@ -1219,7 +1248,9 @@ test.serial("manifest creation with embedded component (invalid manifest.json)",
12191248
"sap.app": {
12201249
"id": "sap.lib1",
12211250
"type": "library",
1222-
"embeds": [],
1251+
"embeds": [
1252+
"component1"
1253+
],
12231254
"applicationVersion": {
12241255
"version": "1.0.0"
12251256
},
@@ -1284,15 +1315,7 @@ test.serial("manifest creation with embedded component (invalid manifest.json)",
12841315
});
12851316
t.is(await result.getString(), expectedManifestContent, "Correct result returned");
12861317

1287-
t.is(errorLogStub.callCount, 1);
1288-
t.is(errorLogStub.getCall(0).args.length, 3);
1289-
t.deepEqual(errorLogStub.getCall(0).args.slice(0, 2), [
1290-
" component '%s': failed to read the component's manifest.json, " +
1291-
"it won't be listed as 'embedded'.\n" +
1292-
"Error details: %s",
1293-
"/resources/sap/lib1/component1"
1294-
]);
1295-
t.true(errorLogStub.getCall(0).args[2].startsWith("SyntaxError: Unexpected token"));
1318+
t.is(errorLogStub.callCount, 0);
12961319
});
12971320

12981321
test.serial("manifest creation for invalid .library content", async (t) => {

0 commit comments

Comments
 (0)