Skip to content

refactor: eliminate middleware transformer functions, implement middleware directly on structs#136

Open
gaarutyunov wants to merge 4 commits into
mainfrom
claude/issue-133-20260416-0711
Open

refactor: eliminate middleware transformer functions, implement middleware directly on structs#136
gaarutyunov wants to merge 4 commits into
mainfrom
claude/issue-133-20260416-0711

Conversation

@gaarutyunov

Copy link
Copy Markdown
Owner

Removes the standalone inbound.ValidatorMiddleware and outbound.Middleware transformer functions. Introduces embeddable base structs (ValidatorBase, ProviderBase) that provide Middleware() methods directly on each concrete type, following the same pattern as transform.CompiledTransforms.RequestMiddleware().

Closes #133

Generated with Claude Code

…eware directly on structs

Removes the standalone `inbound.ValidatorMiddleware` and `outbound.Middleware`
transformer functions that wrapped concrete types into HTTP middleware.

Instead, introduces embeddable base structs (`ValidatorBase` in the inbound
package and `ProviderBase` in the outbound package) that provide a `Middleware()`
method directly on each concrete validator/provider type. Each concrete type
embeds the appropriate base and wires the self-reference in its constructor via
`NewValidatorBase(v)` / `NewProviderBase(p)`.

This follows the same pattern already used by `transform.CompiledTransforms.RequestMiddleware()`,
where the middleware behaviour lives directly on the struct rather than requiring
an external adapter call.

Co-authored-by: German Arutyunov <gaarutyunov@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@claude[bot] has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 59 minutes and 8 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 8 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e9fabc00-0b54-44f0-b110-ac1d579cb8b6

📥 Commits

Reviewing files that changed from the base of the PR and between 5658758 and 6be7f1f.

📒 Files selected for processing (23)
  • pkg/auth/inbound/apikey/apikey.go
  • pkg/auth/inbound/inbound.go
  • pkg/auth/inbound/introspection/introspection.go
  • pkg/auth/inbound/jwt/jwt.go
  • pkg/auth/inbound/middleware.go
  • pkg/auth/outbound/apikey/apikey.go
  • pkg/auth/outbound/bearer/bearer.go
  • pkg/auth/outbound/context.go
  • pkg/auth/outbound/middleware.go
  • pkg/auth/outbound/none/none.go
  • pkg/auth/outbound/oauth2/oauth2.go
  • pkg/auth/outbound/oauth2usersession/oauth2usersession.go
  • pkg/auth/outbound/outbound.go
  • pkg/mcpanything/proxy.go
  • pkg/middleware/middleware.go
  • pkg/middleware/registry.go
  • pkg/ratelimit/ratelimit.go
  • pkg/ratelimit/ratelimit_test.go
  • pkg/runtime/js/js.go
  • pkg/runtime/lua/lua.go
  • pkg/transform/middleware.go
  • pkg/upstream/http/builder.go
  • pkg/upstream/http/refresh.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-133-20260416-0711

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gaarutyunov

Copy link
Copy Markdown
Owner Author

@claude you still created a wrapper. The problem is that middleware is a function. Change middleware to be an interface and implement it on the structs. You will have to change all the code related to middleware.

@claude

claude Bot commented Apr 17, 2026

Copy link
Copy Markdown

Claude finished @gaarutyunov's task in 19m 49s —— View job


Refactoring Middleware to Interface

  • Read current middleware structure and call sites
  • Rewrite pkg/auth/inbound/inbound.go — define Middleware interface + MiddlewareFunc
  • Rewrite pkg/auth/inbound/middleware.go — export helpers, remove ValidatorBase.Middleware()
  • Update inbound sub-packages (jwt, apikey, introspection)
  • Rewrite pkg/auth/outbound/outbound.go — define Middleware interface
  • Rewrite pkg/auth/outbound/middleware.go — export ServeWithProvider, remove ProviderBase.Middleware()
  • Update outbound sub-packages (bearer, apikey, none, oauth2, oauth2usersession)
  • Update pkg/runtime/js/js.go and lua/lua.go
  • Update pkg/mcpanything/proxy.go
  • Run make check — all lint/vet/test/build pass
  • Commit and push

What changed:

Middleware is now an interface in both inbound and outbound packages:

type Middleware interface {
    Wrap(next http.Handler) http.Handler
}

MiddlewareFunc adapter added (mirrors the http.HandlerFunc/http.Handler pattern):

type MiddlewareFunc func(http.Handler) http.Handler
func (f MiddlewareFunc) Wrap(next http.Handler) http.Handler { return f(next) }

ValidatorBase and ProviderBase are gone — each concrete type directly implements Wrap:

  • jwt.Validator.Wrap, apikey.Validator.Wrap, introspection.Validator.Wrap
  • bearer.Provider.Wrap, none.Provider.Wrap, apikey.Provider.Wrap, oauth2.Provider.Wrap, oauth2usersession.Provider.Wrap
  • js.Validator.Wrap, js.Provider.Wrap, lua.Validator.Wrap, lua.Provider.Wrap

Shared middleware logic lives as exported package functions (ServeValidated, ExtractBearerToken, ServeWithProvider) — no more indirection through a wired self reference.

apiKeyHeader is now stored in apikey.Validator at construction time instead of being passed to Middleware().

DispatchMiddleware takes/returns inbound.Middleware. proxy.go uses MiddlewareFunc to convert the registry's func(http.Handler) http.Handler output to the interface at the boundary.
|

- Define inbound.Middleware and outbound.Middleware as interfaces with
  Wrap(next http.Handler) http.Handler
- Add MiddlewareFunc adapters (mirrors http.HandlerFunc / http.Handler)
- Remove ValidatorBase and ProviderBase embedding from all concrete types
- Export ServeValidated and ExtractBearerToken from inbound package,
  ServeWithProvider from outbound package, for use by sub-packages
- Each concrete type (jwt.Validator, apikey.Validator, introspection.Validator,
  bearer.Provider, etc.) now directly implements Wrap on the struct
- Store apiKeyHeader in apikey.Validator at construction time (no longer
  passed as an argument to Middleware())
- DispatchMiddleware now takes/returns inbound.Middleware interface
- proxy.go uses MiddlewareFunc to adapt registry's func(http.Handler) http.Handler

Co-authored-by: German Arutyunov <gaarutyunov@users.noreply.github.com>
@gaarutyunov

Copy link
Copy Markdown
Owner Author

@claude no wraps, check how caddy does it. It uses ServeHTTP standard handler interface. Use same approach, no functions, no wraps. Fix it everywhere!!!!

@claude

claude Bot commented Apr 17, 2026

Copy link
Copy Markdown

Claude finished @gaarutyunov's task in 23m 14s —— View job


Refactoring Middleware to http.Handler Interface (Caddy-style)

All tasks complete. Every middleware struct now directly implements http.Handler with a Next http.Handler field and ServeHTTP method — no Wrap methods, no Middleware interfaces.

  • Read current middleware files and understand the chain
  • pkg/middleware/middleware.go — remove unused Middleware type and Chain
  • pkg/auth/inbound/inbound.go — remove Middleware interface and MiddlewareFunc
  • pkg/auth/inbound/middleware.go — replace DispatchMiddleware with NewDispatchHandler returning http.Handler; add dispatchHandler struct with ServeHTTP
  • Inbound sub-packages (jwt, apikey, introspection) — add Next http.Handler, remove Wrap, add ServeHTTP
  • pkg/auth/outbound/outbound.go — remove Middleware interface and MiddlewareFunc
  • Outbound sub-packages (bearer, apikey, none, oauth2, oauth2usersession) — add Next, remove Wrap, add ServeHTTP
  • pkg/runtime/js/js.go and lua/lua.go — same for Validator and Provider
  • pkg/ratelimit/ratelimit.go — add ClientIPHandler struct implementing ServeHTTP
  • pkg/transform/middleware.go — replace RequestMiddleware() with transform.Handler struct
  • pkg/upstream/http/builder.go and refresh.go — use &transform.Handler{...} directly
  • pkg/mcpanything/proxy.go — use NewDispatchHandler, &ClientIPHandler{Next: h}; buildAuth now returns raw factory funcs
  • make check — lint + vet + test + build all pass
  • Commit and push

What changed

Before: structs had Wrap(next http.Handler) http.Handler methods; a Middleware interface existed in both inbound and outbound packages.

After: every middleware struct (e.g. jwt.Validator, bearer.Provider, lua.Provider, ratelimit.ClientIPHandler, transform.Handler) has:

type Validator struct {
    // ... config fields
    Next http.Handler
}

func (v *Validator) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    // handle request, then call v.Next.ServeHTTP(w, r)
}

DispatchMiddlewareinbound.NewDispatchHandler(globalH, overrides, registry, lookup, bypass) http.Handler
transform.CompiledTransforms.RequestMiddleware()&transform.Handler{Transforms: t, Next: h}
pkgratelimit.ClientIPMiddleware(h)&pkgratelimit.ClientIPHandler{Next: h}

make check passes (0 lint issues, all tests green).

…-style)

Remove the Middleware interface (Wrap method) from inbound and outbound packages.
Remove MiddlewareFunc adapters. Each concrete validator/provider struct now has a
Next http.Handler field and implements ServeHTTP directly, following the standard
net/http handler pattern used by Caddy.

DispatchMiddleware is replaced by NewDispatchHandler which returns http.Handler.
RequestMiddleware() is replaced by transform.Handler struct.
ClientIPMiddleware wraps ClientIPHandler struct. buildAuth returns raw factory
functions; proxy.go wires dispatch per endpoint without a Middleware interface layer.

Co-authored-by: German Arutyunov <gaarutyunov@users.noreply.github.com>
@gaarutyunov

Copy link
Copy Markdown
Owner Author

@claude Refactor the registries to to use the interface and remove all those wrapping functions. You now have functions inside servehttp. And the constructors are returning functions. Get rid of it. Just code inside the servehttp.

@claude

claude Bot commented Apr 17, 2026

Copy link
Copy Markdown

Claude finished @gaarutyunov's task in 22m 31s —— View job


Refactoring Registries — Removing Wrapper Functions

What changed:

  • Read current code to understand patterns
  • Identify constructors returning functions and wrapper functions inside ServeHTTP
  • Refactor pkg/middleware/registry.go — introduce Builder interface
  • Refactor inbound package helpers (export helpers, add WithTokenInfo)
  • Refactor outbound package helpers (export helpers, remove ServeWithProvider)
  • Refactor inbound auth structs (jwt, apikey, introspection) — Build method + inline ServeHTTP
  • Refactor outbound auth structs (bearer, apikey, none, oauth2, oauth2usersession) — same
  • Refactor runtime structs (js, lua) — same
  • Refactor ratelimit — same
  • Update callers: builder.go, refresh.go, proxy.go
  • Run make check — lint + vet + test + build all pass
  • Commit and push

Summary of changes:

middleware.Factory now returns Builder instead of func(http.Handler) http.Handler:

type Builder interface {
    Build(next http.Handler) http.Handler
}
type Factory func(ctx context.Context, cfg any) (Builder, error)

Each init() registration now directly returns the constructed struct — no inner closures:

// Before:
pkgmiddleware.Register("inbound/jwt", func(ctx context.Context, cfg any) (func(http.Handler) http.Handler, error) {
    v, err := NewValidator(ctx, ic.JWT)
    return func(next http.Handler) http.Handler {    // ← inner function
        return &Validator{verifier: v.verifier, Next: next}
    }, nil
})

// After:
pkgmiddleware.Register("inbound/jwt", func(ctx context.Context, cfg any) (pkgmiddleware.Builder, error) {
    ic, ok := cfg.(*config.InboundAuthConfig)
    // ...
    return NewValidator(ctx, ic.JWT)  // returns *Validator directly
})

Each struct implements Build to wire the chain (shared state is preserved across calls), and ServeHTTP contains the auth logic inline — no delegation to ServeValidated/ServeWithProvider:

func (v *Validator) Build(next http.Handler) http.Handler {
    return &Validator{verifier: v.verifier, Next: next}
}

func (v *Validator) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    token := inbound.ExtractBearerToken(r)
    if token == "" {
        inbound.WriteUnauthorized(w, r, "missing_token")
        return
    }
    info, err := v.ValidateToken(r.Context(), token)
    if err != nil { ... }
    v.Next.ServeHTTP(w, r.WithContext(inbound.WithTokenInfo(r.Context(), info)))
}

Callers use .Build(next) instead of calling a function:

// Before: h = outboundMW(executor)
// After:  h = outboundBuilder.Build(executor)

…TTP logic

- Change middleware.Factory return type from func(http.Handler) http.Handler
  to Builder interface with Build(next http.Handler) http.Handler method
- Each struct implements Build directly — no anonymous inner closures in init()
- Inline ServeValidated/ServeWithProvider logic into each struct's ServeHTTP
- Export inbound.WriteUnauthorized, WriteDenied, WithTokenInfo helpers
- Export outbound.WithHeaders, WithAuthResult, AuthErrResult helpers
- Update builder.go, refresh.go, proxy.go to use .Build(next) instead of fn(next)

Co-authored-by: German Arutyunov <gaarutyunov@users.noreply.github.com>
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.

Refactor middleware

1 participant