[discussion]: allow swagger-scanner deepScanRoutes to be recursive or go down an extra level

[raphaelroshan]

Is there an existing issue that is already proposing this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe it

Swagger scanner does not support going more than one level down (with the deepScanRoutes option), and I would like for there to be an additional flag/ parameter that would allow it to scan 2 levels down, or recursively if possible.

Describe the solution you'd like

I would like to implement a recursiveScan flag that would allow Swagger to scan more than one level down, in the case of such consolidated apps. If this is against design decision, perhaps I would adapt the deepScan to accept a parameter of 1 (default) or 2, scanning an extra level down. This change would be implemented entirely in swagger-scanner.ts.

Teachability, documentation, adoption, migration strategy

I will adapt any existing swagger-scanner documentation and update it for this new change.

What is the motivation / use case for changing the behavior?

Swagger was a functional choice when we operated several microservices, as we only needed to deepScan one level down. Recently we have consolidated several microservices into an app, and as a result we need to scan an additional level down, which swagger does not currently support. To comply with the single level deepScan would require shifting all the routes up one level which would mess up the way the app is configured as a consolidation of sub-apps.

[raphaelroshan]

Ideally I would like to implement this so as to benefit the team I am working with, but I also recognise that this would be my first contribution to the nestjs repo so I appreciate any feedback/ suggestions/ improvements. Thank you!

[kamilmysliwiec]

Would you like to create a PR for this issue?

[raphaelroshan]

Yes, I will submit it next week. Anything to note?

[evgenyidf]

Any update ?

[raphaelroshan]

Still working on it! Will submit a PR in a few days hopefully 🙏

[hee-c]

I made PR to this issue.

[raphaelroshan]

Aw dang was still working on mine, but since I couldnt get it to work probably would be better to go with yours! Had a question if anybody here could help me out answering it - this was my updated code for denormalized paths,
and i tried doing a single file test that mocked modules with dependencies as below, however all the routes were picked up even when passing false in the document Options. This was the case even when i tried to split the modules up into seperate files as @hee-c did above. Could anyone explain where I went wrong please?:

const denormalizedPaths = modules.map(
        ({ controllers, metatype, imports }) => {
          let result: ModuleRoute[] = [];
          const isGlobal = (module: Type<any>) => !container.isGlobalModule(module);
          if (deepScanRoutes || recursiveScanRoutes) {
            const scanSubmodules = (module) => {
              if (!isGlobal(module)) {
                module.imports.forEach(subModule => {
                  const modulePath = this.getModulePathMetadata(container, subModule.metatype);
                  result = result.concat(
                    this.scanModuleControllers(
                      subModule.controllers,
                      modulePath,
                      globalPrefix,
                      internalConfigRef,
                      operationIdFactory
                    )
                  );
                  if (recursiveScanRoutes) {
                    scanSubmodules(subModule);
                  }
                });
              }
            };
            scanSubmodules({ metatype, controllers, imports });
          }
          const modulePath = this.getModulePathMetadata(container, metatype);
          result = result.concat(
            this.scanModuleControllers(
              controllers,
              modulePath,
              globalPrefix,
              internalConfigRef,
              operationIdFactory
            )
          );
          return this.transformer.unescapeColonsInPath(app, result);
        }
      );

import { SwaggerScanner } from '../../lib/swagger-scanner';
import { SwaggerDocumentOptions } from '../../lib/interfaces';
import { RootModule } from './root/root.module';
import { Test } from '@nestjs/testing';
import { INestApplication, Module, Controller, Get } from '@nestjs/common';

describe('SwaggerScanner recursive scanning', () => {
  let app: INestApplication;

  beforeEach(async () => {
    const moduleRef = await Test.createTestingModule({
      imports: [RootModule],
    }).compile();

    app = moduleRef.createNestApplication();
    await app.init();
  });

  afterEach(async () => {
    await app.close();
  });

  it('should include only root module controllers in a basic scan', () => {
    const scanner = new SwaggerScanner();
    const options: SwaggerDocumentOptions = {
      recursiveScanRoutes: false,
      deepScanRoutes: false,
    };

    const swaggerDocument = scanner.scanApplication(app, options);
    const paths = swaggerDocument.paths;
    const expectedPaths = ['/root/test'];

    expectedPaths.forEach((path) =>
      expect(paths).toHaveProperty(path),
    );

    expect(paths).not.toHaveProperty('/child/child-test');
    expect(paths).not.toHaveProperty('/grandchild/grandchild-test');
  });

  it('should include up to child module controllers in a deep scan', () => {
    const scanner = new SwaggerScanner();
    const options: SwaggerDocumentOptions = {
      recursiveScanRoutes: false,
      deepScanRoutes: true,
    };

    const swaggerDocument = scanner.scanApplication(app, options);

    const paths = swaggerDocument.paths;
    const expectedPaths = ['/root/test', '/child/child-test'];

    expectedPaths.forEach((path) => 
      expect(paths).toHaveProperty(path),
    );
    expect(paths).not.toHaveProperty('/grandchild/grandchild-test');
  });

  it('should include all controllers in a recursive scan', () => {
    const scanner = new SwaggerScanner();
    const options: SwaggerDocumentOptions = {
      recursiveScanRoutes: true,
    };

    const swaggerDocument = scanner.scanApplication(app, options);
    const paths = swaggerDocument.paths;
    const expectedPaths = [
      '/root/test',
      '/child/child-test',
      '/grandchild/grandchild-test',
    ];

    expectedPaths.forEach((path) =>
      expect(paths).toHaveProperty(path),
    );
  });
});

[hee-c]

If I understand correctly, does this mean that all routes are scanned even when deepScanRoutes is set to false?
When modules are explicitly specified in include, the scanning behavior depends on deepScanRoutes: it scans either root modules or up to 1-depth.
However, if no modules are specified in include, all routes are scanned.

[raphaelroshan]

Ah I see, that's interesting, thanks for explaining where i went wrong

[kamilmysliwiec]

Let's track this here #3186