swagger icon indicating copy to clipboard operation
swagger copied to clipboard

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

Open raphaelroshan opened this issue 1 year ago • 1 comments

Is there an existing issue that is already proposing this?

  • [X] 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 avatar Oct 14 '24 08:10 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!

raphaelroshan avatar Oct 14 '24 08:10 raphaelroshan

Would you like to create a PR for this issue?

kamilmysliwiec avatar Oct 23 '24 09:10 kamilmysliwiec

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

raphaelroshan avatar Oct 30 '24 07:10 raphaelroshan

Any update ?

evgenyidf avatar Nov 10 '24 17:11 evgenyidf

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

raphaelroshan avatar Nov 12 '24 03:11 raphaelroshan

I made PR to this issue.

hee-c avatar Nov 24 '24 04:11 hee-c

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),
    );
  });
});

raphaelroshan avatar Nov 24 '24 09:11 raphaelroshan

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.

hee-c avatar Nov 24 '24 10:11 hee-c

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

raphaelroshan avatar Nov 24 '24 11:11 raphaelroshan

Let's track this here https://github.com/nestjs/swagger/pull/3186

kamilmysliwiec avatar Nov 25 '24 07:11 kamilmysliwiec

The Pull Request is open but hasn't moved forward. What's the status?

shin1rok avatar Dec 05 '25 00:12 shin1rok