feat: add /diff endpoint #69

ritik307 · 2022-03-05T15:28:37Z

Description

Update openapi.yaml
Update package.json
Update src/controllers/diff.controller.ts
Update src/services/diff.service.ts

Related issue(s)

Fixes #56

ritik307 · 2022-03-05T15:31:34Z

@magicmatatjahu I have to init the /diff. Let me know if I am on the right path. 🙂

BOLT04

Hi @ritik307, thanks for working on this 👍.
I have some initial thoughts, but in general, you're going in the right direction 🙂.

Let me know what you think of my comments.

pinging @magicmatatjahu since he probably knows the diff package better than me 😃

src/controllers/diff.controller.ts

BOLT04 · 2022-03-06T16:39:37Z

src/controllers/diff.controller.ts

+
+    private async difference(req: Request, res: Response, next: NextFunction) {
+        try {
+            const output = await diff(req.body.asyncapi, req.body.other);


There is an important step we should do before calling diff, which is to validate the AsyncAPI document that is sent. Like the diff docs say: "(...) but you must make sure that the document is valid and dereferenced.", so you can check out how we use parser in the validate controller 🙂

BOLT04 · 2022-03-06T16:40:51Z

src/controllers/diff.controller.ts

+            const output = await diff(req.body.asyncapi, req.body.other);
+            res.status(200).json(output);
+        } catch (err) {
+            return next(err);


maybe we should make sure we call next with an instance of ProblemException, wdyt?
You can check out other controllers to see how they are implemented, and do something similar 👍

magicmatatjahu

@ritik307 Thanks for contribution, looks good, but we need to improve several things :) Please refer to my comments :)

openapi.yaml

magicmatatjahu · 2022-03-07T10:29:52Z

src/controllers/diff.controller.ts

+    private ajv: Ajv;
+    diffService: DiffService;


You don't need the ajv. We validate the request body in the https://github.com/asyncapi/server-api/blob/master/src/app.ts#L58 middleware which runs on every request. Also diffService is unnecessary because it's only single method, so you don't need to wrap the diff library in the separate service.

src/controllers/diff.controller.ts

src/services/diff.service.ts

src/controllers/diff.controller.ts

ritik307 · 2022-03-07T17:36:51Z

@magicmatatjahu I have made the changes according to my understanding. Kindly look into them.🙂

magicmatatjahu

Good, but we have to still improve some things :)

openapi.yaml

magicmatatjahu · 2022-03-10T15:50:33Z

src/controllers/tests/diff.controller.test.ts

+            .post('/v1/diff')
+            .send({
+                asyncapi: './diff/asyncapi-diff-test-1.yaml',
+                other: './diff/asyncapi-diff-test-2.yaml',
+
+            })
+            .expect(200);


That test is broken :)

src/controllers/diff.controller.ts

openapi.yaml

src/controllers/diff.controller.ts

ritik307 · 2022-03-13T18:07:46Z

@magicmatatjahu I have made changes and also improved my test but it's still failing 😕

magicmatatjahu · 2022-03-14T16:55:55Z

openapi.yaml

+          type: array
+          minItems: 2


We should have only two files, so please add maxItems: 2 as well :)

Suggested change

type: array

minItems: 2

type: array

minItems: 2

maxItems: 2

magicmatatjahu · 2022-03-14T16:57:45Z

src/controllers/tests/diff.controller.test.ts

+      return request(app.getServer())
+        .post('/v1/diff')
+        .send({
+          asyncapis: ['./diff/asyncapi-diff-test-1.yaml','./diff/asyncapi-diff-test-2.yaml']


You operate on content sent by the user, you cannot use filesystem because these files are sent as a value anyway, not a file path. So you need to load that files before making request and then pass them as value. Did I make it clearer?

I am not able to get this part. How should I pass the file like using fs or is there any other way?

@ritik307 You should create tests data like here

server-api/src/controllers/tests/validate.controller.test.ts

Line 8 in 8efbaad

const validJSONAsyncAPI = {

(two AsyncAPI documents whose are different each other) and send it as value:

{ asyncapis: [asyncapi1, asyncapi2] }

Please also check response data in test :)

magicmatatjahu · 2022-03-21T10:01:31Z

src/controllers/tests/diff.controller.test.ts

+      return request(app.getServer())
+        .post('/v1/diff')
+        .send({
+          asyncapis: ['./diff/asyncapi-diff-test-1.yaml','./diff/asyncapi-diff-test-2.yaml']


@ritik307 You should create tests data like here

server-api/src/controllers/tests/validate.controller.test.ts

Line 8 in 8efbaad

const validJSONAsyncAPI = {

(two AsyncAPI documents whose are different each other) and send it as value:

{ asyncapis: [asyncapi1, asyncapi2] }

Please also check response data in test :)

ritik307 · 2022-03-24T16:49:12Z

@magicmatatjahu I have made changes but I am still facing issues in the test. Kindly help me out 😢😢

sonarqubecloud · 2022-03-29T17:16:53Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

magicmatatjahu · 2022-03-29T17:20:09Z

/rtm

asyncapi-bot · 2022-03-29T17:20:27Z

Hello, @magicmatatjahu! 👋🏼
This PR is not up to date with the base branch and can't be merged.
You can add comment to this PR with text: /autoupdate or /au. This way you ask our bot to perform regular updates for you. The only requirement for this to work is to enable Allow edits from maintainers option in your PR. Otherwise the only option that you have is to manually update your branch with latest version of the base branch.
Thanks 😄

magicmatatjahu · 2022-03-29T17:21:14Z

@ritik307 Thanks for contribution and patience! I hope you learned something new :)

asyncapi-bot · 2022-03-29T17:26:30Z

🎉 This PR is included in version 0.8.0 🎉

The release is available on:

GitHub release
v0.8.0

Your semantic-release bot 📦🚀

ritik307 · 2022-03-30T06:18:40Z

@magicmatatjahu Thanks for providing the opportunity to contribute. I surely learned a lot of things while contributing.😊

ritik307 requested review from smoya, BOLT04, magicmatatjahu and asyncapi-bot-eve as code owners March 5, 2022 15:28

ritik307 marked this pull request as draft March 5, 2022 15:29

ritik307 changed the title ~~added /diff~~ feat: /diff Mar 5, 2022

BOLT04 reviewed Mar 6, 2022

View reviewed changes

magicmatatjahu requested changes Mar 7, 2022

View reviewed changes

magicmatatjahu mentioned this pull request Mar 7, 2022

feat: add /bundle endpoint #70

Merged

ritik307 requested review from magicmatatjahu and BOLT04 March 9, 2022 18:04

magicmatatjahu changed the title ~~feat: /diff~~ feat: add /diff endpoint Mar 10, 2022

magicmatatjahu requested changes Mar 10, 2022

View reviewed changes

ritik307 requested a review from magicmatatjahu March 13, 2022 18:08

magicmatatjahu requested changes Mar 14, 2022

View reviewed changes

magicmatatjahu requested changes Mar 21, 2022

View reviewed changes

ritik307 marked this pull request as ready for review March 24, 2022 16:47

magicmatatjahu mentioned this pull request Mar 29, 2022

refactor: combine asyncapi validation with request body #78

Merged

ritik307 and others added 7 commits March 29, 2022 18:42

added /diff

f84d171

made requested changes

2b9ba5d

made requested changes

9ea0595

made requested changes

c8f4dea

made requested changes

957dab0

made requested changes

f45f0be

after rebase

c4d8ea9

magicmatatjahu force-pushed the add-diff branch from ef39ac2 to c4d8ea9 Compare March 29, 2022 17:16

magicmatatjahu approved these changes Mar 29, 2022

View reviewed changes

asyncapi-bot added the ready-to-merge label Mar 29, 2022

asyncapi-bot merged commit 4dac650 into asyncapi:master Mar 29, 2022

asyncapi-bot added the released label Mar 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add /diff endpoint #69

feat: add /diff endpoint #69

ritik307 commented Mar 5, 2022

ritik307 commented Mar 5, 2022

BOLT04 left a comment

BOLT04 Mar 6, 2022

BOLT04 Mar 6, 2022

magicmatatjahu left a comment

magicmatatjahu Mar 7, 2022

ritik307 commented Mar 7, 2022

magicmatatjahu left a comment

magicmatatjahu Mar 10, 2022

ritik307 commented Mar 13, 2022

magicmatatjahu Mar 14, 2022

magicmatatjahu Mar 14, 2022

ritik307 Mar 20, 2022

magicmatatjahu Mar 21, 2022

magicmatatjahu Mar 21, 2022

ritik307 commented Mar 24, 2022

sonarqubecloud bot commented Mar 29, 2022

magicmatatjahu commented Mar 29, 2022

asyncapi-bot commented Mar 29, 2022

magicmatatjahu commented Mar 29, 2022

asyncapi-bot commented Mar 29, 2022

ritik307 commented Mar 30, 2022 •

edited

Loading

feat: add /diff endpoint #69

feat: add /diff endpoint #69

Conversation

ritik307 commented Mar 5, 2022

ritik307 commented Mar 5, 2022

BOLT04 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicmatatjahu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ritik307 commented Mar 7, 2022

magicmatatjahu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ritik307 commented Mar 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ritik307 commented Mar 24, 2022

sonarqubecloud bot commented Mar 29, 2022

magicmatatjahu commented Mar 29, 2022

asyncapi-bot commented Mar 29, 2022

magicmatatjahu commented Mar 29, 2022

asyncapi-bot commented Mar 29, 2022

ritik307 commented Mar 30, 2022 • edited Loading

ritik307 commented Mar 30, 2022 •

edited

Loading