Merge pull request #11197 from zeeraw/gitlab-timestamps

Do not send last_activity_after timestamp to gitlab api when the cache is empty

.changeset/long-spiders-wave.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-backend-module-gitlab': patch
+---
+
+The `last_activity_after` timestamp is now being omitted when querying the GitLab API for the first time.

plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.test.ts

@@ -17,7 +17,7 @@
 import { getVoidLogger } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
 import { LocationSpec } from '@backstage/plugin-catalog-backend';
-import { rest } from 'msw';
+import { rest, RestRequest } from 'msw';
 import { setupServer } from 'msw/node';
 import { GitLabDiscoveryProcessor, parseUrl } from './GitLabDiscoveryProcessor';
 import { GitLabProject } from './lib';
@@ -48,6 +48,7 @@ const GROUP_LOCATION_CUSTOM_BRANCH: LocationSpec = {
   type: 'gitlab-discovery',
   target: `${SERVER_URL}/group/subgroup/blob/test/catalog-info.yaml`,
 };
+const SERVER_TIME = '2001-01-01T12:34:56.000Z';
 
 function setupFakeServer(
   url: string,
@@ -58,9 +59,15 @@ function setupFakeServer(
     data: GitLabProject[];
     nextPage?: number;
   },
+  assertion?: (r: RestRequest) => any,
 ) {
   server.use(
     rest.get(url, (req, res, ctx) => {
+      // Send the request to the assertion to give the test an opportunity to inspect the parameters.
+      if (assertion !== undefined) {
+        assertion(req);
+      }
+
       if (req.headers.get('private-token') !== 'test-token') {
         return res(ctx.status(401), ctx.json({}));
       }
@@ -145,7 +152,7 @@ describe('GitlabDiscoveryProcessor', () => {
   beforeAll(() => {
     server.listen();
     jest.useFakeTimers('modern');
-    jest.setSystemTime(new Date('2001-01-01T12:34:56Z'));
+    jest.setSystemTime(new Date(SERVER_TIME));
   });
   afterEach(() => server.resetHandlers());
   afterAll(() => {
@@ -394,34 +401,43 @@ describe('GitlabDiscoveryProcessor', () => {
     });
 
     it('uses the previous scan timestamp to filter', async () => {
+      const payload = {
+        data: [
+          {
+            id: 1,
+            archived: false,
+            default_branch: 'main',
+            last_activity_at: '2000-01-01T00:00:00Z',
+            web_url: 'https://gitlab.fake/1',
+          },
+          {
+            id: 2,
+            archived: false,
+            default_branch: 'main',
+            last_activity_at: '2002-01-01T00:00:00Z',
+            web_url: 'https://gitlab.fake/2',
+          },
+        ],
+      };
       const processor = getProcessor();
-      setupFakeServer(PROJECTS_URL, request => {
-        switch (request.page) {
-          case 1:
-            return {
-              data: [
-                {
-                  id: 1,
-                  archived: false,
-                  default_branch: 'main',
-                  last_activity_at: '2000-01-01T00:00:00Z',
-                  web_url: 'https://gitlab.fake/1',
-                  path_with_namespace: '1',
-                },
-                {
-                  id: 2,
-                  archived: false,
-                  default_branch: 'main',
-                  last_activity_at: '2002-01-01T00:00:00Z',
-                  web_url: 'https://gitlab.fake/2',
-                  path_with_namespace: '2',
-                },
-              ],
-            };
-          default:
-            throw new Error('Invalid request');
-        }
-      });
+
+      setupFakeServer(
+        PROJECTS_URL,
+        request => {
+          switch (request.page) {
+            case 1:
+              return payload;
+            default:
+              throw new Error('Invalid request');
+          }
+        },
+        request => {
+          // We assert that the last activity timestamp is not being sent to the GitLab API as we expect the cache to be empty.
+          expect(
+            request.url.searchParams.get('last_activity_after'),
+          ).toBeNull();
+        },
+      );
 
       const result: any[] = [];
 
@@ -433,6 +449,18 @@ describe('GitlabDiscoveryProcessor', () => {
 
       // Second scan should have used the mocked Date to set the last scanned time to 2001
       // This should result in only the second repo being scanned, since that has a timestamp of 2002
+      setupFakeServer(
+        PROJECTS_URL,
+        _ => {
+          return payload;
+        },
+        request => {
+          // We assert that the last activity timestamp is being sent to the GitLab API since we expect it to be in the cache.
+          expect(request.url.searchParams.get('last_activity_after')).toMatch(
+            SERVER_TIME,
+          );
+        },
+      );
       const result2: any[] = [];
       await processor.readLocation(PROJECT_LOCATION, false, e => {
         result2.push(e);

plugins/catalog-backend-module-gitlab/src/GitLabDiscoveryProcessor.ts

@@ -84,6 +84,7 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor {
       return false;
     }
 
+    const startTime = new Date();
     const { group, host, branch, catalogPath } = parseUrl(location.target);
 
     const integration = this.integrations.gitlab.byUrl(`https://${host}`);
@@ -97,14 +98,18 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor {
       config: integration.config,
       logger: this.logger,
     });
-    const startTimestamp = Date.now();
     this.logger.debug(`Reading GitLab projects from ${location.target}`);
 
-    const projects = paginated(options => client.listProjects(options), {
+    const lastActivity = (await this.cache.get(this.getCacheKey())) as string;
+    const opts = {
       group,
-      last_activity_after: await this.updateLastActivity(),
       page: 1,
-    });
+      // We check for the existence of lastActivity and only set it if it's present to ensure
+      // that the options doesn't include the key so that the API doesn't receive an empty query parameter.
+      ...(lastActivity && { last_activity_after: lastActivity }),
+    };
+
+    const projects = paginated(options => client.listProjects(options), opts);
 
     const res: Result = {
       scanned: 0,
@@ -156,7 +161,10 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor {
       );
     }
 
-    const duration = ((Date.now() - startTimestamp) / 1000).toFixed(1);
+    // Save an ISO formatted string in the cache as that's what GitLab expects in the API request.
+    await this.cache.set(this.getCacheKey(), startTime.toISOString());
+
+    const duration = ((Date.now() - startTime.getTime()) / 1000).toFixed(1);
     this.logger.debug(
       `Read ${res.scanned} GitLab repositories in ${duration} seconds`,
     );
@@ -164,11 +172,8 @@ export class GitLabDiscoveryProcessor implements CatalogProcessor {
     return true;
   }
 
-  private async updateLastActivity(): Promise<string | undefined> {
-    const cacheKey = `processors/${this.getProcessorName()}/last-activity`;
-    const lastActivity = await this.cache.get(cacheKey);
-    await this.cache.set(cacheKey, new Date().toISOString());
-    return lastActivity as string | undefined;
+  private getCacheKey(): string {
+    return `processors/${this.getProcessorName()}/last-activity`;
   }
 }