Merge pull request #25771 from rmdp-onefootball/plugin-search-backend-module-stack-overflow-collator-call-issue

Plugin search backend module StackOverflow collator call issue

.changeset/quiet-deers-visit.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-search-backend-module-stack-overflow-collator': minor
+---
+
+Always set default request parameters for requests to stackoverflow while allow to overwrite them. Remove site parameter as causing the request to fail.

plugins/search-backend-module-stack-overflow-collator/src/collators/StackOverflowQuestionsCollatorFactory.test.ts

@@ -25,10 +25,12 @@ import { TestPipeline } from '@backstage/plugin-search-backend-node';
 import { ConfigReader } from '@backstage/config';
 import { Readable } from 'stream';
 import { setupServer } from 'msw/node';
-import { rest } from 'msw';
+import { rest, RestRequest } from 'msw';
 
 const logger = mockServices.logger.mock();
 
+const BASE_URL = 'https://api.stackexchange.com/2.3';
+
 const mockQuestion = {
   items: [
     {
@@ -68,27 +70,303 @@ const mockOverrideQuestion = {
   has_more: false,
 };
 
-describe('StackOverflowQuestionsCollatorFactory', () => {
+const testSearchQuery = (
+  request: RestRequest | undefined,
+  expectedSearch: unknown,
+) => {
+  if (!request) {
+    expect(request).not.toBeFalsy();
+    return;
+  }
+
+  const executedSearch: { [key: string]: string } = {};
+  request.url.searchParams.forEach((value: string, key: string) => {
+    executedSearch[key] = value;
+  });
+  expect(executedSearch).toEqual(expectedSearch);
+};
+
+describe('StackOverflowQuestionsCollatorFactory using custom request params', () => {
   const config = new ConfigReader({
     stackoverflow: {
       baseUrl: 'http://stack.overflow.local',
     },
   });
 
-  const defaultOptions: StackOverflowQuestionsCollatorFactoryOptions = {
-    logger,
-    requestParams: {
-      tagged: ['developer-portal'],
-      pagesize: 100,
-      order: 'desc',
-      sort: 'activity',
-    },
-  };
+  const optionsWithCustomRequestParams: StackOverflowQuestionsCollatorFactoryOptions =
+    {
+      logger,
+      requestParams: {
+        tagged: ['developer-portal'],
+        pagesize: 100,
+        order: 'desc',
+        sort: 'activity',
+      },
+    };
 
   it('has expected type', () => {
     const factory = StackOverflowQuestionsCollatorFactory.fromConfig(
       config,
-      defaultOptions,
+      optionsWithCustomRequestParams,
+    );
+    expect(factory.type).toBe('stack-overflow');
+  });
+
+  describe('Manage site query parameter', () => {
+    const worker = setupServer();
+    registerMswTestHooks(worker);
+
+    it('uses site query parameter when provided and baseUrl is not default', async () => {
+      let request;
+      worker.use(
+        rest.get('http://stack.overflow.local/questions', (req, res, ctx) => {
+          request = req;
+
+          return res(ctx.status(200), ctx.json(mockQuestion));
+        }),
+      );
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
+        logger,
+        baseUrl: 'http://stack.overflow.local',
+        requestParams: {
+          site: 'stackoverflow',
+        },
+      });
+
+      const collator = await factory.getCollator();
+      const pipeline = TestPipeline.fromCollator(collator);
+      const { documents } = await pipeline.execute();
+      const expectedSearch = {
+        order: 'desc',
+        site: 'stackoverflow',
+        sort: 'activity',
+        page: '1',
+      };
+      testSearchQuery(request, expectedSearch);
+      expect(documents).toHaveLength(mockQuestion.items.length);
+    });
+
+    it('uses site query parameter when provided and baseUrl is default', async () => {
+      let request;
+      worker.use(
+        rest.get(`${BASE_URL}/questions`, (req, res, ctx) => {
+          request = req;
+
+          return res(ctx.status(200), ctx.json(mockQuestion));
+        }),
+      );
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
+        logger,
+        baseUrl: BASE_URL,
+        requestParams: {
+          site: 'foo_bar_baz',
+        },
+      });
+
+      const collator = await factory.getCollator();
+      const pipeline = TestPipeline.fromCollator(collator);
+      const { documents } = await pipeline.execute();
+      const expectedSearch = {
+        order: 'desc',
+        site: 'foo_bar_baz',
+        sort: 'activity',
+        page: '1',
+      };
+      testSearchQuery(request, expectedSearch);
+      expect(documents).toHaveLength(mockQuestion.items.length);
+    });
+
+    it('uses default when site is not provided and baseUrl is default', async () => {
+      let request;
+      worker.use(
+        rest.get(`${BASE_URL}/questions`, (req, res, ctx) => {
+          request = req;
+
+          return res(ctx.status(200), ctx.json(mockQuestion));
+        }),
+      );
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
+        logger,
+        baseUrl: BASE_URL,
+      });
+
+      const collator = await factory.getCollator();
+      const pipeline = TestPipeline.fromCollator(collator);
+      const { documents } = await pipeline.execute();
+      const expectedSearch = {
+        order: 'desc',
+        site: 'stackoverflow',
+        sort: 'activity',
+        page: '1',
+      };
+      testSearchQuery(request, expectedSearch);
+      expect(documents).toHaveLength(mockQuestion.items.length);
+    });
+  });
+
+  describe('getCollator', () => {
+    const worker = setupServer();
+    registerMswTestHooks(worker);
+
+    it('returns a readable stream', async () => {
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(
+        config,
+        optionsWithCustomRequestParams,
+      );
+      const collator = await factory.getCollator();
+      expect(collator).toBeInstanceOf(Readable);
+    });
+
+    it('fetches from the configured endpoint', async () => {
+      let request;
+      worker.use(
+        rest.get('http://stack.overflow.local/questions', (req, res, ctx) => {
+          request = req;
+
+          return res(ctx.status(200), ctx.json(mockQuestion));
+        }),
+      );
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(
+        config,
+        optionsWithCustomRequestParams,
+      );
+      const collator = await factory.getCollator();
+      const pipeline = TestPipeline.fromCollator(collator);
+      const { documents } = await pipeline.execute();
+
+      const expectedSearch = {
+        order: 'desc',
+        sort: 'activity',
+        tagged: 'developer-portal',
+        page: '1',
+        pagesize: '100',
+      };
+      testSearchQuery(request, expectedSearch);
+      expect(documents).toHaveLength(mockQuestion.items.length);
+    });
+
+    it('fetches from the overridden endpoint', async () => {
+      let request;
+      worker.use(
+        rest.get(
+          'http://stack.overflow.override/questions',
+          (req, res, ctx) => {
+            request = req;
+            return res(ctx.status(200), ctx.json(mockOverrideQuestion));
+          },
+        ),
+      );
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
+        logger,
+        baseUrl: 'http://stack.overflow.override',
+        requestParams: optionsWithCustomRequestParams.requestParams,
+      });
+      const collator = await factory.getCollator();
+
+      const pipeline = TestPipeline.fromCollator(collator);
+      const { documents } = await pipeline.execute();
+
+      const expectedSearch = {
+        order: 'desc',
+        sort: 'activity',
+        tagged: 'developer-portal',
+        page: '1',
+        pagesize: '100',
+      };
+      testSearchQuery(request, expectedSearch);
+      expect(documents).toHaveLength(mockOverrideQuestion.items.length);
+    });
+
+    it('uses API key when provided', async () => {
+      let request;
+      worker.use(
+        rest.get(
+          'http://stack.overflow.override/questions',
+          (req, res, ctx) => {
+            request = req;
+            return req.url.searchParams.get('key') === 'abcdefg'
+              ? res(ctx.status(200), ctx.json(mockOverrideQuestion))
+              : res(ctx.status(401), ctx.json({}));
+          },
+        ),
+      );
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
+        logger,
+        baseUrl: 'http://stack.overflow.override',
+        apiKey: 'abcdefg',
+        requestParams: optionsWithCustomRequestParams.requestParams,
+      });
+      const collator = await factory.getCollator();
+
+      const pipeline = TestPipeline.fromCollator(collator);
+      const { documents } = await pipeline.execute();
+
+      const expectedSearch = {
+        key: 'abcdefg',
+        order: 'desc',
+        sort: 'activity',
+        tagged: 'developer-portal',
+        page: '1',
+        pagesize: '100',
+      };
+      testSearchQuery(request, expectedSearch);
+      expect(documents).toHaveLength(mockOverrideQuestion.items.length);
+    });
+
+    it('uses teamName when provided', async () => {
+      let request;
+      worker.use(
+        rest.get(
+          'http://stack.overflow.override/questions',
+          (req, res, ctx) => {
+            request = req;
+            return req.url.searchParams.get('team') === 'abcdefg'
+              ? res(ctx.status(200), ctx.json(mockOverrideQuestion))
+              : res(ctx.status(401), ctx.json({}));
+          },
+        ),
+      );
+      const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
+        logger,
+        baseUrl: 'http://stack.overflow.override',
+        teamName: 'abcdefg',
+        requestParams: optionsWithCustomRequestParams.requestParams,
+      });
+      const collator = await factory.getCollator();
+
+      const pipeline = TestPipeline.fromCollator(collator);
+      const { documents } = await pipeline.execute();
+
+      const expectedSearch = {
+        team: 'abcdefg',
+        order: 'desc',
+        sort: 'activity',
+        tagged: 'developer-portal',
+        page: '1',
+        pagesize: '100',
+      };
+      testSearchQuery(request, expectedSearch);
+      expect(documents).toHaveLength(mockOverrideQuestion.items.length);
+    });
+  });
+});
+
+describe('StackOverflowQuestionsCollatorFactory using default request params', () => {
+  const config = new ConfigReader({
+    stackoverflow: {
+      baseUrl: BASE_URL,
+    },
+  });
+
+  const optionsWithCustomRequestParams: StackOverflowQuestionsCollatorFactoryOptions =
+    {
+      logger,
+    };
+
+  it('has expected type', () => {
+    const factory = StackOverflowQuestionsCollatorFactory.fromConfig(
+      config,
+      optionsWithCustomRequestParams,
     );
     expect(factory.type).toBe('stack-overflow');
   });
@@ -100,89 +378,128 @@ describe('StackOverflowQuestionsCollatorFactory', () => {
     it('returns a readable stream', async () => {
       const factory = StackOverflowQuestionsCollatorFactory.fromConfig(
         config,
-        defaultOptions,
+        optionsWithCustomRequestParams,
       );
       const collator = await factory.getCollator();
       expect(collator).toBeInstanceOf(Readable);
     });
 
     it('fetches from the configured endpoint', async () => {
+      let request;
       worker.use(
-        rest.get('http://stack.overflow.local/questions', (_, res, ctx) =>
-          res(ctx.status(200), ctx.json(mockQuestion)),
-        ),
+        rest.get(`${BASE_URL}/questions`, (req, res, ctx) => {
+          request = req;
+
+          return res(ctx.status(200), ctx.json(mockQuestion));
+        }),
       );
       const factory = StackOverflowQuestionsCollatorFactory.fromConfig(
         config,
-        defaultOptions,
+        optionsWithCustomRequestParams,
       );
       const collator = await factory.getCollator();
       const pipeline = TestPipeline.fromCollator(collator);
       const { documents } = await pipeline.execute();
 
+      const expectedSearch = {
+        order: 'desc',
+        site: 'stackoverflow',
+        sort: 'activity',
+        page: '1',
+      };
+      testSearchQuery(request, expectedSearch);
       expect(documents).toHaveLength(mockQuestion.items.length);
     });
 
     it('fetches from the overridden endpoint', async () => {
+      let request;
       worker.use(
-        rest.get('http://stack.overflow.override/questions', (_, res, ctx) =>
-          res(ctx.status(200), ctx.json(mockOverrideQuestion)),
-        ),
+        rest.get(`${BASE_URL}/questions`, (req, res, ctx) => {
+          request = req;
+          return res(ctx.status(200), ctx.json(mockOverrideQuestion));
+        }),
       );
       const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
         logger,
-        baseUrl: 'http://stack.overflow.override',
-        requestParams: defaultOptions.requestParams,
+        baseUrl: BASE_URL,
+        requestParams: optionsWithCustomRequestParams.requestParams,
       });
       const collator = await factory.getCollator();
 
       const pipeline = TestPipeline.fromCollator(collator);
       const { documents } = await pipeline.execute();
 
+      const expectedSearch = {
+        order: 'desc',
+        site: 'stackoverflow',
+        sort: 'activity',
+        page: '1',
+      };
+      testSearchQuery(request, expectedSearch);
       expect(documents).toHaveLength(mockOverrideQuestion.items.length);
     });
 
     it('uses API key when provided', async () => {
+      let request;
       worker.use(
-        rest.get('http://stack.overflow.override/questions', (req, res, ctx) =>
-          req.url.searchParams.get('key') === 'abcdefg'
+        rest.get(`${BASE_URL}/questions`, (req, res, ctx) => {
+          request = req;
+          return req.url.searchParams.get('key') === 'abcdefg'
             ? res(ctx.status(200), ctx.json(mockOverrideQuestion))
-            : res(ctx.status(401), ctx.json({})),
-        ),
+            : res(ctx.status(401), ctx.json({}));
+        }),
       );
       const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
         logger,
-        baseUrl: 'http://stack.overflow.override',
+        baseUrl: BASE_URL,
         apiKey: 'abcdefg',
-        requestParams: defaultOptions.requestParams,
+        requestParams: optionsWithCustomRequestParams.requestParams,
       });
       const collator = await factory.getCollator();
 
       const pipeline = TestPipeline.fromCollator(collator);
       const { documents } = await pipeline.execute();
 
+      const expectedSearch = {
+        key: 'abcdefg',
+        order: 'desc',
+        site: 'stackoverflow',
+        sort: 'activity',
+        page: '1',
+      };
+      testSearchQuery(request, expectedSearch);
       expect(documents).toHaveLength(mockOverrideQuestion.items.length);
     });
 
     it('uses teamName when provided', async () => {
+      let request;
       worker.use(
-        rest.get('http://stack.overflow.override/questions', (req, res, ctx) =>
-          req.url.searchParams.get('team') === 'abcdefg'
+        rest.get(`${BASE_URL}/questions`, (req, res, ctx) => {
+          request = req;
+          return req.url.searchParams.get('team') === 'abcdefg'
             ? res(ctx.status(200), ctx.json(mockOverrideQuestion))
-            : res(ctx.status(401), ctx.json({})),
-        ),
+            : res(ctx.status(401), ctx.json({}));
+        }),
       );
       const factory = StackOverflowQuestionsCollatorFactory.fromConfig(config, {
         logger,
-        baseUrl: 'http://stack.overflow.override',
+        baseUrl: BASE_URL,
         teamName: 'abcdefg',
-        requestParams: defaultOptions.requestParams,
+        requestParams: optionsWithCustomRequestParams.requestParams,
       });
       const collator = await factory.getCollator();
 
       const pipeline = TestPipeline.fromCollator(collator);
       const { documents } = await pipeline.execute();
 
+      const expectedSearch = {
+        team: 'abcdefg',
+        order: 'desc',
+        site: 'stackoverflow',
+        sort: 'activity',
+        page: '1',
+      };
+      testSearchQuery(request, expectedSearch);
       expect(documents).toHaveLength(mockOverrideQuestion.items.length);
     });
   });

plugins/search-backend-module-stack-overflow-collator/src/collators/StackOverflowQuestionsCollatorFactory.ts

@@ -58,6 +58,8 @@ export type StackOverflowQuestionsCollatorFactoryOptions = {
   logger: LoggerService;
 };
 
+const DEFAULT_BASE_URL = 'https://api.stackexchange.com/2.3';
+const DEFAULT_MAX_PAGE = 100;
 /**
  * Search collator responsible for collecting stack overflow questions to index.
  *
@@ -81,15 +83,19 @@ export class StackOverflowQuestionsCollatorFactory
     this.apiAccessToken = options.apiAccessToken;
     this.teamName = options.teamName;
     this.maxPage = options.maxPage;
+    this.logger = options.logger.child({ documentType: this.type });
+
     // Sets the same default request parameters as the official API documentation
     // See https://api.stackexchange.com/docs/questions
-    this.requestParams = options.requestParams ?? {
+    this.requestParams = {
       order: 'desc',
       sort: 'activity',
-      site: 'stackoverflow',
       ...(options.requestParams ?? {}),
     };
-    this.logger = options.logger.child({ documentType: this.type });
+
+    if (!options.requestParams?.site && this.baseUrl === DEFAULT_BASE_URL) {
+      this.requestParams.site = 'stackoverflow';
+    }
   }
 
   static fromConfig(
@@ -102,12 +108,12 @@ export class StackOverflowQuestionsCollatorFactory
     );
     const teamName = config.getOptionalString('stackoverflow.teamName');
     const baseUrl =
-      config.getOptionalString('stackoverflow.baseUrl') ||
-      'https://api.stackexchange.com/2.3';
-    const maxPage = options.maxPage || 100;
+      config.getOptionalString('stackoverflow.baseUrl') || DEFAULT_BASE_URL;
+    const maxPage = options.maxPage || DEFAULT_MAX_PAGE;
     const requestParams = config
       .getOptionalConfig('stackoverflow.requestParams')
       ?.get<StackOverflowQuestionsRequestParams>();
+
     return new StackOverflowQuestionsCollatorFactory({
       baseUrl,
       maxPage,