Skip to content

feat: add logger and adjust reflection#63

Merged
akfaiz merged 8 commits into
mainfrom
feat/logger
May 12, 2026
Merged

feat: add logger and adjust reflection#63
akfaiz merged 8 commits into
mainfrom
feat/logger

Conversation

@akfaiz

@akfaiz akfaiz commented May 12, 2026

Copy link
Copy Markdown
Member

Description

  • add logger
  • adjust reflection schema

Closes #

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactor / code quality
  • Other (describe):

Checklist

  • make check passes locally (sync + tidy + lint + test)
  • Tests added or updated to cover the change
  • Golden files updated if spec output changed (make test-update)
  • Relevant documentation updated (README, adapter README, etc.)
  • Commit messages follow Conventional Commits

akfaiz added 4 commits May 12, 2026 06:20
- Types implementing encoding.TextMarshaler+TextUnmarshaler (without
  json.Marshaler) are now reflected as type:string schemas.
- Add openapi.EmbedReferencer interface and refer:"true" struct tag:
  embedded structs opt into allOf $ref instead of field inlining.
- Fix RefSchema to propagate AllOf from StructSchema into component.
- Add golden tests for embed_ref case across all three OAS versions.
@codecov

codecov Bot commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.87671% with 25 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/reflect/reflector.go 77.77% 5 Missing and 3 partials ⚠️
internal/reflect/converter.go 56.25% 5 Missing and 2 partials ⚠️
internal/reflect/utils.go 89.39% 4 Missing and 3 partials ⚠️
option/openapi.go 70.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@llamapreview llamapreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Request Changes

This PR adds valuable logger integration, TextMarshaler/EmbedRef support, and automatic form content-type inference, but introduces a breaking change in struct tags (formDataform) that will silently break existing user-defined types. A critical golden file for the new embed_ref test is also missing, causing test failures.

📄 Documentation Diagram

This diagram documents the new reflection features: TextMarshaler detection, EmbedReferencer allOf $ref, and automatic form content-type inference.

sequenceDiagram
    participant Client
    participant Reflector
    participant Builder
    Client->>Reflector: SchemaForType(t, mode, field)
    Reflector->>Reflector: Check isTextMarshaler(t)
    alt TextMarshaler without json.Marshaler
        Reflector->>Reflector: Return string schema
    else Not TextMarshaler
        Reflector->>Reflector: Normal type switch (int, string, etc.)
    end
    Client->>Reflector: StructSchema(t, nameTag, onlyTagged, mode)
    Reflector->>Reflector: Pre-scan anonymous fields for refer:"true" or EmbedReferencer
    note over Reflector: PR #35;63: new EmbedRef support
    alt Is embed ref
        Reflector->>Reflector: Generate component via RefSchema
        Reflector->>Reflector: Append allOf $ref to parent schema
    else Normal embed
        Reflector->>Reflector: Inline fields
    end
    Client->>Builder: ContentType(cu)
    Builder->>Builder: Check InferContentType(cu.Structure)
    note over Builder: PR #35;63: automatic content-type detection
    alt Form tag present
        alt File type found
            Builder->>Builder: Return multipart/form-data
        else No file type
            Builder->>Builder: Return application/x-www-form-urlencoded
        end
    else No form tag
        Builder->>Builder: Use explicit ContentType or fallback to application/json
    end
Loading

🌟 Strengths

  • Solid new feature additions (logger, TextMarshaler, EmbedRef, form content-type inference) with good test coverage.

⚡ Key Risks & Improvements (P1)

  • internal/testutil/dto/types.go: The formDataform tag rename is a breaking API change; external users with formData tags will lose automatic form recognition. Add backward compatibility or document the migration.

💡 Suggestions (P2)

  • golden_test.go: The embed_ref golden test is added without the corresponding golden file – tests will fail on checkout. Run make test-update and commit the generated file.

📈 Risk Diagram

This diagram illustrates the risk of the formDataform tag migration breaking existing user structs.

sequenceDiagram
    participant User
    participant Reflector
    participant Builder
    User->>Reflector: Struct with formData tags (legacy)
    Reflector->>Reflector: InferContentType looks only for form tag
    note over Reflector: R1(P1): Legacy formData tags ignored
    Reflector->>Builder: Body tag empty → no form fields
    Builder->>Builder: ContentType defaults to application/json
    note over Builder: R1(P1): Struct fields not extracted as form parameters
    Builder->>User: Generated schema missing form fields
    User->>User: Expected application/x-www-form-urlencoded body with fields, got empty
Loading

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

Comment thread internal/testutil/dto/types.go
Comment thread golden_test.go
akfaiz added 3 commits May 12, 2026 07:25
Add ParameterInBody and ParameterInForm constants. When set in
ParameterTagMapping, they override the struct tag used to name
JSON body fields (default "json") and form body fields (default "form").
ParameterField skips these two keys so body-tagged fields are not
misidentified as HTTP parameters.
Previously, def names were unqualified for types in the caller package
(detected via runtime stack walking), and prefixed only for external
packages. This caused cross-package name collisions and relied on
fragile runtime introspection.

Now all named types are prefixed with their Go package name, matching
openapi-go / jsonschema-go behavior:
- models.User → ModelsUser
- dto.Pet     → DtoPet

Multi-segment package names are sanitized to UpperCamel
(spec_test → SpecTest). Unexported type names are title-cased on join.

BREAKING CHANGE: generated $ref component schema names gain a package
prefix. Existing OpenAPI documents will have different schema keys.
Use InterceptDefName or StripDefNamePrefix to strip the prefix if the
old names are required.

Removed: DefNameCallerPkg field on ReflectorConfig,
ensureCallerPkgForDefName, callerPackagePath, packagePathFromFunc.
@akfaiz akfaiz merged commit 280eee2 into main May 12, 2026
16 of 17 checks passed
@akfaiz akfaiz deleted the feat/logger branch May 12, 2026 01:51
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.

1 participant