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

Bugs in swagger #199

Closed
lil131 opened this issue Sep 21, 2021 · 4 comments
Closed

Bugs in swagger #199

lil131 opened this issue Sep 21, 2021 · 4 comments

Comments

@lil131 lil131 changed the title bugs in swagger Bugs in swagger Sep 21, 2021
@peterbroadhurst
Copy link
Contributor

Hey @lil131

(2) is definitely a bug:

https://github.com/hyperledger-labs/firefly/blob/3a2f785cc77a5ad501ddba4266060f6db18dbbd5/internal/apiserver/route_post_new_message_broadcast.go#L63-L65

Did you fancy submitting a PR to fix that?

However, (1) is actually accurate I believe - it's generated from this code, which is the source of truth:
https://github.com/hyperledger-labs/firefly/blob/3a2f785cc77a5ad501ddba4266060f6db18dbbd5/internal/apiserver/route_post_new_message_broadcast.go#L84

However, there are some old docs that are referring to an API that was renamed (and you should find the old API still exists in the swagger, but is marked deprecated):
https://github.com/hyperledger-labs/firefly/blob/main/docs/gettingstarted/broadcast_data.md#example-2-inline-object-data-to-a-topic-no-datatype-verification

Wonder if you fancied updating that doc as well in your PR?

@awrichar
Copy link
Contributor

Note - in addition to topics being incorrect, it seems tag is completely missing from all messaging APIs as well.

The fact that these have been incorrect for a while also implies they're not tested E2E (so the fix should probably include an E2E test as well).

@awrichar
Copy link
Contributor

btw the Swagger schemas for these particular routes are written out by hand, rather than being generated from the Go code. That explains how they got out of sync (and it's somewhat of a concern to me in general) - but assuming we aren't changing that immediately, the schemas can be found in route_post_new_message_broadcast.go and route_post_new_message_private.go.

@lil131 lil131 mentioned this issue Sep 21, 2021
@lil131
Copy link
Contributor Author

lil131 commented Sep 21, 2021

Thanks @peterbroadhurst and @awrichar! Just sent a PR for this, also included tag as @awrichar mentioned.

@lil131 lil131 closed this as completed Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants