From b652ebc4e7e276d963804f3d1a6e5c98c81e0a62 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sun, 8 Nov 2020 14:39:07 +0100
Subject: [PATCH] config-loader: add visibility filtering tests + fix filtering
 logic

---
 .../src/lib/schema/filtering.test.ts          | 105 ++++++++++++++++++
 .../config-loader/src/lib/schema/filtering.ts |  23 ++--
 2 files changed, 120 insertions(+), 8 deletions(-)
 create mode 100644 packages/config-loader/src/lib/schema/filtering.test.ts

diff --git a/packages/config-loader/src/lib/schema/filtering.test.ts b/packages/config-loader/src/lib/schema/filtering.test.ts
new file mode 100644
index 0000000000..8cc907417b
--- /dev/null
+++ b/packages/config-loader/src/lib/schema/filtering.test.ts
@@ -0,0 +1,105 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * 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 { JsonObject } from '@backstage/config';
+import { ConfigVisibility } from './types';
+import { filterByVisibility } from './filtering';
+
+const data = {
+  arr: ['f', 'b', 's'],
+  objArr: [
+    { f: 1, b: 2, s: 3 },
+    { f: 4, b: 5, s: 6 },
+  ],
+  obj: {
+    f: 'a',
+    b: {
+      s: true,
+    },
+  },
+  arrF: [{ never: 'here' }],
+  arrB: [{ never: 'here' }],
+  arrS: [{ never: 'here' }],
+  objF: { never: 'here' },
+  objB: { never: 'here' },
+  objS: { never: 'here' },
+};
+
+const visibilities = new Map<string, ConfigVisibility>(
+  Object.entries({
+    '/arr/0': 'frontend',
+    '/arr/1': 'backend',
+    '/arr/2': 'secret',
+    '/obj/f': 'frontend',
+    '/obj/b': 'backend',
+    '/obj/b/s': 'secret',
+    '/objArr/0/f': 'frontend',
+    '/objArr/0/b': 'backend',
+    '/objArr/0/s': 'secret',
+    '/objArr/1/f': 'frontend',
+    '/objArr/1/b': 'backend',
+    '/objArr/1/s': 'secret',
+    '/arrF': 'frontend',
+    '/arrB': 'backend',
+    '/arrS': 'secret',
+    '/objF': 'frontend',
+    '/objB': 'backend',
+    '/objS': 'secret',
+  }),
+);
+
+describe('filterByVisibility', () => {
+  test.each<[ConfigVisibility[], JsonObject]>([
+    [[], {}],
+    [
+      ['frontend'],
+      {
+        arr: ['f'],
+        objArr: [{ f: 1 }, { f: 4 }],
+        obj: { f: 'a' },
+        arrF: [],
+        objF: {},
+      },
+    ],
+    [
+      ['backend'],
+      {
+        arr: ['b'],
+        objArr: [{ b: 2 }, { b: 5 }],
+        obj: { b: {} },
+        arrF: [{ never: 'here' }],
+        arrB: [{ never: 'here' }],
+        arrS: [{ never: 'here' }],
+        objF: { never: 'here' },
+        objB: { never: 'here' },
+        objS: { never: 'here' },
+      },
+    ],
+    [
+      ['secret'],
+      {
+        arr: ['s'],
+        objArr: [{ s: 3 }, { s: 6 }],
+        obj: { b: { s: true } },
+        arrS: [],
+        objS: {},
+      },
+    ],
+    [['frontend', 'backend', 'secret'], data],
+  ])('should filter correctly with %p', (filter, expected) => {
+    expect(filterByVisibility(data, filter, visibilities)).toEqual(expected);
+  });
+});
diff --git a/packages/config-loader/src/lib/schema/filtering.ts b/packages/config-loader/src/lib/schema/filtering.ts
index 64ccfa5e24..3ed4c958b8 100644
--- a/packages/config-loader/src/lib/schema/filtering.ts
+++ b/packages/config-loader/src/lib/schema/filtering.ts
@@ -23,13 +23,12 @@ export function filterByVisibility(
   visibilityByPath: Map<string, ConfigVisibility>,
 ): JsonObject {
   function transform(jsonVal: JsonValue, path: string): JsonValue | undefined {
+    const isVisible = includeVisibilities.includes(
+      visibilityByPath.get(path) ?? DEFAULT_CONFIG_VISIBILITY,
+    );
+
     if (typeof jsonVal !== 'object') {
-      const visibility =
-        visibilityByPath.get(path) ?? DEFAULT_CONFIG_VISIBILITY;
-      if (includeVisibilities.includes(visibility)) {
-        return jsonVal;
-      }
-      return undefined;
+      return isVisible ? jsonVal : undefined;
     } else if (jsonVal === null) {
       return undefined;
     } else if (Array.isArray(jsonVal)) {
@@ -42,10 +41,14 @@ export function filterByVisibility(
         }
       }
 
-      return arr.length === 0 ? undefined : arr;
+      if (arr.length > 0 || isVisible) {
+        return arr;
+      }
+      return undefined;
     }
 
     const outObj: JsonObject = {};
+    let hasOutput = false;
 
     for (const [key, value] of Object.entries(jsonVal)) {
       if (value === undefined) {
@@ -54,10 +57,14 @@ export function filterByVisibility(
       const out = transform(value, `${path}/${key}`);
       if (out !== undefined) {
         outObj[key] = out;
+        hasOutput = true;
       }
     }
 
-    return Object.keys(outObj).length === 0 ? undefined : outObj;
+    if (hasOutput || isVisible) {
+      return outObj;
+    }
+    return undefined;
   }
 
   return (transform(data, '') as JsonObject) ?? {};