Merge pull request #86 from ggicci/feat/no-runtime-reorder-directives

feat: copy a separate resolver for scanning

core/core.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"mime"
 	"net/http"
+	"sort"
 	"sync"
 
 	"github.com/ggicci/owl"
@@ -16,10 +17,12 @@ var builtResolvers sync.Map // map[reflect.Type]*owl.Resolver
 // Who is responsible for decoding an HTTP request to an instance of such struct
 // type.
 type Core struct {
-	resolver               *owl.Resolver
+	resolver               *owl.Resolver // for decoding
+	scanResolver           *owl.Resolver // for encoding
 	errorHandler           ErrorHandler
 	maxMemory              int64 // in bytes
 	enableNestedDirectives bool
+	resolverMu             sync.RWMutex
 }
 
 // New creates a new Core instance for the given intpuStruct. It will build a resolver
@@ -99,6 +102,7 @@ func (c *Core) NewRequest(method string, url string, input any) (*http.Request,
 // performance penalty for doing so. Because there's a cache layer for all the
 // Core instances.
 func (c *Core) NewRequestWithContext(ctx context.Context, method string, url string, input any) (*http.Request, error) {
+	c.prepareScanResolver()
 	req, err := http.NewRequestWithContext(ctx, method, url, nil)
 	if err != nil {
 		return nil, err
@@ -107,21 +111,11 @@ func (c *Core) NewRequestWithContext(ctx context.Context, method string, url str
 	rb := &RequestBuilder{}
 
 	// NOTE(ggicci): the error returned a joined error by using errors.Join.
-	if err = c.resolver.Scan(
+	if err = c.scanResolver.Scan(
 		input,
 		owl.WithNamespace(encoderNamespace),
 		owl.WithValue(CtxRequestBuilder, rb),
 		owl.WithNestedDirectivesEnabled(c.enableNestedDirectives),
-
-		// FIXME(ggicci): reorder the directives only once at the build time to improve the performance.
-		owl.WithDirectiveRunOrder(func(d1, d2 *owl.Directive) bool {
-			if d1.Name == "default" {
-				return true // always the first one to run
-			} else if d1.Name == "nonzero" {
-				return true // always the second one to run
-			}
-			return false
-		}),
 	); err != nil {
 		// err is a list of *owl.ScanError that joined by errors.Join.
 		if errs, ok := err.(interface{ Unwrap() []error }); ok {
@@ -153,6 +147,27 @@ func (c *Core) GetErrorHandler() ErrorHandler {
 	return globalCustomErrorHandler
 }
 
+func (c *Core) prepareScanResolver() {
+	c.resolverMu.RLock()
+	if c.scanResolver == nil {
+		c.resolverMu.RUnlock()
+		c.resolverMu.Lock()
+		defer c.resolverMu.Unlock()
+
+		if c.scanResolver == nil {
+			c.scanResolver = c.resolver.Copy()
+
+			// Reorder the directives to make sure the "default" and "nonzero" directives work properly.
+			c.scanResolver.Iterate(func(r *owl.Resolver) error {
+				sort.Sort(directiveOrderForEncoding(r.Directives))
+				return nil
+			})
+		}
+	} else {
+		c.resolverMu.RUnlock()
+	}
+}
+
 // buildResolver builds a resolver for the inputStruct. It will run normalizations
 // on the resolver and cache it.
 func buildResolver(inputStruct any) (*owl.Resolver, error) {
@@ -240,3 +255,22 @@ func ensureDirectiveExecutorsRegistered(r *owl.Resolver) error {
 	}
 	return nil
 }
+
+type directiveOrderForEncoding []*owl.Directive
+
+func (d directiveOrderForEncoding) Len() int {
+	return len(d)
+}
+
+func (d directiveOrderForEncoding) Less(i, j int) bool {
+	if d[i].Name == "default" {
+		return true // always the first one to run
+	} else if d[i].Name == "nonzero" {
+		return true // always the second one to run
+	}
+	return false
+}
+
+func (d directiveOrderForEncoding) Swap(i, j int) {
+	d[i], d[j] = d[j], d[i]
+}

go.mod

@@ -3,7 +3,7 @@ module github.com/ggicci/httpin
 go 1.20
 
 require (
-	github.com/ggicci/owl v0.6.1
+	github.com/ggicci/owl v0.7.0
 	github.com/go-chi/chi/v5 v5.0.11
 	github.com/gorilla/mux v1.8.1
 	github.com/justinas/alice v1.2.0

go.sum

@@ -2,6 +2,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/ggicci/owl v0.6.1 h1:VSnpN2z+/fTA9BWeRyZOvWEE+qfJkKMl8xNodsElANs=
 github.com/ggicci/owl v0.6.1/go.mod h1:TRPWshRwYej6uES//YW5aNgLB370URwyta1Ytfs7KXs=
+github.com/ggicci/owl v0.7.0 h1:+AMlCR0AY7j72q7hjtN4pm8VJiikwpROtMgvPnXtuik=
+github.com/ggicci/owl v0.7.0/go.mod h1:TRPWshRwYej6uES//YW5aNgLB370URwyta1Ytfs7KXs=
 github.com/go-chi/chi/v5 v5.0.8 h1:lD+NLqFcAi1ovnVZpsnObHGW4xb4J8lNmoYVfECH1Y0=
 github.com/go-chi/chi/v5 v5.0.8/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
 github.com/go-chi/chi/v5 v5.0.11 h1:BnpYbFZ3T3S1WMpD79r7R5ThWX40TaFB7L31Y8xqSwA=

httpin_test.go

@@ -58,6 +58,31 @@ func TestDecode(t *testing.T) {
 	}()
 }
 
+func TestDecode_ErrBuildResolverFailed(t *testing.T) {
+	r, _ := http.NewRequest("GET", "/", nil)
+	r.Form = url.Values{
+		"page":     {"1"},
+		"per_page": {"100"},
+	}
+
+	type Foo struct {
+		Name string `in:"nonexistent=foo"`
+	}
+
+	assert.Error(t, Decode(r, &Foo{}))
+}
+
+func TestDecode_ErrDecodeFailure(t *testing.T) {
+	r, _ := http.NewRequest("GET", "/", nil)
+	r.Form = url.Values{
+		"page":     {"1"},
+		"per_page": {"one-hundred"},
+	}
+
+	p := &Pagination{}
+	assert.Error(t, Decode(r, p))
+}
+
 type EchoInput struct {
 	Token  string `in:"form=access_token;header=x-api-key;required"`
 	Saying string `in:"form=saying"`
@@ -153,3 +178,8 @@ func TestNewRequest(t *testing.T) {
 	expected.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 	assert.Equal(t, expected, req)
 }
+
+func TestNewRequest_ErrNewFailure(t *testing.T) {
+	_, err := NewRequest("GET", "/products", 123)
+	assert.Error(t, err)
+}