Skip to content

Commit

Permalink
[FIX] middleware/versionInfo: Only process dependencies of type 'libr…
Browse files Browse the repository at this point in the history
…ary'

Ignore any other project types, even if the project contains a .library
file.

Some projects might consume UI5 libraries configured as type "module".
In order to align the ui5-server's behavior with the ui5-builder, those
projects must be ignored when generating the sap-ui-version.json.

For the same reason the serveResources middleware must not attempt to
generate a missing manifest.json for such projects.

Resolves SAP/ui5-tooling#954
  • Loading branch information
RandomByte committed May 15, 2024
1 parent 75264f1 commit 4c99455
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 22 deletions.
6 changes: 4 additions & 2 deletions lib/middleware/helper/generateLibraryManifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export default async function generateLibraryManifest(middlewareUtil, dotLibReso
return middlewareUtil.getProject(projectName)?.getVersion();
}
});
res.setProject(project);
return res;
if (res) {
res.setProject(project);
return res;
}
}
10 changes: 5 additions & 5 deletions lib/middleware/serveResources.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ function createMiddleware({resources, middlewareUtil}) {
// Attempt to find a .library file, which is required for generating a manifest.json
const dotLibraryPath = pathname.replace(rManifest, "/.library");
const dotLibraryResource = await resources.all.byPath(dotLibraryPath);
if (!dotLibraryResource) {
log.verbose(
`Could not find a .library to generate manifest.json from at ${dotLibraryPath}. ` +
`This might indicate that the project is not a library project.`);
if (dotLibraryResource && dotLibraryResource.getProject()?.getType() === "library") {
resource = await generateLibraryManifest(middlewareUtil, dotLibraryResource);
}
if (!resource) {
// Not a library project, missing .library file or other reason for failed manifest.json generation
next();
return;
}
resource = await generateLibraryManifest(middlewareUtil, dotLibraryResource);
}

const resourcePath = resource.getPath();
Expand Down
4 changes: 3 additions & 1 deletion lib/middleware/versionInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ function createMiddleware({resources, middlewareUtil}) {
return async function versionInfo(req, res, next) {
try {
const dependencies = resources.dependencies;
const dotLibResources = await dependencies.byGlob("/resources/**/.library");
let dotLibResources = await dependencies.byGlob("/resources/**/.library");

dotLibResources = dotLibResources.filter((res) => res.getProject()?.getType() === "library");

dotLibResources.sort((a, b) => {
return a.getProject().getName().localeCompare(b.getProject().getName());
Expand Down
84 changes: 74 additions & 10 deletions test/lib/server/middleware/serveResources.js
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,9 @@ test.serial("Check if utf8 characters are correctly processed in version replace
test.serial("Missing manifest.json is generated", async (t) => {
// For projects not extending type "ComponentProject" the method "getPropertiesFileSourceEncoding" is not available
const project = {
getName: () => "library",
getNamespace: () => "library",
getName: () => "my.library",
getNamespace: () => "my/namespace",
getType: () => "library",
getVersion: () => "1.0.0",
getSpecVersion: () => {
return {
Expand All @@ -544,11 +545,11 @@ test.serial("Missing manifest.json is generated", async (t) => {

project.getReader = () => readerWriter;

const dotLibraryMock = await writeResource(readerWriter, "/resources/foo/.library", 1024 * 1024,
const dotLibraryMock = await writeResource(readerWriter, "/resources/my/namespace/.library", 1024 * 1024,
`dot library content`, project);

const manifestMock = resourceFactory.createResource({
path: "/resources/foo/manifest.json",
path: "/resources/my/namespace/manifest.json",
string: "mocked manifest.json ${version}",
project,
});
Expand All @@ -572,7 +573,7 @@ test.serial("Missing manifest.json is generated", async (t) => {
});

const req = {
url: "/resources/foo/manifest.json",
url: "/resources/my/namespace/manifest.json",
headers: {}
};
const res = new Writable();
Expand Down Expand Up @@ -602,8 +603,9 @@ test.serial("Missing manifest.json is generated", async (t) => {
test.serial("Missing manifest.json is not generated with missing .library", async (t) => {
// For projects not extending type "ComponentProject" the method "getPropertiesFileSourceEncoding" is not available
const project = {
getName: () => "library",
getNamespace: () => "library",
getName: () => "my.library",
getNamespace: () => "my/namespace",
getType: () => "library",
getVersion: () => "1.0.0",
getSpecVersion: () => {
return {
Expand Down Expand Up @@ -634,7 +636,7 @@ test.serial("Missing manifest.json is not generated with missing .library", asyn
});

const req = {
url: "/resources/foo/manifest.json",
url: "/resources/my/namespace/manifest.json",
headers: {}
};

Expand All @@ -652,8 +654,9 @@ test.serial("Missing manifest.json is not generated with missing .library", asyn
test.serial("Missing manifest.json is not generated for request outside /resources", async (t) => {
// For projects not extending type "ComponentProject" the method "getPropertiesFileSourceEncoding" is not available
const project = {
getName: () => "library",
getNamespace: () => "library",
getName: () => "my.library",
getNamespace: () => "my/namespace",
getType: () => "library",
getVersion: () => "1.0.0",
getSpecVersion: () => {
return {
Expand All @@ -665,6 +668,11 @@ test.serial("Missing manifest.json is not generated for request outside /resourc

const readerWriter = resourceFactory.createAdapter({virBasePath: "/"});

project.getReader = () => readerWriter;

await writeResource(readerWriter, "/.library", 1024 * 1024,
`dot library content`, project);

const generateLibraryManifestHelperStub = sinon.stub().resolves();
const serveResourcesMiddlewareWithMock = t.context.serveResourcesMiddlewareWithMock =
await esmock.p("../../../../lib/middleware/serveResources", {
Expand Down Expand Up @@ -698,3 +706,59 @@ test.serial("Missing manifest.json is not generated for request outside /resourc
});
});
});

test.serial("Missing manifest.json is not generated for non-library projects", async (t) => {
// For projects not extending type "ComponentProject" the method "getPropertiesFileSourceEncoding" is not available
const project = {
getName: () => "my.library",
getNamespace: () => "my/namespace",
getType: () => "module", // => Looks like a library, has a .library file but is of type module
getVersion: () => "1.0.0",
getSpecVersion: () => {
return {
toString: () => "3.0",
lte: () => false,
};
}
};

const readerWriter = resourceFactory.createAdapter({virBasePath: "/", project});

project.getReader = () => readerWriter;

await writeResource(readerWriter, "/resources/my/namespace/.library", 1024 * 1024,
`dot library content`, project);

const generateLibraryManifestHelperStub = sinon.stub().resolves();
const serveResourcesMiddlewareWithMock = t.context.serveResourcesMiddlewareWithMock =
await esmock.p("../../../../lib/middleware/serveResources", {
"../../../../lib/middleware/helper/generateLibraryManifest.js": generateLibraryManifestHelperStub
});

const middleware = serveResourcesMiddlewareWithMock({
middlewareUtil: new MiddlewareUtil({
graph: {
getProject: () => project
},
project: "project"
}),
resources: {
all: readerWriter
}
});

const req = {
url: "/resources/my/namespace/manifest.json",
headers: {}
};

return new Promise((resolve, reject) => {
middleware(req, undefined, function(err) {
if (err) {
throw new Error(`Next callback called with error: ${err.message}`);
}
t.is(generateLibraryManifestHelperStub.callCount, 0, "generateLibraryManifest helper never got called");
resolve();
});
});
});
18 changes: 14 additions & 4 deletions test/lib/server/middleware/versionInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ const projectCache = {};
*
* @param {string[]} names e.g. ["lib", "a"]
* @param {string} [version="3.0.0-<library name>"] Project version
* @param {string} [type="library"] Project type
* @returns {object} Project mock
*/
const createProjectMetadata = (names, version) => {
const createProjectMetadata = (names, version, type) => {
const key = names.join(".");

// Cache projects in order to return same object instance
Expand All @@ -31,6 +32,7 @@ const createProjectMetadata = (names, version) => {
getName: () => key,
getNamespace: () => names.join("/"),
getVersion: () => version || "3.0.0-" + key,
getType: () => type || "library"
};
};

Expand Down Expand Up @@ -122,9 +124,9 @@ function createDepWorkspace(names, oOptions = {
virBasePath: "/resources"
}) {
const project = createProjectMetadata(names);
oOptions = Object.assign(oOptions, {
oOptions = Object.assign({
project
});
}, oOptions);
const workspace = resourceFactory.createAdapter(oOptions);
// Connect the project back to the created workspace, this allows for accessing the reader via a resources project
project.getReader = () => workspace;
Expand Down Expand Up @@ -183,12 +185,20 @@ test.serial("test all inner API calls within middleware", async (t) => {
const dependenciesA = createDepWorkspace(["lib", "a"], {virBasePath: "/"});
const dependenciesB = createDepWorkspace(["lib", "b"], {virBasePath: "/"});
const dependenciesC = createDepWorkspace(["lib", "c"], {virBasePath: "/"});
const dependenciesX = createDepWorkspace(["module", "x"], {
virBasePath: "/",
project: createProjectMetadata(["module", "x"], "1.0.0", "module")
});
// create lib.a without manifest
await createDotLibrary(dependenciesA, resourceFactory, ["lib", "a"], [{name: "lib.b"}, {name: "lib.c"}]);
// create lib.b with manifest: no manifestCreator call expected
await createResources(dependenciesB, resourceFactory, ["lib", "b"], []);
// create lib.c without manifest but with dummy files
await createDotLibrary(dependenciesC, resourceFactory, ["lib", "c"]);
// create module.x without manifest but with dummy files
// Since this is not a library project, no manifest.json should be generated
await createDotLibrary(dependenciesX, resourceFactory, ["module", "x"]);

[
// relevant file extensions for manifest creation
"js", "json", "less", "css", "theming", "theme", "properties",
Expand All @@ -201,7 +211,7 @@ test.serial("test all inner API calls within middleware", async (t) => {
const resources = {
dependencies: resourceFactory.createReaderCollection({
name: "dependencies",
readers: [dependenciesA, dependenciesB, dependenciesC]
readers: [dependenciesA, dependenciesB, dependenciesC, dependenciesX]
})
};
const middlewareUtil = {
Expand Down

0 comments on commit 4c99455

Please sign in to comment.