From b5369fa23a9d77f28029b3da8de9aa5cba57e0e3 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Mon, 3 Apr 2023 12:02:45 +0200
Subject: [PATCH] config-loader: RemoteConfigSource test + fix

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
---
 packages/config-loader/package.json           |  1 +
 .../src/sources/RemoteConfigSource.test.ts    | 85 +++++++++++++++++++
 .../src/sources/RemoteConfigSource.ts         | 13 +--
 yarn.lock                                     |  1 +
 4 files changed, 91 insertions(+), 9 deletions(-)
 create mode 100644 packages/config-loader/src/sources/RemoteConfigSource.test.ts

diff --git a/packages/config-loader/package.json b/packages/config-loader/package.json
index 0397dc89eb..42886dc597 100644
--- a/packages/config-loader/package.json
+++ b/packages/config-loader/package.json
@@ -52,6 +52,7 @@
     "yup": "^0.32.9"
   },
   "devDependencies": {
+    "@backstage/backend-test-utils": "workspace:^",
     "@backstage/cli": "workspace:^",
     "@types/json-schema-merge-allof": "^0.6.0",
     "@types/mock-fs": "^4.10.0",
diff --git a/packages/config-loader/src/sources/RemoteConfigSource.test.ts b/packages/config-loader/src/sources/RemoteConfigSource.test.ts
new file mode 100644
index 0000000000..c62e33abc9
--- /dev/null
+++ b/packages/config-loader/src/sources/RemoteConfigSource.test.ts
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2023 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { rest } from 'msw';
+import { setupRequestMockHandlers } from '@backstage/backend-test-utils';
+import { setupServer } from 'msw/node';
+import { RemoteConfigSource } from './RemoteConfigSource';
+import { readN } from './__testUtils__/testUtils';
+
+describe('RemoteConfigSource', () => {
+  const worker = setupServer();
+  setupRequestMockHandlers(worker);
+
+  it('should load config from a remote URL', async () => {
+    worker.use(
+      rest.get('http://localhost/config.yaml', (_req, res, ctx) =>
+        res(
+          ctx.body(`
+app:
+  title: Example App
+  substituted: \${VALUE}
+  escaped: \$\${VALUE}
+`),
+        ),
+      ),
+    );
+
+    const source = RemoteConfigSource.create({
+      url: 'http://localhost/config.yaml',
+      substitutionFunc: async () => 'x',
+    });
+
+    await expect(readN(source, 1)).resolves.toEqual([
+      [
+        {
+          context: 'http://localhost/config.yaml',
+          data: {
+            app: {
+              title: 'Example App',
+              substituted: 'x',
+              escaped: '${VALUE}',
+            },
+          },
+        },
+      ],
+    ]);
+  });
+
+  it('should reload config from a remote URL', async () => {
+    let fetched = false;
+
+    worker.use(
+      rest.get('http://localhost/config.yaml', (_req, res, ctx) => {
+        if (!fetched) {
+          fetched = true;
+          return res(ctx.body('x: 1'));
+        }
+        return res(ctx.body('x: 2'));
+      }),
+    );
+
+    const source = RemoteConfigSource.create({
+      url: 'http://localhost/config.yaml',
+      reloadIntervalSeconds: 0,
+    });
+
+    await expect(readN(source, 2)).resolves.toEqual([
+      [{ context: 'http://localhost/config.yaml', data: { x: 1 } }],
+      [{ context: 'http://localhost/config.yaml', data: { x: 2 } }],
+    ]);
+  });
+});
diff --git a/packages/config-loader/src/sources/RemoteConfigSource.ts b/packages/config-loader/src/sources/RemoteConfigSource.ts
index 64e3c082ad..4d2e03cd01 100644
--- a/packages/config-loader/src/sources/RemoteConfigSource.ts
+++ b/packages/config-loader/src/sources/RemoteConfigSource.ts
@@ -101,7 +101,8 @@ export class RemoteConfigSource implements ConfigSource {
       if (options?.signal?.aborted) {
         return;
       }
-      const loadStart = Date.now();
+
+      await this.#wait(options?.signal);
 
       try {
         const newData = await this.#load(options?.signal);
@@ -112,9 +113,6 @@ export class RemoteConfigSource implements ConfigSource {
       } catch (error) {
         console.error(`Failed to read config from ${this.#url}, ${error}`);
       }
-      const loadTime = Date.now() - loadStart;
-
-      await this.#wait(loadTime, options?.signal);
     }
   }
 
@@ -140,12 +138,9 @@ export class RemoteConfigSource implements ConfigSource {
     return data;
   }
 
-  async #wait(loadTimeMs: number, signal?: AbortSignal) {
+  async #wait(signal?: AbortSignal) {
     return new Promise<void>(resolve => {
-      const timeoutId = setTimeout(
-        onDone,
-        Math.max(0, this.#reloadIntervalSeconds * 1000 - loadTimeMs),
-      );
+      const timeoutId = setTimeout(onDone, this.#reloadIntervalSeconds * 1000);
       signal?.addEventListener('abort', onDone);
 
       function onDone() {
diff --git a/yarn.lock b/yarn.lock
index a6188d8c6d..c35f795817 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3923,6 +3923,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "@backstage/config-loader@workspace:packages/config-loader"
   dependencies:
+    "@backstage/backend-test-utils": "workspace:^"
     "@backstage/cli": "workspace:^"
     "@backstage/cli-common": "workspace:^"
     "@backstage/config": "workspace:^"