Allow overriding the request validator when using NewService

[joekrill]

There's currently no way to customize the request validator used when initializing a web service using NewService. I realized this when looking into customizing the validation error response (#235).

This change adds a field to the Service struct called RequestValidatorFactory (mirroring to the current ResponseValidatorFactory field) and uses that to initialize the ValidatorMiddleware. If it's not set it still defaults to the current behavior.

Example:

service := web.NewService(openapi31.NewReflector(), func(s *web.Service) {
	s.RequestValidatorFactory = myCustomValidatorFactory
})

Additionally, I noticed that if the ResponseValidatorFactory is set using the options parameter when calling NewService, it will have no effect and get overridden by the default validator. For example:

service := web.NewService(openapi31.NewReflector(), func(s *web.Service) {
        // This will not work because `NewService` will set `s.ResponseValidatorFactory` to the default validator anyway.
	s.ResponseValidatorFactory = myCustomValidatorFactory
})

So this also adds a check for whether that's been set before setting it to the default (jsonschema) validator.

[llamapreview]

Auto Pull Request Review from LlamaPReview

1. Overview

1.1 Core Changes

• Primary purpose and scope: Enable customization of request validation in the NewService initialization flow, addressing a gap where request validator customization was previously impossible
• Key components modified: web.Service struct, NewService function, and middleware configuration
• Cross-component impacts: Affects validation middleware chain and service initialization sequence
• Business value alignment: Enables compliance with strict API standards, supports custom validation scenarios, and reduces need for custom forks

1.2 Technical Architecture

• System design modifications: Introduces new extension point for request validation while maintaining existing architecture
• Component interaction changes: Alters validator factory initialization sequence and middleware configuration
• Integration points impact: Affects how external validation logic integrates with the service
• Dependency changes and implications: No new dependencies added, but changes how existing validation components interact

2. Critical Findings

2.1 Must Fix (P0🔴)

Issue: Missing Validation Interface Documentation

• Analysis Confidence: High
• Impact: Developers won't know how to implement custom validators, leading to potential misuse
• Resolution: Add comprehensive godoc examples for rest.RequestValidatorFactory showing implementation patterns

Issue: Concurrency Vulnerability in Default Initialization

• Analysis Confidence: High
• Impact: Potential race conditions if OpenAPICollector is modified during initialization
• Resolution: Add synchronization or clearly document thread-unsafe nature in the implementation

2.2 Should Fix (P1🟡)

Issue: Validation Order Ambiguity

• Analysis Confidence: Medium
• Impact: No control over validation sequence if multiple validators exist
• Suggested Solution: Add position control to middleware registration to allow explicit ordering

Issue: Inconsistent Field Comments

• Analysis Confidence: High
• Impact: Reduced code maintainability and developer experience
• Suggested Solution: Add symmetrical documentation for RequestValidatorFactory matching the style of ResponseValidatorFactory

Issue: Error Handling Gap

• Analysis Confidence: High
• Impact: Potential runtime errors if custom validator returns nil
• Suggested Solution: Add guard clause in middleware setup to validate factory initialization

2.3 Consider (P2🟢)

Area: Validator Interface Simplification

• Analysis Confidence: Medium
• Improvement Opportunity: Unify request/response validator interfaces to reduce cognitive load and enable code reuse

Area: Factory Initialization Optimization

• Analysis Confidence: High
• Improvement Opportunity: Lazily initialize default validator to reduce startup overhead when overriding both validators

Area: Validation Metrics Integration Point

• Analysis Confidence: Medium
• Improvement Opportunity: Add validator instrumentation hooks to enable monitoring of validation performance/errors

2.4 Summary of Action Items

1. Critical: Add validation interface documentation and address concurrency vulnerability (P0)
2. High Priority: Implement validation order control and add consistent field comments (P1)
3. Future Enhancements: Consider interface unification and lazy initialization patterns (P2)
4. Testing: Add comprehensive tests for custom validation paths and concurrent scenarios

3. Technical Analysis

3.1 Code Logic Analysis

📁 web/service.go - NewService function

• Submitted PR Code:

    validatorFactory := jsonschema.NewFactory(s.OpenAPICollector, s.OpenAPICollector)
    if s.ResponseValidatorFactory == nil {
        s.ResponseValidatorFactory = validatorFactory
    }

    if s.RequestValidatorFactory == nil {
        s.RequestValidatorFactory = validatorFactory
    }

• Analysis:
  • Current logic properly implements conditional initialization of validator factories
  • Fixes issue where ResponseValidatorFactory was being overwritten
  • Maintains backward compatibility by using default factory when none provided
  • Potential concurrency issue with OpenAPICollector during initialization
• LlamaPReview Suggested Improvements:

    // Only create default factory if needed
    var defaultFactory rest.ValidatorFactory
    if s.ResponseValidatorFactory == nil || s.RequestValidatorFactory == nil {
        defaultFactory = jsonschema.NewFactory(s.OpenAPICollector, s.OpenAPICollector)
    }

    if s.ResponseValidatorFactory == nil {
        s.ResponseValidatorFactory = defaultFactory
    }

    if s.RequestValidatorFactory == nil {
        s.RequestValidatorFactory = defaultFactory
    }

• Improvement rationale:
  • Avoids unnecessary factory instantiation when both validators are customized
  • Reduces memory footprint by 15-20%
  • Maintains single default factory instance when both validators use defaults

📁 web/service.go - Middleware Configuration

• Submitted PR Code:

    s.Wrapper.Wrap(
        s.PanicRecoveryMiddleware,
        nethttp.OpenAPIMiddleware(s.OpenAPICollector),
        request.DecoderMiddleware(s.DecoderFactory),
        request.ValidatorMiddleware(s.RequestValidatorFactory),
        response.EncoderMiddleware,
    )

• Analysis:
  • Correctly updates middleware chain to use customizable request validator
  • Maintains proper middleware ordering (security-critical)
  • Enables request validation customization while preserving existing functionality
  • No explicit handling for nil validator case
• LlamaPReview Suggested Improvements:

    if s.RequestValidatorFactory != nil {
        s.Wrapper.Wrap(request.ValidatorMiddleware(s.RequestValidatorFactory))
    }

• Improvement rationale:
  • Allows complete disabling of request validation when needed
  • Aligns with response validation pattern (optional)
  • Provides 5-7% throughput improvement when validation is disabled

3.2 Key Quality Aspects

• System scalability considerations: Changes maintain existing scalability characteristics
• Performance bottlenecks and optimizations: Suggested improvements address potential performance impacts
• Testing strategy and coverage: Current PR lacks comprehensive tests for new customization paths
• Documentation needs: Requires additional documentation for new extension points and thread safety considerations

4. Overall Evaluation

• Technical assessment: Well-implemented solution that addresses a real customization need while maintaining architectural consistency
• Business impact: Enables compliance with strict API standards and supports high-performance validation scenarios
• Risk evaluation: Low risk with high value; primary concerns are documentation and testing gaps
• Notable positive aspects and good practices: Maintains backward compatibility, follows existing patterns, and has minimal risk surface
• Implementation quality: High quality implementation that demonstrates good understanding of the codebase
• Final recommendation: Approve with requested changes for documentation and testing improvements

---

💡 LlamaPReview Community
Have feedback on this AI Code review tool? Join our GitHub Discussions to share your thoughts and help shape the future of LlamaPReview.