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.

Sign up for GitHub

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

Jump to bottom

fix: add /v1 prefix to the all paths and change 200 response of /generate path #35

Merged
merged 10 commits into from
Jan 18, 2022
Merged

fix: add /v1 prefix to the all paths and change 200 response of /generate path #35

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5fdbe7d
fix: add prefix to the all paths and fix 200 response od /generate
magicmatatjahu Jan 17, 2022
6219268
fix tests
magicmatatjahu Jan 17, 2022
5182d79
fix: add prefix to the all paths and fix 200 response od /generate
magicmatatjahu Jan 17, 2022
cf6797f
fix tests
magicmatatjahu Jan 17, 2022
0013d05
after rebase
magicmatatjahu Jan 17, 2022
8585752
fix DockerFile
magicmatatjahu Jan 17, 2022
61b7289
fix DockerFile
magicmatatjahu Jan 17, 2022
185d329
after review
magicmatatjahu Jan 17, 2022
86149d8
revert changes
magicmatatjahu Jan 17, 2022
f6d3818
change octet-stream to zip
magicmatatjahu Jan 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ FROM base AS build

WORKDIR /app
COPY . .

# delete package-lock.json - more info https://github.com/asyncapi/.github/issues/123
# delete that line and install by `npm ci` when mentioned issue will be resolved
RUN rm -rf package-lock.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we create a follow up issue so we can fix this once asyncapi/.github#123 is solved (I hope it does soon)? Removing the package-lock.json file here has unexpected behavior as we don't control which version is gonna be installed of each package if they are not pinned to an specific version...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also against but sometimes switching from v1 to v2 (of packageLock version) can create errors, so bad and so not good 😆 but ok, we can go with that package-lock.json

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smoya changed :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And also issue is created #36

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh no, it's not switching to one or another version something that scares me, no.
The fact we are not downloading exactly the same dependency versions as expected unless they are pinned to an exact version... this scares me 😆

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smoya So current changes are ok for you or we should hardcode version of the dependencies?

Copy link
Member

@smoya smoya Jan 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I think this change should not be added into this PR but to a new one so:

  • We don't hide side effects. This change is totally unexpected according to the name of the final commit that will be merged.
  • We can quickly and easily revert the changes as they will be isolated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smoya Created new PR #37

# install dependencies
RUN npm ci
RUN npm install

# build to a production Javascript
RUN npm run build:prod
Expand All @@ -24,8 +28,7 @@ FROM base AS release
WORKDIR /app
COPY --from=build /app/dist ./dist
# A wildcard is used to ensure both package.json AND package-lock.json are copied
# where available (npm@5+)
COPY package* ./
COPY --from=build /app/package* ./
# install only production dependencies (defined in "dependencies")
RUN npm ci --only=production
# copy OpenaAPI document
Expand Down
5 changes: 3 additions & 2 deletions openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ paths:
'200':
description: Template successfully generated.
content:
application/json:
application/octet-stream:
BOLT04 marked this conversation as resolved.
Show resolved Hide resolved
schema:
$ref: '#/components/schemas/Template'
type: string
format: binary
'400':
description: Failed to generate the given template due to invalid AsyncAPI document.
content:
Expand Down
3 changes: 2 additions & 1 deletion src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ export class App {

private initializeControllers(controller: Controller[]) {
controller.forEach(controller => {
this.app.use('/', controller.boot());
// in the `openapi.yaml` we have prefix `v1` for all paths
this.app.use('/v1/', controller.boot());
});
}

Expand Down
6 changes: 3 additions & 3 deletions src/controllers/tests/generator.controller.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('GeneratorController', () => {
const app = new App([new GenerateController()]);

return request(app.getServer())
.post('/generate')
.post('/v1/generate')
.send({
asyncapi: {
asyncapi: '2.2.0',
Expand All @@ -33,7 +33,7 @@ describe('GeneratorController', () => {
const app = new App([new GenerateController()]);

return request(app.getServer())
.post('/generate')
.post('/v1/generate')
.send({
asyncapi: {
asyncapi: '2.2.0',
Expand All @@ -52,7 +52,7 @@ describe('GeneratorController', () => {
const app = new App([new GenerateController()]);

return request(app.getServer())
.post('/generate')
.post('/v1/generate')
.send({
asyncapi: {
asyncapi: '2.2.0',
Expand Down
6 changes: 3 additions & 3 deletions src/controllers/tests/validate.controller.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('ValidateController', () => {
const app = new App([new ValidateController()]);

return request(app.getServer())
.post('/validate')
.post('/v1/validate')
.send({
asyncapi: validJSONAsyncAPI
})
Expand All @@ -93,7 +93,7 @@ describe('ValidateController', () => {
const app = new App([new ValidateController()]);

return request(app.getServer())
.post('/validate')
.post('/v1/validate')
.send({
asyncapi: validYAMLAsyncAPI
})
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('ValidateController', () => {
const app = new App([new ValidateController()]);

return request(app.getServer())
.post('/validate')
.post('/v1/validate')
.send({
asyncapi: invalidJSONAsyncAPI
})
Expand Down
4 changes: 3 additions & 1 deletion src/middlewares/request-body-validation.middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ const ajv = new Ajv({
* Retrieve proper AJV's validator function, create or reuse it.
*/
async function getValidator(req: Request) {
const { path: reqPath, method } = req;
const method = req.method;
let reqPath = req.path;
reqPath = reqPath.startsWith('/v1') ? reqPath.replace('/v1', '') : reqPath;
const schemaName = `${reqPath}->${method}`;

const validate = ajv.getSchema(schemaName);
Expand Down
6 changes: 3 additions & 3 deletions src/middlewares/tests/document-validation.middleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('documentValidationMiddleware', () => {
const app = new App([new TestController()]);

return await request(app.getServer())
.post('/test')
.post('/v1/test')
.send({})
.expect(200, {
success: true,
Expand All @@ -40,7 +40,7 @@ describe('documentValidationMiddleware', () => {
const app = new App([new TestController()]);

return await request(app.getServer())
.post('/test')
.post('/v1/test')
.send({
asyncapi: {
asyncapi: '2.2.0',
Expand Down Expand Up @@ -97,7 +97,7 @@ describe('documentValidationMiddleware', () => {
const app = new App([new TestController()]);

return await request(app.getServer())
.post('/test')
.post('/v1/test')
.send({
// without title, version and channels
asyncapi: {
Expand Down
4 changes: 2 additions & 2 deletions src/middlewares/tests/problem.middleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('problemMiddleware', () => {
const app = new App([new TestController()]);

return await request(app.getServer())
.post('/test')
.post('/v1/test')
.send({})
.expect(422, {
type: ProblemException.createType('custom-problem'),
Expand All @@ -43,7 +43,7 @@ describe('problemMiddleware', () => {
const app = new App([new TestController()]);

return await request(app.getServer())
.post('/test')
.post('/v1/test')
.send({})
.expect(200, {
success: true,
Expand Down
6 changes: 3 additions & 3 deletions src/middlewares/tests/request-body-validation.middleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('requestBodyValidationMiddleware', () => {
const app = new App([new TestController()]);

return await request(app.getServer())
.post('/generate')
.post('/v1/generate')
.send({
asyncapi: {
asyncapi: '2.2.0',
Expand All @@ -39,7 +39,7 @@ describe('requestBodyValidationMiddleware', () => {

it('should throw error when request body is invalid', async () => {
const TestController = createTestController({
path: '/test',
path: '/generate',
method: 'post',
callback: (_, res) => {
res.status(200).send({ success: true });
Expand All @@ -48,7 +48,7 @@ describe('requestBodyValidationMiddleware', () => {
const app = new App([new TestController()]);

return await request(app.getServer())
.post('/generate')
.post('/v1/generate')
.send({
asyncapi: {
asyncapi: '2.2.0',
Expand Down