From b89ee7522ba69f26036d3d89f03629895733beeb Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Thu, 24 Sep 2026 14:05:52 +0200 Subject: [PATCH 1/2] test(project): Add failing BuildReader theme-library routing tests A theme library contributes resources under a path that can collide with another project's namespace: themelib_sap_horizon has no namespace and serves /resources/sap/ui/core/themes/sap_horizon/library.css. BuildReader walks that path, matches the sap/ui/core namespace of the sap.ui.core library (which does not own the resource), reads null, then falls back to a reader for every project. That fallback (re)builds unrelated stale projects such as sap.m. Add an integration-style harness that records which projects a single byPath request would build, and three tests: the colliding theme resource, the base theme (owned by the library itself), and a non-colliding theme resource. The two theme-library routing tests fail against the current fallback-to-all behavior. The base-theme test already passes. --- .../project/test/lib/build/BuildReader.js | 159 +++++++++++++++++- 1 file changed, 156 insertions(+), 3 deletions(-) diff --git a/packages/project/test/lib/build/BuildReader.js b/packages/project/test/lib/build/BuildReader.js index a8205720fe7..f5a1633c9b7 100644 --- a/packages/project/test/lib/build/BuildReader.js +++ b/packages/project/test/lib/build/BuildReader.js @@ -63,7 +63,7 @@ test("byPath: falls back to all projects when primary reader returns null", asyn t.is(result, resource); }); -test("_getReaderForResource: final fallback when path doesn't match any namespace", async (t) => { +test("byPath: final fallback when path doesn't match any namespace", async (t) => { const projects = [ createMockProject("proj-a", "ns/a"), createMockProject("proj-b", "ns/b"), @@ -79,7 +79,7 @@ test("_getReaderForResource: final fallback when path doesn't match any namespac t.true(buildServerInterface.getReaderForProjects.called); }); -test("_getReaderForResource: uses cached reader to identify project", async (t) => { +test("byPath: uses cached reader to identify project", async (t) => { const projects = [ createMockProject("proj-a", "ns/a"), createMockProject("proj-b", "ns/b"), @@ -97,7 +97,7 @@ test("_getReaderForResource: uses cached reader to identify project", async (t) t.is(result, foundResource); }); -test("_getReaderForResource: application fallback for non-resource paths", async (t) => { +test("byPath: application fallback for non-resource paths", async (t) => { const projects = [ createMockProject("my-app", "my/app", "application"), createMockProject("my-lib", "my/lib"), @@ -114,3 +114,156 @@ test("_getReaderForResource: application fallback for non-resource paths", async const result = await reader.byPath("/index.html"); t.is(result, foundResource); }); + +// Integration-style harness modelling a realistic multi-project build server. Each project owns a +// set of virtual resource paths. Requesting a reader for a project (getReaderForProject / +// getReaderForProjects) records the project name in `builtProjects` to stand in for the build the +// BuildServer would trigger for a non-fresh project, so a test can assert which projects a single +// byPath request would (re)build. +function createBuildServerHarness(projectResources) { + const builtProjects = new Set(); + const projects = []; + + function createProjectReader(name) { + const paths = projectResources[name]; + const project = {getName: () => name}; + return { + async byPath(virPath) { + if (paths.has(virPath)) { + return {getPath: () => virPath, getProject: () => project}; + } + return null; + }, + async byGlob() { + return []; + }, + }; + } + + function createCombinedReader(names) { + const readers = names.map((name) => createProjectReader(name)); + return { + async byPath(virPath) { + for (const reader of readers) { + const res = await reader.byPath(virPath); + if (res) { + return res; + } + } + return null; + }, + async byGlob() { + return []; + }, + }; + } + + const buildServerInterface = { + async getReaderForProject(name) { + builtProjects.add(name); + return createProjectReader(name); + }, + async getReaderForProjects(names) { + for (const name of names) { + builtProjects.add(name); + } + return createCombinedReader(names); + }, + // Nothing is fresh in these cold-start scenarios, so no cached reader is available to + // identify the owning project without a build. + getCachedReadersForProjects() { + return undefined; + }, + }; + + return {projects, builtProjects, buildServerInterface}; +} + +// Regression: a theme library contributes resources under a path that collides with another +// project's namespace. "themelib.horizon" has no namespace (theme libraries can serve multiple) +// and provides "/resources/sap/ui/core/themes/sap_horizon/library.css". Walking that path matches +// the "sap/ui/core" namespace of the sap.ui.core library, which does not own the resource. The +// request must still resolve from the theme library without requesting a reader for every project, +// which would (re)build unrelated stale projects such as the sap.m library. +test("byPath: routes colliding theme-library resource without building unrelated projects", async (t) => { + const {builtProjects, buildServerInterface} = createBuildServerHarness({ + "app.a": new Set(["/index.html"]), + "sap.ui.core": new Set([ + "/resources/sap/ui/core/library.js", + // sap.ui.core ships the base theme itself + "/resources/sap/ui/core/themes/base/library.css", + ]), + "themelib.horizon": new Set([ + "/resources/sap/ui/core/themes/sap_horizon/library.css", + ]), + "sap.m": new Set(["/resources/sap/m/library.js"]), + }); + const projects = [ + createMockProject("app.a", "app/a", "application"), + createMockProject("sap.ui.core", "sap/ui/core", "library"), + createMockProject("themelib.horizon", null, "theme-library"), + createMockProject("sap.m", "sap/m", "library"), + ]; + const reader = new BuildReader("test", projects, buildServerInterface); + + const res = await reader.byPath("/resources/sap/ui/core/themes/sap_horizon/library.css"); + + t.truthy(res, "Resource is found"); + t.is(res.getProject().getName(), "themelib.horizon", "Resource resolves from the theme library"); + t.false(builtProjects.has("sap.m"), "Unrelated library sap.m is not built"); + t.false(builtProjects.has("app.a"), "Unrelated application is not built"); +}); + +// The base theme lives in the sap.ui.core library itself, so a request for it must resolve from the +// namespace-matched library without building any theme library. +test("byPath: routes base-theme resource to owning library without building theme libraries", async (t) => { + const {builtProjects, buildServerInterface} = createBuildServerHarness({ + "app.a": new Set(["/index.html"]), + "sap.ui.core": new Set([ + "/resources/sap/ui/core/themes/base/library.css", + ]), + "themelib.horizon": new Set([ + "/resources/sap/ui/core/themes/sap_horizon/library.css", + ]), + }); + const projects = [ + createMockProject("app.a", "app/a", "application"), + createMockProject("sap.ui.core", "sap/ui/core", "library"), + createMockProject("themelib.horizon", null, "theme-library"), + ]; + const reader = new BuildReader("test", projects, buildServerInterface); + + const res = await reader.byPath("/resources/sap/ui/core/themes/base/library.css"); + + t.truthy(res, "Resource is found"); + t.is(res.getProject().getName(), "sap.ui.core", "Resource resolves from the owning library"); + t.false(builtProjects.has("themelib.horizon"), "Theme library is not built for a base-theme request"); +}); + +// A theme library whose resource path does not collide with any namespace (nothing in the path +// matches a namespace) must still route to the theme library rather than falling back to all +// projects. +test("byPath: routes non-colliding theme-library resource without building unrelated projects", async (t) => { + const {builtProjects, buildServerInterface} = createBuildServerHarness({ + "app.a": new Set(["/index.html"]), + "sap.ui.core": new Set(["/resources/sap/ui/core/library.js"]), + "theme.library.e": new Set([ + "/resources/theme/library/e/themes/my_theme/library.css", + ]), + "sap.m": new Set(["/resources/sap/m/library.js"]), + }); + const projects = [ + createMockProject("app.a", "app/a", "application"), + createMockProject("sap.ui.core", "sap/ui/core", "library"), + createMockProject("theme.library.e", null, "theme-library"), + createMockProject("sap.m", "sap/m", "library"), + ]; + const reader = new BuildReader("test", projects, buildServerInterface); + + const res = await reader.byPath("/resources/theme/library/e/themes/my_theme/library.css"); + + t.truthy(res, "Resource is found"); + t.is(res.getProject().getName(), "theme.library.e", "Resource resolves from the theme library"); + t.false(builtProjects.has("sap.m"), "Unrelated library sap.m is not built"); + t.false(builtProjects.has("app.a"), "Unrelated application is not built"); +}); From 6c2cea6836a8c57cfa89e16c24a0b3fd14f0234c Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Thu, 24 Sep 2026 14:05:53 +0200 Subject: [PATCH 2/2] fix(project): Route theme-library resources in BuildReader Theme libraries have no namespace and serve their resources under a path owned by another project, e.g. themelib_sap_horizon serves /resources/sap/ui/core/themes/sap_horizon/library.css. The previous routing matched such a path to the sap/ui/core namespace of the sap.ui.core library, read null there, then requested a reader for every project. That request (re)builds every non-fresh project, including ones unrelated to the theme. Replace the single best-guess reader with an ordered list of candidate reader factories evaluated lazily by byPath, returning the first resource found: 1. cached readers of already-built projects (free, identifies the owner) 2. namespace matches, most specific first 3. theme libraries, when the path has a themes/ segment 4. the application root project, for non-/resources paths 5. all projects (last resort) A reader is requested from the build server only when the preceding candidates did not yield the resource, so the minimal set of projects is built. The base theme, owned by sap.ui.core itself, still resolves at the namespace step without building any theme library. --- packages/project/lib/build/BuildReader.js | 121 ++++++++++++------ .../project/test/lib/build/BuildReader.js | 13 +- 2 files changed, 93 insertions(+), 41 deletions(-) diff --git a/packages/project/lib/build/BuildReader.js b/packages/project/lib/build/BuildReader.js index 51abd136ee3..709e1ee6588 100644 --- a/packages/project/lib/build/BuildReader.js +++ b/packages/project/lib/build/BuildReader.js @@ -14,6 +14,7 @@ class BuildReader extends AbstractReader { #projects; #projectNames; #applicationProjectName; + #themeLibraryProjectNames = []; #namespaces = new Map(); #buildServerInterface; @@ -46,6 +47,15 @@ class BuildReader extends AbstractReader { if (project.getType() === "application") { this.#applicationProjectName = project.getName(); } + + // Theme libraries have no namespace (they can contribute themes to several) and serve + // their resources under a path owned by another project, e.g. themelib_sap_horizon + // serves /resources/sap/ui/core/themes/sap_horizon/. Namespace matching therefore + // routes such a request to the wrong project, so theme libraries are tracked + // separately and offered as a routing candidate for theme resource paths. + if (project.getType() === "theme-library") { + this.#themeLibraryProjectNames.push(project.getName()); + } } } @@ -66,8 +76,10 @@ class BuildReader extends AbstractReader { /** * Locates a resource by path * - * Attempts to determine the appropriate project reader based on the resource path - * and namespace. Falls back to searching all projects if the resource cannot be found. + * Tries candidate readers in priority order (see {@link BuildReader#_getReaderCandidates}) + * and returns the first resource found. Each candidate reader is requested from the build server + * only when the preceding ones did not yield the resource, so the minimal set of projects is + * built. The reader for all projects is the last resort. * * @public * @param {string} virPath Virtual path of the resource @@ -75,61 +87,98 @@ class BuildReader extends AbstractReader { * @returns {Promise<@ui5/fs/Resource|null>} Promise resolving to resource or null if not found */ async byPath(virPath, ...args) { - const reader = await this._getReaderForResource(virPath); - let res = await reader.byPath(virPath, ...args); - if (!res) { - // Fallback to unspecified projects - const allReader = await this.#buildServerInterface.getReaderForProjects(this.#projectNames); - res = await allReader.byPath(virPath, ...args); + for (const getReader of this._getReaderCandidates(virPath)) { + const reader = await getReader(); + if (!reader) { + continue; + } + const res = await reader.byPath(virPath, ...args); + if (res) { + return res; + } } - return res; + return null; } /** - * Gets the appropriate reader for a resource at the given path + * Builds the ordered list of candidate readers for a resource path * - * Determines which project(s) might contain the resource based on namespace matching - * and returns a reader for those projects. For single-project readers, returns that - * project's reader directly. + * Each entry is a factory resolving to a reader (or undefined when its strategy does not apply). + * Factories are evaluated lazily by {@link BuildReader#byPath} and requesting a reader from the + * build server may (re)build the associated projects, so ordering minimizes unnecessary builds: + * cheaper and more specific strategies come first, the reader for all projects comes last. * * @param {string} virPath Virtual path of the resource - * @returns {Promise<@ui5/fs/AbstractReader>} Promise resolving to appropriate reader + * @returns {Array>} Ordered readers */ - async _getReaderForResource(virPath) { + _getReaderCandidates(virPath) { if (this.#projects.length === 1) { // Filtering on a single project (typically the root project) - return await this.#buildServerInterface.getReaderForProject(this.#projectNames[0]); - } - // Determine project for resource path - const projects = this._getProjectsForResourcePath(virPath); - if (projects.length) { - return await this.#buildServerInterface.getReaderForProjects(projects); + return [ + () => this.#buildServerInterface.getReaderForProject(this.#projectNames[0]), + ]; } - // Unable to determine project for resource using path - // Fallback 1: Try to find resource in cached readers (if available) to identify the relevant project - const cachedReader = this.#buildServerInterface.getCachedReadersForProjects(this.#projectNames); - if (cachedReader) { + const readers = []; + + // Cached readers hold the results of already-built (fresh) projects and are free to query, + // so consult them first: a hit identifies the owning project without building anything. + readers.push(async () => { + const cachedReader = this.#buildServerInterface.getCachedReadersForProjects(this.#projectNames); + if (!cachedReader) { + return; + } const res = await cachedReader.byPath(virPath); if (res) { - // Found resource in one of the cached readers. Assume it still belongs to the associated project - return this.#buildServerInterface.getReaderForProject(res.getProject().getName()); + // Found in a cached reader. Request the project's own reader so a subsequent + // invalidation is reflected, assuming the resource still belongs to that project. + return await this.#buildServerInterface.getReaderForProject(res.getProject().getName()); } + }); + + // Namespace matches, most specific first. Offered individually so a more specific match is + // tried (and its project built) before a less specific one. + for (const projectName of this._getProjectsForResourcePath(virPath)) { + readers.push(() => this.#buildServerInterface.getReaderForProject(projectName)); } - // Fallback 2: If the root project is of type application, and the request does not start with - // /resources/ or /test-resources/, test whether the resource can be found in the root project + // Theme libraries serve resources under a path owned by another project's namespace, so the + // namespace match above can miss them. When the path looks like a theme resource, offer the + // theme libraries as a candidate before falling back to all projects. + if (this.#themeLibraryProjectNames.length && this._isThemeResourcePath(virPath)) { + readers.push(() => + this.#buildServerInterface.getReaderForProjects(this.#themeLibraryProjectNames)); + } + + // If the root project is an application and the request does not start with /resources/ or + // /test-resources/, the resource may live in the application project itself. if (this.#applicationProjectName && !virPath.startsWith("/resources/") && !virPath.startsWith("/test-resources/")) { - const appReader = await this.#buildServerInterface.getReaderForProject(this.#applicationProjectName); - const res = await appReader.byPath(virPath); - if (res) { - return appReader; - } + readers.push(() => this.#buildServerInterface.getReaderForProject(this.#applicationProjectName)); } - // Fallback to request a reader for all projects - return await this.#buildServerInterface.getReaderForProjects(this.#projectNames); + // Last resort: a reader for all projects. This (re)builds every non-fresh project, so it is + // only reached when no more specific strategy located the resource. + readers.push(() => this.#buildServerInterface.getReaderForProjects(this.#projectNames)); + + return readers; + } + + /** + * Checks whether a path looks like a theme-library resource + * + * Theme libraries serve their resources under a "themes//" segment + * (e.g. /resources/sap/ui/core/themes/sap_horizon/library.css), so only such paths + * are routed to theme libraries. + * + * @param {string} virPath Virtual resource path + * @returns {boolean} True if the path is a resource path with a "themes" segment + */ + _isThemeResourcePath(virPath) { + if (!virPath.startsWith("/resources/") && !virPath.startsWith("/test-resources/")) { + return false; + } + return virPath.split("/").includes("themes"); } /** diff --git a/packages/project/test/lib/build/BuildReader.js b/packages/project/test/lib/build/BuildReader.js index f5a1633c9b7..7ad3a95737e 100644 --- a/packages/project/test/lib/build/BuildReader.js +++ b/packages/project/test/lib/build/BuildReader.js @@ -49,18 +49,21 @@ test("byPath: returns resource from primary reader", async (t) => { t.is(result, resource); }); -test("byPath: falls back to all projects when primary reader returns null", async (t) => { +test("byPath: single project queries only its own reader", async (t) => { + // For a single project, #getReaderForProjects short-circuits to the same reader as + // #getReaderForProject, so byPath offers only the single project's reader and consults + // nothing else when it returns null. const projects = [createMockProject("proj-a", "my/ns")]; - const resource = {getPath: () => "/resources/my/ns/a.js"}; const primaryReader = {byPath: sinon.stub().resolves(null)}; - const fallbackReader = {byPath: sinon.stub().resolves(resource)}; const buildServerInterface = { getReaderForProject: sinon.stub().resolves(primaryReader), - getReaderForProjects: sinon.stub().resolves(fallbackReader), + getReaderForProjects: sinon.stub().resolves(primaryReader), }; const reader = new BuildReader("test", projects, buildServerInterface); const result = await reader.byPath("/resources/my/ns/a.js"); - t.is(result, resource); + t.is(result, null); + t.is(buildServerInterface.getReaderForProject.callCount, 1); + t.is(buildServerInterface.getReaderForProjects.callCount, 0); }); test("byPath: final fallback when path doesn't match any namespace", async (t) => {