Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validator does not work with express router #65

Closed
SpencerLawrenceBrown opened this issue Oct 10, 2019 · 5 comments
Closed

Validator does not work with express router #65

SpencerLawrenceBrown opened this issue Oct 10, 2019 · 5 comments
Labels
question Further information is requested

Comments

@SpencerLawrenceBrown
Copy link
Contributor

I am not sure if this is a feature request or a bug, but the validator does not validate requests and responses to/from express routers.

When I originally set up my app, all of my routes lived in the app.ts file and were being validated correctly.

app.ts

const app = express();
app.use(bodyParser.json());
app.use(express.json());

new OpenApiValidator({
  apiSpec: './workflow-tracker-openapi.yml',
  validateRequests: true,
  validateResponses: true,
}).install(app);

// Validates correctly! :)
app.get('/v1/progress/:workflowType', function(req, res, next) {
  res.json([{id: 1, name: 'test'}]);
});

however I am going to have a lot of routes + middlewares and want to use express routers to split them up. So I split my routes to use a router and the validator stopped working:

app.ts

const app = express();
app.use(bodyParser.json());
app.use(express.json());
app.use('/v1', progressRouter);

new OpenApiValidator({
  apiSpec: './workflow-tracker-openapi.yml',
  validateRequests: true,
  validateResponses: true,
}).install(app);

api/progress.ts

const router = express.Router();

// No longer validated :(
router.get('/progress/:workflowType', function(req, res, next) {
  res.json([{id: 1, name: 'test'}]);
});

export { router as progressRouter };

Furthermore I tried to install the routers to the OpenApiValidator but it only accepts Applications.

Am I missed a configuration step or are routers not supported currently? If they are not, is there timeline for when they would be supported?

Is there another workaround for this?

thank you!

@SpencerLawrenceBrown
Copy link
Contributor Author

Looks like the validator middleware only gets pushed onto the app not the routes:

express-openapi-validator/dist/index.js

        if (this.options.validateRequests)
            use.push(validateMiddleware);
        if (this.options.validateResponses)
            use.push(resOav.validate());
        app.use(use);

I imagine this is the issue.

@SpencerLawrenceBrown
Copy link
Contributor Author

ok wow, did more digging and it turns out that the order in which the middlewares are added matters
:(

Validates correctly

const app = express();
new OpenApiValidator({
  apiSpec: './workflow-tracker-openapi.yml',
  validateRequests: true,
  validateResponses: true,
}).install(app);
app.use(bodyParser.json());
app.use(express.json());
app.use('/v1', progressRouter);

Does not validate

const app = express();
app.use('/v1', progressRouter);
new OpenApiValidator({
  apiSpec: './workflow-tracker-openapi.yml',
  validateRequests: true,
  validateResponses: true,
}).install(app);
app.use(bodyParser.json());
app.use(express.json());

@SpencerLawrenceBrown
Copy link
Contributor Author

Let me know if you'd like me to open up a pr to talk about this in the documentation

@cdimascio
Copy link
Owner

cdimascio commented Oct 10, 2019

I believe the issue is the fact in the second example (Does not validate), the bodyParser middleware is setup after the routes are setup. To parse json, text, etc the parse middleware should come earlier in the chain

Assuming this is the case, perhaps, a note should be added to the documentation.

Note that I can certainly have the validator install the parser middleware automatically, however I feel the choice of parser should be left to the user and not to the library.

@cdimascio cdimascio added the question Further information is requested label Oct 11, 2019
@cdimascio
Copy link
Owner

@SpencerLawrenceBrown i've added some notes to the doc. feel free to provide feedback. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants