feat: refactor how annotation are handler to be more inline with the …

…context.Context interface
tigerwill90 · Dec 20, 2024 · d67a4d0 · d67a4d0
1 parent 05a6a33
commit d67a4d0
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 83 deletions.
diff --git a/SECURITY.md b/SECURITY.md
@@ -0,0 +1,6 @@
+## Reporting a Vulnerability
+To report a security issue, do not open a public GitHub issue.
+
+Contact us securely:
+* contact@sylvainmuller.ch
+* Privately on [Mastodon](https://infosec.exchange/@DigitalDissident)
diff --git a/context_test.go b/context_test.go
@@ -131,12 +131,13 @@ func TestContext_Annotations(t *testing.T) {
 		http.MethodGet,
 		"/foo",
 		emptyHandler,
-		WithAnnotations(Annotation{Key: "foo", Value: "bar"}, Annotation{Key: "foo", Value: "baz"}),
-		WithAnnotations(Annotation{Key: "john", Value: 1}),
+		WithAnnotation("foo", "bar"),
+		WithAnnotation("john", 1),
 	)
 	rte := f.Route(http.MethodGet, "/foo")
 	require.NotNil(t, rte)
-	assert.Equal(t, []Annotation{{Key: "foo", Value: "bar"}, {Key: "foo", Value: "baz"}, {Key: "john", Value: 1}}, slices.Collect(rte.Annotations()))
+	assert.Equal(t, "bar", rte.Annotation("foo").(string))
+	assert.Equal(t, 1, rte.Annotation("john").(int))
 }
 
 func TestContext_Clone(t *testing.T) {

diff --git a/error.go b/error.go
@@ -22,6 +22,7 @@ var (
 	ErrSettledTxn              = errors.New("transaction settled")
 	ErrParamKeyTooLarge        = errors.New("parameter key too large")
 	ErrTooManyParams           = errors.New("too many params")
+	ErrInvalidConfig           = errors.New("invalid config")
 )
 
 // RouteConflictError is a custom error type used to represent conflicts when

diff --git a/fox_test.go b/fox_test.go
@@ -3006,48 +3006,35 @@ func TestInsertUpdateAndDeleteWithHostname(t *testing.T) {
 		},
 	}
 
-	insertAnnot := Annotation{
-		Key:   "foo",
-		Value: "bar",
-	}
-	updateAnnot := Annotation{
-		Key:   "john",
-		Value: "doe",
-	}
-	updateAnnot2 := Annotation{
-		Key:   "billi",
-		Value: "boulou",
-	}
-
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			f := New()
 			routeCopy := make([]struct{ path string }, len(tc.routes))
 			copy(routeCopy, tc.routes)
 
 			for _, rte := range tc.routes {
-				require.NoError(t, onlyError(f.Handle(http.MethodGet, rte.path, emptyHandler, WithAnnotations(insertAnnot))))
+				require.NoError(t, onlyError(f.Handle(http.MethodGet, rte.path, emptyHandler, WithAnnotation("foo", "bar"))))
 			}
 			for _, rte := range tc.routes {
-				require.NoError(t, onlyError(f.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotations(updateAnnot))))
+				require.NoError(t, onlyError(f.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotation("foo", "bar"))))
 			}
 			for _, rte := range tc.routes {
 				r := f.Route(http.MethodGet, rte.path)
 				require.NotNilf(t, r, "missing method=%s;path=%s", http.MethodGet, rte.path)
-				assert.Equal(t, []Annotation{updateAnnot}, slices.Collect(r.Annotations()))
+				assert.Equal(t, "bar", r.Annotation("foo").(string))
 			}
 
 			for _, rte := range tc.routes {
 				require.NoError(t, f.Delete(http.MethodGet, rte.path))
 				routeCopy = slices.Delete(routeCopy, 0, 1)
 				assert.Falsef(t, f.Has(http.MethodGet, rte.path), "found method=%s;path=%s", http.MethodGet, rte.path)
 				for _, rte := range routeCopy {
-					require.NoError(t, onlyError(f.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotations(updateAnnot2))))
+					require.NoError(t, onlyError(f.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotation("john", "doe"))))
 				}
 				for _, rte := range routeCopy {
 					r := f.Route(http.MethodGet, rte.path)
 					require.NotNilf(t, r, "missing method=%s;path=%s", http.MethodGet, rte.path)
-					assert.Equal(t, []Annotation{updateAnnot2}, slices.Collect(r.Annotations()))
+					assert.Equal(t, "doe", r.Annotation("john").(string))
 				}
 			}
 
@@ -3176,19 +3163,6 @@ func TestInsertUpdateAndDeleteWithHostnameTxn(t *testing.T) {
 		},
 	}
 
-	insertAnnot := Annotation{
-		Key:   "foo",
-		Value: "bar",
-	}
-	updateAnnot := Annotation{
-		Key:   "john",
-		Value: "doe",
-	}
-	updateAnnot2 := Annotation{
-		Key:   "billi",
-		Value: "boulou",
-	}
-
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			f := New()
@@ -3197,15 +3171,15 @@ func TestInsertUpdateAndDeleteWithHostnameTxn(t *testing.T) {
 
 			require.NoError(t, f.Updates(func(txn *Txn) error {
 				for _, rte := range tc.routes {
-					if err := onlyError(txn.Handle(http.MethodGet, rte.path, emptyHandler, WithAnnotations(insertAnnot))); err != nil {
+					if err := onlyError(txn.Handle(http.MethodGet, rte.path, emptyHandler, WithAnnotation("foo", "bar"))); err != nil {
 						return err
 					}
 				}
 				return nil
 			}))
 			require.NoError(t, f.Updates(func(txn *Txn) error {
 				for _, rte := range tc.routes {
-					if err := onlyError(txn.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotations(updateAnnot))); err != nil {
+					if err := onlyError(txn.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotation("foo", "bar"))); err != nil {
 						return err
 					}
 				}
@@ -3215,7 +3189,7 @@ func TestInsertUpdateAndDeleteWithHostnameTxn(t *testing.T) {
 			for _, rte := range tc.routes {
 				r := f.Route(http.MethodGet, rte.path)
 				require.NotNilf(t, r, "missing method=%s;path=%s", http.MethodGet, rte.path)
-				assert.Equal(t, []Annotation{updateAnnot}, slices.Collect(r.Annotations()))
+				assert.Equal(t, "bar", r.Annotation("foo").(string))
 			}
 
 			require.NoError(t, f.Updates(func(txn *Txn) error {
@@ -3226,14 +3200,14 @@ func TestInsertUpdateAndDeleteWithHostnameTxn(t *testing.T) {
 					routeCopy = slices.Delete(routeCopy, 0, 1)
 					assert.Falsef(t, txn.Has(http.MethodGet, rte.path), "found method=%s;path=%s", http.MethodGet, rte.path)
 					for _, rte := range routeCopy {
-						if err := onlyError(txn.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotations(updateAnnot2))); err != nil {
+						if err := onlyError(txn.Update(http.MethodGet, rte.path, emptyHandler, WithAnnotation("john", "doe"))); err != nil {
 							return err
 						}
 					}
 					for _, rte := range routeCopy {
 						r := txn.Route(http.MethodGet, rte.path)
 						if !assert.NotNilf(t, r, "missing method=%s;path=%s", http.MethodGet, rte.path) {
-							assert.Equal(t, []Annotation{updateAnnot2}, slices.Collect(r.Annotations()))
+							assert.Equal(t, "doe", r.Annotation("john").(string))
 						}
 					}
 				}
@@ -5805,9 +5779,7 @@ func TestDataRace(t *testing.T) {
 			defer wg.Done()
 			for route := range iterutil.Right(f.Iter().All()) {
 				route.Pattern()
-				for range route.Annotations() {
-
-				}
+				route.Annotation("foo")
 			}
 		}()
 

diff --git a/options.go b/options.go
@@ -6,6 +6,7 @@ package fox
 
 import (
 	"cmp"
+	"reflect"
 )
 
 type Option interface {
@@ -237,13 +238,21 @@ func WithClientIPResolver(resolver ClientIPResolver) Option {
 	})
 }
 
-// WithAnnotations attach arbitrary metadata to routes. Annotations are key-value pairs that allow middleware, handler or
+// WithAnnotation attach arbitrary metadata to routes. Annotations are key-value pairs that allow middleware, handler or
 // any other components to modify behavior based on the attached metadata. Unlike context-based metadata, which is tied to
 // the request lifetime, annotations are bound to the route's lifetime and remain static across all requests for that route.
-// Annotations must be explicitly reapplied when updating a route.
-func WithAnnotations(annotations ...Annotation) RouteOption {
+// The provided key must be comparable and should not be of type string or any other built-in type to avoid collisions between
+// packages that use route annotation. Annotations must be explicitly reapplied when updating a route.
+func WithAnnotation(key, value any) RouteOption {
 	return routeOptionFunc(func(route *Route) {
-		route.annots = append(route.annots, annotations...)
+		if !reflect.TypeOf(key).Comparable() {
+			// TODO returns errors
+			panic("key is not comparable")
+		}
+		if route.annots == nil {
+			route.annots = make(map[any]any, 1)
+		}
+		route.annots[key] = value
 	})
 }
 

diff --git a/recovery.go b/recovery.go
@@ -82,16 +82,11 @@ func recovery(logger *slog.Logger, c Context, handle RecoveryFunc) {
 		sb.WriteString(stacktrace(3, 6))
 
 		params := slices.Collect(mapParamsToAttr(c.Params()))
-		var annotations []any
-		if route := c.Route(); route != nil {
-			annotations = slices.Collect(mapAnnotationsToAttr(route.Annotations()))
-		}
 
 		logger.Error(
 			sb.String(),
 			slog.String("route", c.Pattern()),
 			slog.Group("params", params...),
-			slog.Group("annotations", annotations...),
 			slog.Any("error", err),
 		)
 
@@ -149,13 +144,3 @@ func mapParamsToAttr(params iter.Seq[Param]) iter.Seq[any] {
 		}
 	}
 }
-
-func mapAnnotationsToAttr(annotations iter.Seq[Annotation]) iter.Seq[any] {
-	return func(yield func(any) bool) {
-		for a := range annotations {
-			if !yield(slog.Any(a.Key, a.Value)) {
-				break
-			}
-		}
-	}
-}
diff --git a/route.go b/route.go
@@ -1,17 +1,5 @@
 package fox
 
-import (
-	"iter"
-)
-
-// Annotation represents a single key-value pair that provides metadata for a route.
-// Annotations are typically used to store information that can be leveraged by middleware, handlers, or external
-// libraries to modify or customize route behavior.
-type Annotation struct {
-	Key   string
-	Value any
-}
-
 // Route represent a registered route in the router.
 type Route struct {
 	clientip              ClientIPResolver
@@ -20,7 +8,7 @@ type Route struct {
 	hall                  HandlerFunc
 	pattern               string
 	mws                   []middleware
-	annots                []Annotation
+	annots                map[any]any
 	hostSplit             int // 0 if no host
 	redirectTrailingSlash bool
 	ignoreTrailingSlash   bool
@@ -53,15 +41,10 @@ func (r *Route) Path() string {
 	return r.pattern[r.hostSplit:]
 }
 
-// Annotations returns a range iterator over annotations associated with the route.
-func (r *Route) Annotations() iter.Seq[Annotation] {
-	return func(yield func(Annotation) bool) {
-		for _, a := range r.annots {
-			if !yield(a) {
-				return
-			}
-		}
-	}
+// Annotation returns the value associated with this [Route] for key, or nil if no value is associated with key.
+// Successive calls to Annotation with the same key returns the same result.
+func (r *Route) Annotation(key any) any {
+	return r.annots[key]
 }
 
 // RedirectTrailingSlashEnabled returns whether the route is configured to automatically