refactor(codegen): better Swagger compression, modern naming#1909
Conversation
a8c88a5 to
00e6831
Compare
00e6831 to
a077bd0
Compare
a077bd0 to
7b2834d
Compare
|
Thanks for this - is there an issue this solves? Or was this something you noticed and think it's worthwhile doing? |
Kusari Analysis Results:
No pinned version dependency changes, code issues or exposed secrets detected! Note View full detailed analysis result for more information on the output and the checks that were run.
Found this helpful? Give it a 👍 or 👎 reaction! |
Hi! There’s no specific issue, these changes just provide a small performance boost:
|
|
Kusari PR Analysis rerun based on - 7ceda90 performed at: 2025-06-15T13:56:55Z - link to updated analysis |
|
Kusari PR Analysis rerun based on - 549d69f performed at: 2025-07-15T19:09:45Z - link to updated analysis |
|
Kusari PR Analysis rerun based on - 65f9a84 performed at: 2025-07-17T09:44:18Z - link to updated analysis |
|
Kusari PR Analysis rerun based on - 152681f performed at: 2025-07-28T17:48:33Z - link to updated analysis |
|
Kusari PR Analysis rerun based on - de8869a performed at: 2025-08-29T15:40:28Z - link to updated analysis |
|
Kusari PR Analysis rerun based on - 7767cdf performed at: 2025-09-28T17:35:58Z - link to updated analysis |
|
(Feel free to not keep this branch updated, when I come back to this PR, I'll get it updated / address any conflicts if needed) |
Greptile SummaryThis PR replaces gzip with raw deflate ( Confidence Score: 5/5Safe to merge; the only finding is a P2 shared-slice concern on the new GetSpecJSON helper All changes are consistent across every backend and fixture file; GetSwagger() is retained as a deprecated wrapper preserving backward compatibility; the compression switch is symmetric between inline.go and inline.tmpl; no exported symbols are removed. The single P2 finding (GetSpecJSON returning a mutable reference to the cached slice) does not block merge. pkg/codegen/templates/inline.tmpl — GetSpecJSON returns the raw cached slice without copying
|
| Filename | Overview |
|---|---|
| pkg/codegen/inline.go | Switches compression from gzip to raw deflate (flate); error handling style improved; symmetric with template change |
| pkg/codegen/templates/inline.tmpl | Renames GetSwagger→GetSpec (with deprecated wrapper), adds GetSpec and GetSpecJSON; GetSpecJSON returns the cached byte slice directly without copying, which is a shared-state risk |
| pkg/codegen/templates/imports.tmpl | Swaps compress/gzip import for compress/flate to match inline.tmpl change |
| internal/test/compatibility/preserve-original-operation-id-casing-in-embedded-spec/spec_test.go | Updates GetSwagger() call site to GetSpec(); correct and consistent with the deprecation |
| examples/petstore-expanded/echo/api/petstore-server.gen.go | Generated file: compression format and API functions updated consistently with template changes |
Reviews (2): Last reviewed commit: "Add GetSpec/GetSpecJSON; revert const-co..." | Re-trigger Greptile
|
I'm going to build on top of your PR. It's good to use I will also take the opportunity to rename "GetSwagger" to "GetSpec", and expose the raw JSON via GetSpecJSON, this way, we can use other OpenApi middleware. |
# Conflicts: # internal/test/externalref/packageA/externalref.gen.go # internal/test/externalref/packageB/externalref.gen.go # internal/test/issues/issue-1378/common/common.gen.go # internal/test/strict-server/chi/server.gen.go # internal/test/strict-server/echo/server.gen.go # internal/test/strict-server/fiber/server.gen.go # internal/test/strict-server/gin/server.gen.go # internal/test/strict-server/gorilla/server.gen.go # internal/test/strict-server/iris/server.gen.go # internal/test/strict-server/stdhttp/server.gen.go
Three follow-ups to PR #1909, all in `pkg/codegen/templates/inline.tmpl` (plus the regenerated `*.gen.go` files and migrated callers): 1. Revert the embedded spec back to `var swaggerSpec = []string{...}`. PR #1909 replaced the historical slice-of-chunks form with a single `const swaggerSpec = "" + "chunk" + "chunk" + ...`. With thousands of 80-char chunks (large APIs produce them) the Go compiler folds the chained `+` at compile time, and that fold is materially slower than parsing a slice literal. `decodeSpec` now joins the slice via `strings.Join` before base64-decoding; flate compression, base64 encoding, the 80-char chunk size, the lazy `decodeSpecCached` / `rawSpec` pipeline, and `PathToRawSpec` all stay exactly as PR #1909 landed them. 2. Add `GetSpec() (*openapi3.T, error)` as the canonical accessor, and demote `GetSwagger()` to a `// Deprecated:` thin wrapper. The kin-openapi spec type was renamed from `openapi3.Swagger` to `openapi3.T` years ago; `GetSwagger`'s name is a stale relic. `GetSpec` carries the body that `GetSwagger` had, and `GetSwagger` becomes a one-line `return GetSpec()` retained for backwards compatibility. External callers still upgrading to a newer version of oapi-codegen will see the `// Deprecated:` hint surfaced by gopls/godoc; in-tree callers across `examples/` and `internal/test/` are migrated to `GetSpec()` because the repo's lint rule rejects calls into deprecated functions and would otherwise block CI. 3. Add `GetSpecJSON() ([]byte, error)` returning the embedded JSON. Decompressed, base64-decoded, but not unmarshaled into `*openapi3.T`. A one-line wrapper around the existing `rawSpec()` cache, so repeated calls don't re-decompress. This fills a long-standing gap: callers who want to operate on the raw OpenAPI document (re-marshal to YAML, hand to a different parser, run an overlay, dump to disk) previously had to either invoke `openapi3.T.MarshalJSON` after `GetSpec` or hand-roll base64+flate decode. Migrated callers (mechanical; no logic changes): - 7 user-side example files under `examples/` (petstore variants for each framework, authenticated-api echo + stdhttp). - 12 test files under `examples/petstore-expanded/*/petstore_test.go`, `internal/test/externalref/imports_test.go`, `internal/test/issues/issue1825/overlay_test.go`, `internal/test/compatibility/preserve-original-operation-id-casing-in-embedded-spec/spec_test.go`. New test: - `examples/petstore-expanded/chi/petstore_test.go::TestSpecAccess` covers `GetSpec` and `GetSpecJSON` end-to-end (asserts the bytes are valid JSON and the parsed spec has a non-empty `OpenAPI` version field). `GetSwagger` is intentionally not exercised — its body is `return GetSpec()` and the lint rule blocks an in-tree call. If we later want regression coverage for the wrapper specifically, one test annotated with `//nolint:staticcheck // SA1019` would do it. Verification: - `make generate` is idempotent; spot-checking `examples/petstore-expanded/chi/api/petstore.gen.go` confirms the slice form, the three new accessors, and the `// Deprecated:` marker on `GetSwagger`. - `make test` passes (exit 0) across the root module and all eight child modules. - `make lint` reports `0 issues.` across the same nine modules — no remaining in-tree calls into the deprecated wrapper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptileai rereview, comments addressed. |
|
Hello, thank you. I would note that the original idea was to use the same approach that is used in protobuf code generation starting from version v1.36.6. |
No description provided.