From 3b7d4ebbd6445364889d61b7b1b1c65206534d34 Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Tue, 27 Apr 2021 22:50:12 +0300
Subject: [PATCH] fix(docs-infra): do not redirect disambiguated URLs (#41842)

In #41788, logic was added to disambiguate case-insensitively equal docs
paths/URLs. This process includes appending a `-\d+` suffix to some
paths/URLs (for example, `/.../inject-1`). Unfortunately, some of the
Firebase redirects configured in `firebase.json` would match these URLs
and redirect them to non-existing paths.
Example failures: [stable][1], [next][2]

NOTE:
This was not picked up in the regular CI tests run for PRs, because the
local devserver and the preview server used to test PRs do not support
Firebase-like redirects.

This commit fixes this by ensuring these disambiguated paths/URLs are
not matched by the redirect rules by checking whether the part of the
suffix after the `-` contains any numeric digits. While this check is
not ideal, it should be good enough for our purpose, since the legacy
URLs that we do want to redirect contain suffixes such as `-class`,
`-function` and thus no numeric digits.

[1]: https://circleci.com/gh/angular/angular/974345
[2]: https://circleci.com/gh/angular/angular/974346

PR Close #41842
---
 aio/firebase.json                                | 11 ++++++-----
 aio/ngsw-config.json                             |  2 +-
 .../unit/testFirebaseRedirection.spec.ts         | 16 ++++++++++++++++
 .../unit/testServiceWorkerRoutes.spec.ts         | 14 ++++++++++++++
 4 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/aio/firebase.json b/aio/firebase.json
index 3fa9c39bd8..bbf7b2220a 100644
--- a/aio/firebase.json
+++ b/aio/firebase.json
@@ -92,11 +92,12 @@
       {"type": 301, "source": "/**/HTTP_PROVIDERS*", "destination": "/api/http/HttpModule"},
 
       // URLs that use the old scheme of adding the type to the end (e.g. `SomeClass-class`)
-      {"type": 301, "source": "/api/:package/:api-*", "destination": "/api/:package/:api"},
-      {"type": 301, "source": "/api/:package/testing/index/:api-*", "destination": "/api/:package/testing/:api"},
-      {"type": 301, "source": "/api/:package/testing/:api-*", "destination": "/api/:package/testing/:api"},
-      {"type": 301, "source": "/api/upgrade/:package/index/:api-*", "destination": "/api/upgrade/:package/:api"},
-      {"type": 301, "source": "/api/upgrade/:package/:api-*", "destination": "/api/upgrade/:package/:api"},
+      // (Exclude disambiguated URLs that might be suffixed with `-\d+` (e.g. `SomeClass-1`))
+      {"type": 301, "regex": "^/api/(?P<package>[^/]+)/(?P<api>[^/]+)-\\D*$", "destination": "/api/:package/:api"},
+      {"type": 301, "regex": "^/api/(?P<package>[^/]+)/testing/index/(?P<api>[^/]+)$", "destination": "/api/:package/testing/:api"},
+      {"type": 301, "regex": "^/api/(?P<package>[^/]+)/testing/(?P<api>[^/]+)-\\D*$", "destination": "/api/:package/testing/:api"},
+      {"type": 301, "regex": "^/api/upgrade/(?P<package>[^/]+)/index/(?P<api>[^/]+)$", "destination": "/api/upgrade/:package/:api"},
+      {"type": 301, "regex": "^/api/upgrade/(?P<package>[^/]+)/(?P<api>[^/]+)-\\D*$", "destination": "/api/upgrade/:package/:api"},
 
       // URLs that use the old scheme before we moved the docs to the angular/angular repo
       {"type": 301, "source": "/docs/*/latest", "destination": "/docs"},
diff --git a/aio/ngsw-config.json b/aio/ngsw-config.json
index ce6c12bfaf..1231b6dece 100644
--- a/aio/ngsw-config.json
+++ b/aio/ngsw-config.json
@@ -76,7 +76,7 @@
     "!/**/*__*/**",
     "!/**/stackblitz",
     "!/**/stackblitz.html",
-    "!/api/*/**/*-*",
+    "!/api/*/**/*-\\D{0,}",
     "!/api/**/AnimationStateDeclarationMetadata*",
     "!/api/**/CORE_DIRECTIVES*",
     "!/api/**/DirectiveMetadata*",
diff --git a/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts b/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts
index 9ac74675f2..7623227ae7 100644
--- a/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts
+++ b/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts
@@ -33,4 +33,20 @@ describe('firebase.json redirect config', () => {
       });
     });
   });
+
+  it('should not redirect disambiguated URLs', () => {
+    const redirector = getRedirector();
+
+    // Disambiguated URL.
+    const url1 = '/api/core/Foo-0';
+    expect(redirector.redirect(url1)).toBe(url1);
+
+    // Disambiguated URL.
+    const url2 = '/api/core/BAR-1337';
+    expect(redirector.redirect(url2)).toBe(url2);
+
+    // Non-disambiguated URL with dash.
+    const url3 = '/api/core/baz-class';
+    expect(redirector.redirect(url3)).toBe('/api/core/baz');
+  });
 });
diff --git a/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts b/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts
index 98d37fb007..41fc8429ee 100644
--- a/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts
+++ b/aio/tests/deployment/unit/testServiceWorkerRoutes.spec.ts
@@ -35,4 +35,18 @@ describe('ServiceWorker navigation URLs', () => {
     navigationUrls.forEach(url => expect(isNavigationUrl(url)).toBeTruthy(url));
     nonNavigationUrls.forEach(url => expect(isNavigationUrl(url)).toBeFalsy(url));
   });
+
+  it('should treat disambiguated URLs as navigation URLs', () => {
+    // Disambiguated URL.
+    const url1 = '/api/core/Foo-0';
+    expect(isNavigationUrl(url1)).toBeTruthy(url1);
+
+    // Disambiguated URL.
+    const url2 = '/api/core/BAR-1337';
+    expect(isNavigationUrl(url2)).toBeTruthy(url2);
+
+    // Non-disambiguated URL with dash.
+    const url3 = '/api/core/baz-class';
+    expect(isNavigationUrl(url3)).toBeFalsy(url3);
+  });
 });