Draft
Conversation
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
…e_layer # Conflicts: # dhis-2/dhis-support/dhis-support-external/src/main/java/org/hisp/dhis/external/conf/ConfigurationKey.java
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
|
Instance deployed to https://dev.im.dhis2.org/pr-23086 |
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
Adds a feature-flagged canonical app serving mode where all app resources are served from
/apps/{appName}/instead of the legacy/dhis-web-{appName}/. This eliminates the double resource loading that occurs when the global shell is enabled, where shared dependencies (fonts, CSS) are downloaded twice from different URL namespaces.Feature flag:
canonicalAppPathssystem setting (default:false). Zero behavior change until explicitly enabled. RequiresglobalShellEnabled=true-- automatically treated asfalsewhen the global shell is disabled.No frontend changes required. The global-shell-app already handles both naming schemes.
Builds on top of PR #23000 (static cache layer).
Problem
When the global shell is enabled and a user navigates to
/apps/reports:GlobalShellFilterserves the global-shell'sindex.htmlwith relative asset paths (./assets/main.js)/apps/reports/assets/main.js-- served from the global-shell app/dhis-web-reports/index.html?redirect=false(the legacy URL fromdefaultAction)/dhis-web-reports/assets/font.woff2/apps/...and once from/dhis-web-reports/...This happens because the browser treats them as different URLs despite serving identical content.
Root cause traced
The chain of causation starts in the backend:
App.init()usesBUNDLED_APP_PREFIX = "dhis-web-"to buildbasePathandlaunchUrlWebModule.getModule()copieslaunchUrlintodefaultActionGET /api/apps/menureturnsdefaultAction: "/dhis-web-reports/index.html"PluginLoadercreates an iframe at that legacy URLThe global-shell-app frontend code already anticipates the change:
Solution
Feature flag
A
canonicalAppPathssystem setting gates all new behavior:When disabled (default): zero behavior change -- everything works exactly as before.
What changes when enabled
1.
App.init()uses canonical pathsAll apps (bundled AND installed) use
CANONICAL_APP_PREFIX = "apps/"instead of"dhis-web-"or"api/apps/". This changesbasePath,baseUrl,launchUrl, and consequentlydefaultActionin/api/apps/menu.Before:
defaultAction: "/dhis-web-reports/index.html"After:
defaultAction: "/apps/reports/index.html"2.
GlobalShellFilterroutes subresources to the actual appPreviously, ALL
/apps/**subresource requests were forwarded to the global-shell app. Now the routing is app-aware:/apps/{appName}(exact, no subpath)index.htmlwith rewritten asset paths/apps/{appName}/index.html(noredirect=false)/apps/{appName}(shell's router only understands the short form)/apps/{appName}/index.html?redirect=false/apps/global-shell/{resource}/apps/{knownApp}/{resource}+ flag ON{knownApp}app's resource/apps/{unknown}/{resource}or flag OFFThe "known app" check (
appManager.getApp(appName) != null) prevents paths like/apps/assets/main.jsfrom being misrouted --assetsis not an app, so it falls back to global-shell'sassets/main.js.3. Direct URL rewriting for global shell assets
When serving the global shell's
index.htmlat/apps/{appName}, all relative asset paths are rewritten to absolute paths pointing at the global-shell app:This is done via string replacement on the Jsoup-processed HTML:
Uses
request.getContextPath()so it works correctly with context path deployments (host.com/server_a).Why direct URL rewriting instead of
<base>tag: The initial approach injected<base href="{contextPath}/apps/global-shell/">. While curl confirmed the tag was present in the HTML, browsers did not respect it (assets still resolved against the document URL). Direct URL rewriting is foolproof and has no side effects on JavaScript relative URL resolution.4. Canonical-aware service worker replacement
When canonical paths are ON, the global shell's standard
@dhis2/pwaservice worker is replaced with a lightweight canonical-aware service worker. Any request forservice-worker.jsunder/apps/is intercepted and our replacement is served from a classpath resource (canonical-service-worker.js).The canonical SW:
/apps/global-shell/**) with cache-first strategy for instant reloads@dhis2/pwamessage protocol (GET_CLIENTS_INFO,SKIP_WAITING,CLAIM_CLIENTS,GET_IMMEDIATE_DHIS2_CONNECTION_STATUS_UPDATE) so the shell's PWA code initializes correctlyCache-Controlheaders fromStaticCacheControlServiceThis is necessary because the standard
@dhis2/pwaSW was designed for the legacy URL scheme -- it precaches the shell'sindex.htmland serves it for ALL navigation requests under/apps/, including iframe URLs for actual apps. With canonical paths, this causes redirect loops, 404s, and React Router errors.When canonical paths are OFF, the real service worker is served as before.
5. Legacy paths get 302 redirects (only for actual apps)
/dhis-web-{appName}/**requests are 302-redirected to/apps/{appName}/**instead of being internally forwarded, but only when{appName}is a known app. Non-app legacy paths (e.g./dhis-web-commons/menu/getModules.action,/dhis-web-apps/apps-bundle.json) are not redirected and fall through to their original handlers. This:commons(shared static) andapps(bundle info)GlobalShellFilterandAppOverrideFilterdo thisResult: single URL namespace
Files changed
SystemSettings.javagetCanonicalAppPaths()setting (default:false); returnsfalsewhenglobalShellEnabledisfalseAppManager.javaCANONICAL_APP_PREFIX = "apps/"constantApp.javainit(contextPath, canonicalAppPaths)overloadDefaultAppManager.javaSystemSettingsProvider, passes flag toApp.init()DefaultAppManagerTest.javaGlobalShellFilter.java/apps/{app}/index.htmlredirectAppOverrideFilter.javacanonical-service-worker.js@dhis2/pwamessage protocolGotchas (fixed during implementation)
301 vs 302: Initially used 301 (Moved Permanently). Browsers cache 301 redirects aggressively. When the user disabled the feature flag, cached 301s caused legacy paths to still redirect to canonical, then 404. Fix: Use 302 (Found) during the feature-flag phase so redirects are not cached permanently.
Redirecting non-app paths: The legacy pattern
/dhis-web-{X}/**matched any X. Paths like/dhis-web-commons/menu/getModules.actionand/dhis-web-apps/apps-bundle.jsonwere redirected to/apps/commons/...and/apps/apps/..., which then 404'd ("App 'commons' not found"). Fix: Only redirect whenappManager.getApp(appName)returns a non-null app. Non-app paths fall through to their original handlers.<base>tag not respected by browsers: Injecting<base href="/apps/global-shell/">into the shell's HTML was confirmed present by curl, but browsers ignored it -- assets still resolved relative to the document URL. Fix: Replaced with direct URL rewriting (href="./tohref="/apps/global-shell/). This is simpler, more reliable, and avoids<base>tag side effects on JavaScript relative URL resolution.APP_SUBRESOURCE_PATTERNtoo aggressive: The pattern treating the first path segment after/apps/as an app name broke paths like/apps/assets/main.js(whereassetsis a directory in global-shell, not an app). Fix: CheckappManager.getApp(appName) != nullbefore routing to an app. Unknown names fall back to the original behavior (full path treated as global-shell resource).RequestDispatcher.forward() hang: The initial approach used
RequestDispatcher.forward()with anHttpServletResponseWrapperto capture the shell's HTML for rewriting. The forward blocked indefinitely (likely re-entering the filter chain). Fix: Load the resource directly viaAppManager.getAppResource()and apply cache-busting + URL rewriting in the filter, bypassing the servlet dispatch entirely.canonicalAppPathsrequiresglobalShellEnabled: With global shell OFF,redirectDisabledGlobalShell()redirects ALL/apps/**subresource paths to the root URL. With canonical paths ON, the app'slaunchUrlbecomes/apps/{appName}/index.html, which triggers that redirect -- breaking the app. Fix:SystemSettings.getCanonicalAppPaths()now returnsfalsewhengetGlobalShellEnabled()isfalse. The dependency is enforced at the settings level so all callers automatically get the right behavior.Service worker incompatible with canonical paths: The
@dhis2/pwaWorkbox SW precachesindex.htmland serves it for ALL navigation requests, causing redirect loops and 404s when canonical paths route different resources through/apps/. Fix: Replace the SW with a canonical-aware version that only caches/apps/global-shell/**assets (cache-first) and handles the@dhis2/pwamessage protocol (GET_CLIENTS_INFO,SKIP_WAITING,CLAIM_CLIENTS,GET_IMMEDIATE_DHIS2_CONNECTION_STATUS_UPDATE). Key details:self.clients.claim()in activate -- without it,navigator.serviceWorker.controllerisnulland the shell'sOfflineInterface.swMessage()throws, crashing the React tree on reload.GET_CLIENTS_INFO(notCLIENTS_INFO) -- the message types are asymmetric (client sendsGET_..., SW responds with...).GET_IMMEDIATE_DHIS2_CONNECTION_STATUS_UPDATEwith a defaultisConnected: trueto prevent errors from the connection status subscription.caches.keys()+Promise.all(delete)combined withclients.claim()caused the shell to hang. Only clean up known Workbox cache names (workbox-*,other-assets,app-shell).Shell router doesn't match
/apps/{app}/index.html: The shell's React Router only matches/apps/{appName}(noindex.html). If the browser URL changes to/apps/dashboard/index.html#/(via the app's router or<base target="_top">), a reload serves the shell HTML but the router can't match the path. Fix: Redirect/apps/{appName}/index.html(withoutredirect=false) to/apps/{appName}. The iframe URL with?redirect=falsestill serves the actual app HTML.Deployment / rollout plan
truefor 2.43-rc1No frontend changes or coordination needed. The global-shell-app already handles both naming conventions.
Important for QA: When toggling the flag between ON and OFF during testing, clear the browser's service worker and site data to avoid stale cached responses. In production, this is not an issue since the flag will be set once and stay.
Test plan
Flag OFF (default) -- verify zero behavior change
Flag ON -- enable and verify
Regression checks
/api/systemSettings/globalShellEnabled = false)Service worker testing note
When toggling
canonicalAppPathsbetween ON and OFF during QA testing:This is only needed during testing when flipping the flag. In production, the flag is set once and the SW naturally refreshes on the next app upgrade.
Test results
AppControllerTest(integration)DefaultAppManagerTestStaticCacheControlServiceTestService Worker Analysis: Global Shell App
Context
The global-shell-app registers a Workbox-based service worker (
pwa: { enabled: true }ind2.config.js). This analysis documents how the SW conflicts with canonical app paths, the replacement SW we built, and the technical details of the@dhis2/pwamessage protocol.What the original
@dhis2/pwaservice worker doesWorkbox precaching
Precaches the shell's
index.htmlduring install. On navigation, serves the precached HTML instead of going to the network (after checking auth via a network fetch first).Navigation route handler
Intercepts ALL navigation requests. If the network fetch succeeds with 200, returns the PRECACHED
index.html(not the network response). This is the standard SPA pattern -- every URL gets the same HTML, the client-side router renders the right view.Caching strategies
Message handlers
GET_CLIENTS_INFO-> responds withCLIENTS_INFO+{ clientsCount }SKIP_WAITING-> callsself.skipWaiting()CLAIM_CLIENTS-> callsself.clients.claim()+ reload clientsGET_IMMEDIATE_DHIS2_CONNECTION_STATUS_UPDATE-> broadcasts connection statusSTART_RECORDING/COMPLETE_RECORDING-> offline recording modePWA features
How the original SW conflicts with canonical paths
The navigation route is the root cause
The original SW's navigation handler serves the PRECACHED
index.htmlfor every navigation under/apps/. With canonical paths:/apps/dashboard-> SW intercepts, serves precached shell HTML/apps/dashboard/index.html?redirect=falseindex.htmlinstead of the DASHBOARD's HTMLStale cache after flag toggle
The Workbox precache contains the shell's
index.htmlwithout URL rewriting (cached before canonical paths was enabled). Even with the flag toggled, the SW serves the old cached version.redirect=falsenot understood by SWThe
?redirect=falsequery parameter is meaningful toGlobalShellFilter(serve actual app, not shell). But the SW's navigation handler doesn't check for it -- it serves precached HTML regardless of query parameters.The canonical-aware replacement SW
Located at
dhis-web-api/src/main/resources/canonical-service-worker.js. Served byGlobalShellFilterwhencanonicalAppPathsis ON.What it does
Caches global-shell assets (
/apps/global-shell/**) with cache-first strategy. Hashed filenames are immutable, so the cache is always correct. After first load, shell assets are served from SW cache on subsequent navigations.Handles the
@dhis2/pwamessage protocol so the shell's React code initializes correctly:GET_CLIENTS_INFO-> responds withCLIENTS_INFO+ client countSKIP_WAITING-> callsself.skipWaiting()CLAIM_CLIENTS-> callsself.clients.claim()GET_IMMEDIATE_DHIS2_CONNECTION_STATUS_UPDATE-> responds withisConnected: trueDoes NOT intercept navigation or non-shell fetches. All other caching is handled by HTTP
Cache-Controlheaders fromStaticCacheControlService.Cleans up old Workbox caches on activation (
workbox-*,other-assets,app-shell).What it does NOT do
isConnected: true)Critical implementation details
These were discovered through iterative debugging:
Must call
self.clients.claim()in activate: Without this,navigator.serviceWorker.controllerisnullon the page. The shell'sOfflineInterface.swMessage()helper checksnavigator.serviceWorker.controllerand THROWS if it's null. This crashes the React tree on page reload (first load works because the SW isn't controlling yet).Must listen for
GET_CLIENTS_INFO(notCLIENTS_INFO): The message types are asymmetric. The client sendsGET_CLIENTS_INFO, the SW responds withCLIENTS_INFO. Getting this wrong causes the shell's PWA code to time out, and the@dhis2/app-runtimeProvider may block further initialization.Must respond to
GET_IMMEDIATE_DHIS2_CONNECTION_STATUS_UPDATE: The shell subscribes to connection status updates early in initialization. TheswMessage()helper sends this message vianavigator.serviceWorker.controller.postMessage(). If the SW doesn't handle it, the message is silently lost (no error), but responding with{ isConnected: true }prevents potential edge cases.Must NOT clear all caches synchronously with
clients.claim(): Usingcaches.keys().then(keys => Promise.all(keys.map(k => caches.delete(k)))).then(() => self.clients.claim())in the activate handler caused the shell to hang indefinitely. The fix: only delete known Workbox cache names (workbox-*,other-assets,app-shell), or skip cache clearing entirely.SW scope matters: The shell registers the SW from
./service-worker.jsrelative to the document URL. When the document is/apps/dashboard, the SW URL is/apps/service-worker.js(parent directory). The scope is/apps/. This means the SW controls ALL requests under/apps/, including app subresources. The fetch handler must be careful not to interfere.@dhis2/pwamessage protocol referenceSource:
@dhis2/pwa/src/lib/constants.jsgetClientsInfoGET_CLIENTS_INFOclientsInfoCLIENTS_INFO{ clientsCount }skipWaitingSKIP_WAITINGclaimClientsCLAIM_CLIENTSdhis2ConnectionStatusUpdateDHIS2_CONNECTION_STATUS_UPDATEgetImmediateDhis2ConnectionStatusUpdateGET_IMMEDIATE_DHIS2_CONNECTION_STATUS_UPDATEstartRecordingSTART_RECORDINGrecordingStartedRECORDING_STARTEDrecordingErrorRECORDING_ERRORconfirmRecordingCompletionCONFIRM_RECORDING_COMPLETIONcompleteRecordingCOMPLETE_RECORDINGrecordingCompletedRECORDING_COMPLETEDRegistration flow on localhost
The
@dhis2/pwaregistration code (registration.js) has a special localhost path:window.addEventListener('load', ...)-- waits for page loadcheckValidSW(swUrl)-- fetchesservice-worker.jsvia XHRnavigator.serviceWorker.ready.then(reg => reg.unregister().then(() => reload()))-- WARNING:navigator.serviceWorker.readynever resolves if no SW is registered, causing the unregister+reload to silently hang (no error, no action)registerValidSW(swUrl)->navigator.serviceWorker.register(swUrl)This is why returning 404 for the SW works on fresh sessions (no pre-existing SW) but can cause issues when an old SW exists (the unregister code hangs on
ready).Recommendation
Current approach (2.43)
Serve the canonical-aware replacement SW when
canonicalAppPathsis ON. This preserves the shell's PWA initialization, caches shell assets for fast reloads, and avoids all conflicts with canonical path routing.Future (global-shell-app /
@dhis2/pwaupdate)When the global-shell-app is updated for canonical paths, the
@dhis2/pwapackage could adopt the canonical-aware SW approach we've already implemented and validated incanonical-service-worker.js. The key changes are documented in this analysis (message protocol,clients.claim()requirement, cache-first for shell assets, no navigation interception). This would allow the backend to stop interceptingservice-worker.jsand let the shell serve its own canonical-aware SW natively.