Merge pull request #6113 from djamaile/master

fix: todo-plugin ignores images and files that are bigger than/equal 200k

.changeset/fair-otters-applaud.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Add an optional `info` parameter to the `readTree` filter option with a `size` property.

.changeset/mean-spiders-run.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-todo-backend': patch
+---
+
+Ignore images and files that are larger than 200KB.

packages/backend-common/api-report.md

@@ -593,7 +593,7 @@ export function useHotMemoize<T>(_module: NodeModule, valueFactory: () => T): T;
 // src/cache/types.d.ts:34:5 - (ae-forgotten-export) The symbol "ClientOptions" needs to be exported by the entry point index.d.ts
 // src/middleware/errorHandler.d.ts:17:26 - (tsdoc-malformed-html-name) Invalid HTML element: A space is not allowed here
 // src/reading/AzureUrlReader.d.ts:9:9 - (ae-forgotten-export) The symbol "ReadTreeResponseFactory" needs to be exported by the entry point index.d.ts
-// src/reading/types.d.ts:106:5 - (ae-forgotten-export) The symbol "ReadTreeResponseDirOptions" needs to be exported by the entry point index.d.ts
+// src/reading/types.d.ts:108:5 - (ae-forgotten-export) The symbol "ReadTreeResponseDirOptions" needs to be exported by the entry point index.d.ts
 // src/service/types.d.ts:12:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
 // src/service/types.d.ts:22:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
 // src/service/types.d.ts:30:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen

packages/backend-common/src/reading/tree/TarArchiveResponse.ts

@@ -43,7 +43,7 @@ export class TarArchiveResponse implements ReadTreeResponse {
     private readonly subPath: string,
     private readonly workDir: string,
     public readonly etag: string,
-    private readonly filter?: (path: string) => boolean,
+    private readonly filter?: (path: string, info: { size: number }) => boolean,
   ) {
     if (subPath) {
       if (!subPath.endsWith('/')) {
@@ -92,7 +92,7 @@ export class TarArchiveResponse implements ReadTreeResponse {
 
       const path = relativePath.slice(this.subPath.length);
       if (this.filter) {
-        if (!this.filter(path)) {
+        if (!this.filter(path, { size: entry.remain })) {
           entry.resume();
           return;
         }
@@ -155,7 +155,7 @@ export class TarArchiveResponse implements ReadTreeResponse {
       tar.extract({
         strip,
         cwd: dir,
-        filter: path => {
+        filter: (path, stat) => {
           // File path relative to the root extracted directory. Will remove the
           // top level dir name from the path since its name is hard to predetermine.
           const relativePath = stripFirstDirectoryFromPath(path);
@@ -164,7 +164,7 @@ export class TarArchiveResponse implements ReadTreeResponse {
           }
           if (this.filter) {
             const innerPath = path.split('/').slice(strip).join('/');
-            return this.filter(innerPath);
+            return this.filter(innerPath, { size: stat.size });
           }
           return true;
         },

packages/backend-common/src/reading/tree/ZipArchiveResponse.ts

@@ -37,7 +37,7 @@ export class ZipArchiveResponse implements ReadTreeResponse {
     private readonly subPath: string,
     private readonly workDir: string,
     public readonly etag: string,
-    private readonly filter?: (path: string) => boolean,
+    private readonly filter?: (path: string, info: { size: number }) => boolean,
   ) {
     if (subPath) {
       if (!subPath.endsWith('/')) {
@@ -75,7 +75,11 @@ export class ZipArchiveResponse implements ReadTreeResponse {
       }
     }
     if (this.filter) {
-      return this.filter(this.getInnerPath(entry.path));
+      return this.filter(this.getInnerPath(entry.path), {
+        size:
+          (entry.vars as { uncompressedSize?: number }).uncompressedSize ??
+          entry.vars.compressedSize,
+      });
     }
     return true;
   }

packages/backend-common/src/reading/types.ts

@@ -104,7 +104,7 @@ export type ReadTreeOptions = {
    *
    * If no filter is provided all files are extracted.
    */
-  filter?(path: string): boolean;
+  filter?(path: string, info?: { size: number }): boolean;
 
   /**
    * An etag can be provided to check whether readTree's response has changed from a previous execution.
@@ -164,7 +164,7 @@ export type FromArchiveOptions = {
   // etag of the blob
   etag: string;
   // Filter passed on from the ReadTreeOptions
-  filter?: (path: string) => boolean;
+  filter?: (path: string, info?: { size: number }) => boolean;
 };
 
 export interface ReadTreeResponseFactory {

plugins/todo-backend/src/lib/TodoReader/TodoScmReader.test.ts

@@ -74,7 +74,13 @@ describe('TodoScmReader', () => {
       ],
     };
 
-    await expect(todoReader.readTodos({ url })).resolves.toEqual(expected);
+    // These two reads should only result in a single call to readTree
+    await expect(
+      Promise.all([
+        todoReader.readTodos({ url }),
+        todoReader.readTodos({ url }),
+      ]),
+    ).resolves.toEqual([expected, expected]);
 
     expect(reader.readTree).toHaveBeenCalledTimes(1);
     expect(reader.readTree).toHaveBeenCalledWith(

plugins/todo-backend/src/lib/TodoReader/TodoScmReader.ts

@@ -27,6 +27,19 @@ import {
 } from './types';
 import { Config } from '@backstage/config';
 import { createTodoParser } from './createTodoParser';
+import path from 'path';
+
+const excludedExtensions = [
+  '.png',
+  '.svg',
+  '.jpg',
+  '.jpeg',
+  '.gif',
+  '.raw',
+  '.lock',
+  '.ico',
+];
+const MAX_FILE_SIZE = 200000;
 
 type Options = {
   logger: Logger;
@@ -47,6 +60,7 @@ export class TodoScmReader implements TodoReader {
   private readonly integrations: ScmIntegrations;
 
   private readonly cache = new Map<string, CacheItem>();
+  private readonly inFlightReads = new Map<string, Promise<CacheItem>>();
 
   static fromConfig(config: Config, options: Omit<Options, 'integrations'>) {
     return new TodoScmReader({
@@ -63,16 +77,26 @@ export class TodoScmReader implements TodoReader {
   }
 
   async readTodos({ url }: ReadTodosOptions): Promise<ReadTodosResult> {
+    const inFlightRead = this.inFlightReads.get(url);
+    if (inFlightRead) {
+      return inFlightRead.then(read => read.result);
+    }
+
     const cacheItem = this.cache.get(url);
-    try {
-      const newCacheItem = await this.doReadTodos({ url }, cacheItem?.etag);
-      this.cache.set(url, newCacheItem);
-      return newCacheItem.result;
-    } catch (error) {
+    const newRead = this.doReadTodos({ url }, cacheItem?.etag).catch(error => {
       if (cacheItem && error.name === 'NotModifiedError') {
-        return cacheItem.result;
+        return cacheItem;
       }
       throw error;
+    });
+
+    this.inFlightReads.set(url, newRead);
+    try {
+      const newCacheItem = await newRead;
+      this.cache.set(url, newCacheItem);
+      return newCacheItem.result;
+    } finally {
+      this.inFlightReads.delete(url);
     }
   }
 
@@ -82,8 +106,16 @@ export class TodoScmReader implements TodoReader {
   ): Promise<CacheItem> {
     const tree = await this.reader.readTree(url, {
       etag,
-      filter(path) {
-        return !path.startsWith('.') && !path.includes('/.');
+      filter(filePath, info) {
+        const extname = path.extname(filePath);
+        if (info && info.size > MAX_FILE_SIZE) {
+          return false;
+        }
+        return (
+          !filePath.startsWith('.') &&
+          !filePath.includes('/.') &&
+          !excludedExtensions.includes(extname)
+        );
       },
     });