Merge pull request #3283 from backstage/rugvip/retree

backend-common: refactor readTree to only accept URL and options + add response helper

plugins/catalog-backend/src/ingestion/processors/CodeOwnersProcessor.test.ts

@@ -157,14 +157,14 @@ describe('CodeOwnersProcessor', () => {
       const read = jest
         .fn()
         .mockResolvedValue(mockReadResult({ data: ownersText }));
-      const reader = { read };
+      const reader = { read, readTree: jest.fn() };
       const result = await findRawCodeOwners(mockLocation(), reader);
       expect(result).toEqual(ownersText);
     });
 
     it('should raise error when no codeowner', async () => {
       const read = jest.fn().mockRejectedValue(mockReadResult());
-      const reader = { read };
+      const reader = { read, readTree: jest.fn() };
 
       await expect(
         findRawCodeOwners(mockLocation(), reader),
@@ -178,7 +178,7 @@ describe('CodeOwnersProcessor', () => {
         .mockImplementationOnce(() => mockReadResult({ error: 'foo' }))
         .mockImplementationOnce(() => mockReadResult({ error: 'bar' }))
         .mockResolvedValue(mockReadResult({ data: ownersText }));
-      const reader = { read };
+      const reader = { read, readTree: jest.fn() };
 
       const result = await findRawCodeOwners(mockLocation(), reader);
 
@@ -197,7 +197,7 @@ describe('CodeOwnersProcessor', () => {
       const read = jest
         .fn()
         .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() }));
-      const reader = { read };
+      const reader = { read, readTree: jest.fn() };
 
       const owner = await resolveCodeOwner(mockLocation(), reader);
       expect(owner).toBe('backstage-core');
@@ -207,7 +207,7 @@ describe('CodeOwnersProcessor', () => {
       const read = jest
         .fn()
         .mockImplementation(() => mockReadResult({ error: 'error: foo' }));
-      const reader = { read };
+      const reader = { read, readTree: jest.fn() };
 
       await expect(
         resolveCodeOwner(mockLocation(), reader),
@@ -221,7 +221,7 @@ describe('CodeOwnersProcessor', () => {
       const read = jest
         .fn()
         .mockResolvedValue(mockReadResult({ data: mockCodeOwnersText() }));
-      const reader = { read };
+      const reader = { read, readTree: jest.fn() };
       const processor = new CodeOwnersProcessor({ reader });
 
       return { entity, processor, read };

plugins/catalog-backend/src/ingestion/processors/PlaceholderProcessor.test.ts

@@ -27,7 +27,7 @@ import {
 
 describe('PlaceholderProcessor', () => {
   const read: jest.MockedFunction<ResolverRead> = jest.fn();
-  const reader: UrlReader = { read };
+  const reader: UrlReader = { read, readTree: jest.fn() };
 
   beforeEach(() => {
     jest.resetAllMocks();

plugins/catalog-backend/src/service/CatalogBuilder.test.ts

@@ -40,7 +40,10 @@ const dummyEntityYaml = yaml.stringify(dummyEntity);
 
 describe('CatalogBuilder', () => {
   let db: Knex<any, unknown[]>;
-  const reader: jest.Mocked<UrlReader> = { read: jest.fn() };
+  const reader: jest.Mocked<UrlReader> = {
+    read: jest.fn(),
+    readTree: jest.fn(),
+  };
   const env: CatalogEnvironment = {
     logger: getVoidLogger(),
     database: { getClient: async () => db },

plugins/techdocs-backend/src/helpers.test.ts

@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 
+import { Readable } from 'stream';
 import { getDocFilesFromRepository } from './helpers';
 import { UrlReader, ReadTreeResponse } from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
 
 describe('getDocFilesFromRepository', () => {
-  it('should take the directory from UrlReader.readTree and add the docs path when mkdocs.yml is in root', async () => {
+  it('should read a remote directory using UrlReader.readTree', async () => {
     class MockUrlReader implements UrlReader {
       async read() {
         return Buffer.from('mock');
@@ -30,11 +31,11 @@ describe('getDocFilesFromRepository', () => {
           dir: async () => {
             return '/tmp/testfolder';
           },
-          files: () => {
+          files: async () => {
             return [];
           },
           archive: async () => {
-            return Buffer.from('');
+            return Readable.from('');
           },
         };
       }
@@ -45,7 +46,7 @@ describe('getDocFilesFromRepository', () => {
         namespace: 'default',
         annotations: {
           'backstage.io/techdocs-ref':
-            'url:https://github.com/backstage/backstage/blob/master/mkdocs.yml',
+            'url:https://github.com/backstage/backstage/blob/master/subfolder/',
         },
         name: 'mytestcomponent',
         description: 'A component for testing',
@@ -64,54 +65,6 @@ describe('getDocFilesFromRepository', () => {
       mockEntity,
     );
 
-    expect(output).toBe('/tmp/testfolder/.');
-  });
-
-  it('should take the directory from UrlReader.readTree and add the docs path when mkdocs.yml is in a subfolder', async () => {
-    class MockUrlReader implements UrlReader {
-      async read() {
-        return Buffer.from('mock');
-      }
-
-      async readTree(): Promise<ReadTreeResponse> {
-        return {
-          dir: async () => {
-            return '/tmp/testfolder';
-          },
-          files: () => {
-            return [];
-          },
-          archive: async () => {
-            return Buffer.from('');
-          },
-        };
-      }
-    }
-
-    const mockEntity: Entity = {
-      metadata: {
-        namespace: 'default',
-        annotations: {
-          'backstage.io/techdocs-ref':
-            'url:https://github.com/backstage/backstage/blob/master/subfolder/mkdocs.yml',
-        },
-        name: 'mytestcomponent',
-        description: 'A component for testing',
-      },
-      apiVersion: 'backstage.io/v1alpha1',
-      kind: 'Component',
-      spec: {
-        type: 'documentation',
-        lifecycle: 'experimental',
-        owner: 'testuser',
-      },
-    };
-
-    const output = await getDocFilesFromRepository(
-      new MockUrlReader(),
-      mockEntity,
-    );
-
-    expect(output).toBe('/tmp/testfolder/subfolder');
+    expect(output).toBe('/tmp/testfolder');
   });
 });

plugins/techdocs-backend/src/helpers.ts

@@ -179,24 +179,7 @@ export const getDocFilesFromRepository = async (
     entity,
   );
 
-  const { ref, filepath: mkdocsPath } = parseGitUrl(target);
+  const response = await reader.readTree(target);
 
-  const docsRootPath = path.dirname(mkdocsPath);
-  const docsFolderPath = path.join(docsRootPath, 'docs');
-
-  if (reader.readTree) {
-    const readTreeResponse = await reader.readTree(
-      parseGitUrl(target).toString(),
-      ref,
-      [mkdocsPath, docsFolderPath],
-    );
-
-    const tmpDir = await readTreeResponse.dir();
-
-    return `${tmpDir}/${docsRootPath}`;
-  }
-
-  throw new Error(
-    `No readTree method available on the UrlReader for ${target}`,
-  );
+  return await response.dir();
 };