Add swagger definitions for the receive end point#842
Conversation
This is done with a custom script to embed the schemas from signal-cli into the docs.go file
Co-authored-by: Copilot <copilot@github.com>
|
Thanks for the PR! I noticed that you do a quite some bit of string parsing to find the correct place in the json/go files. Have you maybe considered unmarshaling the json/go file into a |
|
Thanks for reviewing! I tried first doing it as you suggest but the marshaling back into json modified the file, as it would create diffs where there weren't any. As far as I recall the modifications were on reordering fields, maybe also on blank spaces. If you think that's an acceptable trade-off, I'm happy to go down that route. |
|
As you are not compiling signal-cli anymore in this repo. I temporarily pointed the download of the schemas to my fork of signal-cli. I plan to make new releases for the schemas there until AsamK/signal-cli#2040 gets merged in the main repo. I considered adding generating the schemas in your new repos but that seemed like a lot more work. |
But I guess that would only be a problem the first time? 🤔 What I am also not sure is whether the automatic generation via the CI is a good idea or whether it would be better to just notify (e.g by failing the build) in case the swagger documentation needs to be manually re-generated. What I am always worried is, that those automations open attack vectors for cleverly crafted (shell) injection attacks or open up threat vectors in case an upstream repository gets compromised. But maybe I am unnecessarily cautious here. |
|
Yes, the extra changes would only happen the first time. I get the worry about the new CI job, there has been so many attacks lately. The only thing I can think of is to pin the versions of the actions, like it was done in #838. You'll be the one creating the releases, so it depends on whether you prefer the extra work of generating the docs and re-run a failing CI or use the job. I like automating as much as possible, so if I were you I'd keep the automation. From my side, I just care that the final docs are generated correctly. I don't think it'll be much work to add a check to the release work to fail if the docs are out of date, if that's what you want. |
I guess that should be fine then. I think reducing the complexity of the script would be worth it - especially, since I would like to switch to OpenApi 3.x at some point and I guess some changes might be needed then to the script. If it's not easily do-able with golang, I think a Python script should be okay too. For the CI: I think I would prefer a simple build failure/CI failure for now. I know, that sounds a bit boring, but I'd like to see how far that gets us. I think the API documentation doesn't change that often, so I think manually a manual rebuild should be fine for now. That way it should be possible to really lock down the permissions of the CI job. :) Thanks a lot for all the effort you are putting into this - it's very much appreciated! |
There was a problem hiding this comment.
To check that this file contains the same information but now reordered alphabetically, I wrote a tiny parser to check each key/value. I loaded this file and one I generated from before the changes using commit 82f7151
| func toValidJson(content string) string { | ||
| content = strings.ReplaceAll(content, "` + \"`\" + `", "`") | ||
| content = strings.Replace(content, schemesTemplateValue, `"`+schemesPlaceholderToken+`"`, 1) | ||
| return content | ||
| } | ||
|
|
||
| func encodeForGoRawString(content string) string { | ||
| content = strings.ReplaceAll(content, "`", "` + \"`\" + `") | ||
| content = strings.Replace(content, `"`+schemesPlaceholderToken+`"`, schemesTemplateValue, 1) | ||
| return content | ||
| } |
There was a problem hiding this comment.
I've simplified the script as requested to marshal/unmarshal the whole json. I still had a to add a few checks in these two functions because the docs.go doesn't contain a fully valid json. So these lines convert it into a valid one and then set it back to how it was.
|
many thanks! |
2 participants
This PR closes #822, #622 and #199.
As discussed in #822 it generates json-schema files using the signal-cli utility for it and then uses a custom script to embed those in the generated
docs.gofile.I made the script as simple as possible, which means that it doesn't update the
.yamlfile. So I removed the file from the repo.I also updated the Readme for the docs.
If swaggo/swag#2165 gets fixed, I'll make a new PR to remove the script and update the annotations accordingly but I think it still makes sense to merge as is and not wait for that.
I used copilot extensively to write this as I'm not so familiar with writing go code. I still reviewed everything and checked that it makes sense but please, have a careful look at the script in case I missed something.