Skip to content

Conversation

@moosebay
Copy link
Contributor

@moosebay moosebay commented Feb 9, 2026

Summary

  • New topenapiv2 translator package supporting both JSON and YAML Swagger 2.0 / OpenAPI 3.x specs
  • Format detection for swagger/openapi with confidence scoring
  • URL fetching: when textData is a URL, fetch content before detection
  • Resolve textData→Data in ImportUnified so text-based imports work
  • Frontend: update import dialog to mention Swagger/OpenAPI support

Tests

  • Petstore Swagger 2.0 (14 operations across pet/store/user)
  • Stripe-like OpenAPI 3.0 (9 operations across customers/charges/refunds)
  • Verifies request counts, HTTP methods, path params, query params, headers, request bodies, flow structure, and body kind correctness

Add ability to import Swagger 2.0 and OpenAPI 3.x specs, both as file
uploads and by pasting a URL to a spec endpoint (e.g. petstore.swagger.io).

- New `topenapiv2` translator package supporting both JSON and YAML specs
- Format detection for swagger/openapi with confidence scoring
- URL fetching: when textData is a URL, fetch content before detection
- Resolve textData->Data in ImportUnified so text-based imports work
- Frontend: update import dialog to mention Swagger/OpenAPI support
- Handle resp.Body.Close() error return for errcheck linter

Tests:
- Petstore Swagger 2.0 (14 operations across pet/store/user)
- Stripe-like OpenAPI 3.0 (9 operations across customers/charges/refunds)
- Verifies request counts, HTTP methods, path params, query params,
  headers, request bodies, flow structure, and body kind correctness
@ElecTwix
Copy link
Contributor

ElecTwix commented Feb 9, 2026

the overall architecture is clean. The translator plugs into the existing registry pattern well, the normalized internal spec struct is a good abstraction over Swagger 2.0 vs OpenAPI 3.x, and the test coverage is thorough (especially the structural integrity tests for flow graphs and ID uniqueness).

A few things to address before merging:


Bugs

1. URL fetching is bypassed through ImportUnifiedWithTextData

ImportUnifiedWithTextData (service.go:711) converts TextData to Data bytes before calling ImportUnified. Then resolveInputData (service.go:728) sees Data is already populated and short-circuits — never checking IsURL().

If a user pastes https://petstore.swagger.io/v2/swagger.json as text input and the RPC handler routes through ImportUnifiedWithTextData, the literal URL string gets sent to format detection instead of being fetched. The URL fetching feature only works if ImportUnified is called directly with TextData set and Data empty.

Fix: either call resolveInputData before the text→bytes conversion in ImportUnifiedWithTextData, or remove the text→bytes conversion there entirely and let resolveInputData handle all three input modes (it already does).

2. No HTTP client timeout on DefaultURLFetcher

urlfetch.go:28 creates &http.Client{} with no timeout. The existing codebase pattern in httpclient.go uses Timeout: 60 * time.Second. The service-level context timeout is 30 minutes, which is far too long for a single URL fetch. A slow or unresponsive remote server would hold the connection open.

client: &http.Client{Timeout: 60 * time.Second},

3. Non-deterministic assert status code selection

topenapiv2.go:670 iterates op.Responses (a Go map) looking for the first 2xx code. Map iteration order is random in Go, so if an operation defines both 200 and 201 responses, the assert could pick either one across different runs.

Paths and methods are already sorted for determinism — same pattern should apply here. Could sort the response codes first, or explicitly prefer 200.


Limitations to document

4. No $ref resolution

parseSchema (topenapiv2.go:422) reads type, example, and properties directly but doesn't resolve $ref references. Most real-world OpenAPI specs use $ref extensively (e.g. "schema": { "$ref": "#/definitions/Pet" }), so importing them would produce empty request bodies and missing parameters.

The test fixtures work because they inline everything. This is fine for a V1, but worth adding a comment or note in the code/docs so users understand why their real spec might import with missing data. Could be a fast follow-up.

5. Swagger 2.0 consumes field ignored

topenapiv2.go:280 hardcodes ContentType: "application/json" for Swagger 2.0 body parameters instead of reading the spec's consumes field. Works for most APIs but technically incorrect for XML/form-data specs. A // TODO comment would be enough for now.


Nits

6. Dead code in detectOpenAPI

format_detection.go:474-476: the confidence < 0 guard is unreachable since all operations in detectOpenAPI are additive (+=) from 0.0. The existing HAR/Postman detectors need this check (they have -= penalties), but this one doesn't.

7. parseRequestBody non-deterministic for multi-content-type specs

topenapiv2.go:401-416 iterates the content map to find application/json. If JSON is present it's always found (loop continues until it). But if a spec only has multiple non-JSON types (application/xml + multipart/form-data), whichever comes last in random map iteration wins.


Overall: bugs 1-3 should be fixed before merge. 4-5 are fine as documented limitations for V1. The rest are nits.

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

Successfully merging this pull request may close these issues.

2 participants