From 11fa6e43a5da09adeabfeea40e452de2f416959e Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 16:25:03 +0300 Subject: [PATCH 01/21] g --- mux/mux_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mux/mux_test.go b/mux/mux_test.go index 2938aced..42916524 100644 --- a/mux/mux_test.go +++ b/mux/mux_test.go @@ -11,6 +11,16 @@ import ( "go.akshayshah.org/attest" ) +func tarpitRoutes() []Route { + return []Route{ + NewRoute( + "/libraries/joomla/", + MethodAll, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), + ), + } +} + func TestNew(t *testing.T) { l := log.New(context.Background(), &bytes.Buffer{}, 500) @@ -41,13 +51,3 @@ func TestNew(t *testing.T) { _ = New(config.DevOpts(l, "secretKey12@34String"), nil, rtz...) }) } - -func tarpitRoutes() []Route { - return []Route{ - NewRoute( - "/libraries/joomla/", - MethodAll, - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), - ), - } -} From 6ac6ebb5b86b402217e55c5697c80147da9e05b0 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 18:23:10 +0300 Subject: [PATCH 02/21] g --- internal/mx/mx.go | 13 +++++++++++++ internal/mx/mx_test.go | 25 +++++++++++++++++++++++++ mux/mux.go | 5 +++++ mux/mux_test.go | 17 +++++++++++++++++ 4 files changed, 60 insertions(+) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index 162e0e16..504c3656 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -132,6 +132,19 @@ func (m Muxer) AddRoute(rt Route) error { ) } +// Merge combines the mxs into m. The resulting muxer +func (m Muxer) Merge(mxs []Muxer) Muxer { + if len(mxs) < 1 { + return m + } + + for _, v := range mxs { + m.router.routes = append(m.router.routes, v.router.routes...) + } + + return m +} + // Param gets the path/url parameter from the specified Context. func Param(ctx context.Context, param string) string { vStr, ok := ctx.Value(muxContextKey(param)).(string) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index 0e3562c9..0d1d2872 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -406,6 +406,31 @@ func TestMux(t *testing.T) { attest.Equal(t, string(rb2), "someOtherMuxHandler") } }) + + t.Run("merge", func(t *testing.T) { + t.Parallel() + + httpsPort := tst.GetPort() + domain := "localhost" + + rt1, err := NewRoute("/abc", MethodGet, someMuxHandler("hello")) + attest.Ok(t, err) + + mux1, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt1) + attest.Ok(t, err) + + rt2, err := NewRoute("/xyz", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + attest.Ok(t, err) + + mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2) + attest.Ok(t, err) + + m := mux1.Merge([]Muxer{mux2}) + fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) + attest.Equal(t, m.opt, mux1.opt) + attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) + attest.Equal(t, len(m.router.routes), 2) + }) } func TestMuxFlexiblePattern(t *testing.T) { diff --git a/mux/mux.go b/mux/mux.go index d6414203..9ec1094c 100644 --- a/mux/mux.go +++ b/mux/mux.go @@ -24,6 +24,11 @@ const ( MethodTrace = mx.MethodTrace ) +// func Merge(m []Muxer) Muxer { + +// nw:=New(m[0].internalMux) +// } + // Muxer is a HTTP request multiplexer. // // It matches the URL of each incoming request against a list of registered diff --git a/mux/mux_test.go b/mux/mux_test.go index 42916524..3ce99b2d 100644 --- a/mux/mux_test.go +++ b/mux/mux_test.go @@ -51,3 +51,20 @@ func TestNew(t *testing.T) { _ = New(config.DevOpts(l, "secretKey12@34String"), nil, rtz...) }) } + +// func TestMergeMux(t *testing.T) { +// l := log.New(context.Background(), &bytes.Buffer{}, 500) + +// rt1 := []Route{ +// NewRoute("/home", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), +// NewRoute("/health/", MethodAll, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), +// } + +// rt2 := []Route{ +// NewRoute("/uri2", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), +// } + +// mx1 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt1...) +// mx2 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt2...) + +// } From 7dd3f8574b1aa9b5b9502fcc48cf1926f0e59765 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 18:23:35 +0300 Subject: [PATCH 03/21] g --- internal/mx/mx.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index 504c3656..ff40744a 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -138,6 +138,7 @@ func (m Muxer) Merge(mxs []Muxer) Muxer { return m } + // TODO: detect conflicts. for _, v := range mxs { m.router.routes = append(m.router.routes, v.router.routes...) } From c2443e8a47db7efec12c454fb7975a4d665f5442 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 18:28:41 +0300 Subject: [PATCH 04/21] g --- internal/mx/mx_test.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index 0d1d2872..4413a859 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -413,23 +413,27 @@ func TestMux(t *testing.T) { httpsPort := tst.GetPort() domain := "localhost" - rt1, err := NewRoute("/abc", MethodGet, someMuxHandler("hello")) - attest.Ok(t, err) + { + rt1, err := NewRoute("/abc", MethodGet, someMuxHandler("hello")) + attest.Ok(t, err) - mux1, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt1) - attest.Ok(t, err) + mux1, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt1) + attest.Ok(t, err) - rt2, err := NewRoute("/xyz", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - attest.Ok(t, err) + rt2, err := NewRoute("/ijk", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + attest.Ok(t, err) + rt3, err := NewRoute("/xyz", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + attest.Ok(t, err) - mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2) - attest.Ok(t, err) + mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2, rt3) + attest.Ok(t, err) - m := mux1.Merge([]Muxer{mux2}) - fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) - attest.Equal(t, m.opt, mux1.opt) - attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) - attest.Equal(t, len(m.router.routes), 2) + m := mux1.Merge([]Muxer{mux2}) + fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) + attest.Equal(t, m.opt, mux1.opt) + attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) + attest.Equal(t, len(m.router.routes), 3) + } }) } From c53a5d6ffe7db62a8364f0b0811a5e2abc49c8d0 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 18:32:17 +0300 Subject: [PATCH 05/21] g --- internal/mx/mx_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index 4413a859..5ffe91a0 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -434,6 +434,29 @@ func TestMux(t *testing.T) { attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) attest.Equal(t, len(m.router.routes), 3) } + + { // conflict + rt1, err := NewRoute("/abc", MethodGet, someMuxHandler("hello")) + attest.Ok(t, err) + + mux1, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt1) + attest.Ok(t, err) + + rt2, err := NewRoute("/ijk", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + attest.Ok(t, err) + rt3, err := NewRoute("/abc", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + attest.Ok(t, err) + + mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2, rt3) + attest.Ok(t, err) + + m := mux1.Merge([]Muxer{mux2}) + fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) + attest.Equal(t, m.opt, mux1.opt) + attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) + attest.Equal(t, len(m.router.routes), 3) + attest.True(t, "TODO:" == "assert that there's a panic for conflict") + } }) } From a354f685d7e4fab9be62954d25841262a9d6cc30 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 22:32:00 +0300 Subject: [PATCH 06/21] g --- internal/mx/mx.go | 67 +++++++++++++++++++++++++++++++++++++++--- internal/mx/mx_test.go | 17 +++++++---- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index ff40744a..e0dc6a4d 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -8,6 +8,9 @@ import ( "fmt" "net/http" "net/url" + "path" + "slices" + "strings" "github.com/komuw/ong/config" "github.com/komuw/ong/middleware" @@ -132,10 +135,10 @@ func (m Muxer) AddRoute(rt Route) error { ) } -// Merge combines the mxs into m. The resulting muxer -func (m Muxer) Merge(mxs []Muxer) Muxer { +// Merge combines mxs into m. The resulting muxer uses the opts & notFoundHandler of m. +func (m Muxer) Merge(mxs []Muxer) (Muxer, error) { if len(mxs) < 1 { - return m + return m, nil } // TODO: detect conflicts. @@ -143,7 +146,63 @@ func (m Muxer) Merge(mxs []Muxer) Muxer { m.router.routes = append(m.router.routes, v.router.routes...) } - return m + // TODO: merge this logic with `router.detectConflict` + for k := range m.router.routes { + incoming := m.router.routes[k] // TODO: rename this to candidate. + pattern := incoming.pattern + incomingSegments := pathSegments(pattern) + + for _, rt := range m.router.routes { + if pattern == rt.pattern && (slices.Equal(incoming.segments, rt.segments)) { + // TODO: this is bad, we should not include the one for incoming. + // Is this enough?? + continue + } + + existingSegments := rt.segments + sameLen := len(incomingSegments) == len(existingSegments) + if !sameLen { + // no conflict + continue + } + + errMsg := fmt.Errorf(` +You are trying to add + pattern: %s + method: %s + handler: %v +However + pattern: %s + method: %s + handler: %v +already exists and would conflict`, + pattern, + strings.ToUpper(incoming.method), + getfunc(incoming.originalHandler), + path.Join(rt.segments...), + strings.ToUpper(rt.method), + getfunc(rt.originalHandler), + ) + + if len(existingSegments) == 1 && existingSegments[0] == "*" && len(incomingSegments) > 0 { + return m, errMsg + } + + if pattern == rt.pattern { + return m, errMsg + } + + if strings.Contains(pattern, ":") && (incomingSegments[0] == existingSegments[0]) { + return m, errMsg + } + + if strings.Contains(rt.pattern, ":") && (incomingSegments[0] == existingSegments[0]) { + return m, errMsg + } + } + } + + return m, nil } // Param gets the path/url parameter from the specified Context. diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index 5ffe91a0..26fbe9f3 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -413,7 +413,8 @@ func TestMux(t *testing.T) { httpsPort := tst.GetPort() domain := "localhost" - { + t.Run("success", func(t *testing.T) { + t.Parallel() rt1, err := NewRoute("/abc", MethodGet, someMuxHandler("hello")) attest.Ok(t, err) @@ -428,14 +429,16 @@ func TestMux(t *testing.T) { mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2, rt3) attest.Ok(t, err) - m := mux1.Merge([]Muxer{mux2}) + m, err := mux1.Merge([]Muxer{mux2}) + attest.Ok(t, err) fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) + attest.Equal(t, m.opt, mux1.opt) attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) attest.Equal(t, len(m.router.routes), 3) - } + }) - { // conflict + t.Run("conflict", func(t *testing.T) { rt1, err := NewRoute("/abc", MethodGet, someMuxHandler("hello")) attest.Ok(t, err) @@ -450,13 +453,15 @@ func TestMux(t *testing.T) { mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2, rt3) attest.Ok(t, err) - m := mux1.Merge([]Muxer{mux2}) + m, err := mux1.Merge([]Muxer{mux2}) + attest.Ok(t, err) fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) + attest.Equal(t, m.opt, mux1.opt) attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) attest.Equal(t, len(m.router.routes), 3) attest.True(t, "TODO:" == "assert that there's a panic for conflict") - } + }) }) } From 578a93c25668fac76fcfdcdc2b9de454060114f8 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 22:48:51 +0300 Subject: [PATCH 07/21] g --- internal/mx/mx.go | 2 +- internal/mx/mx_test.go | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index e0dc6a4d..7643ebef 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -153,7 +153,7 @@ func (m Muxer) Merge(mxs []Muxer) (Muxer, error) { incomingSegments := pathSegments(pattern) for _, rt := range m.router.routes { - if pattern == rt.pattern && (slices.Equal(incoming.segments, rt.segments)) { + if pattern == rt.pattern && (slices.Equal(incoming.segments, rt.segments)) && (getfunc(incoming.originalHandler) == getfunc(rt.originalHandler)) { // TODO: this is bad, we should not include the one for incoming. // Is this enough?? continue diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index 26fbe9f3..7d9dc985 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -453,14 +453,12 @@ func TestMux(t *testing.T) { mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2, rt3) attest.Ok(t, err) - m, err := mux1.Merge([]Muxer{mux2}) - attest.Ok(t, err) - fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) - - attest.Equal(t, m.opt, mux1.opt) - attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) - attest.Equal(t, len(m.router.routes), 3) - attest.True(t, "TODO:" == "assert that there's a panic for conflict") + _, errM := mux1.Merge([]Muxer{mux2}) + attest.Error(t, errM) + rStr := errM.Error() + attest.Subsequence(t, rStr, "would conflict") + attest.Subsequence(t, rStr, "ong/internal/mx/mx_test.go:28") // location where `someMuxHandler` is declared. + attest.Subsequence(t, rStr, "ong/internal/mx/mx_test.go:450") // location where the other handler is declared. }) }) } From 74fcbbbf6a269d50a0d511c4ee71335cb5b9cb69 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 22:54:15 +0300 Subject: [PATCH 08/21] g --- internal/mx/mx.go | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index 7643ebef..1bc542e8 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -146,7 +146,23 @@ func (m Muxer) Merge(mxs []Muxer) (Muxer, error) { m.router.routes = append(m.router.routes, v.router.routes...) } - // TODO: merge this logic with `router.detectConflict` + if err := detectConflict(m); err != nil { + return m, err + } + + return m, nil +} + +// Param gets the path/url parameter from the specified Context. +func Param(ctx context.Context, param string) string { + vStr, ok := ctx.Value(muxContextKey(param)).(string) + if !ok { + return "" + } + return vStr +} + +func detectConflict(m Muxer) error { for k := range m.router.routes { incoming := m.router.routes[k] // TODO: rename this to candidate. pattern := incoming.pattern @@ -185,31 +201,22 @@ already exists and would conflict`, ) if len(existingSegments) == 1 && existingSegments[0] == "*" && len(incomingSegments) > 0 { - return m, errMsg + return errMsg } if pattern == rt.pattern { - return m, errMsg + return errMsg } if strings.Contains(pattern, ":") && (incomingSegments[0] == existingSegments[0]) { - return m, errMsg + return errMsg } if strings.Contains(rt.pattern, ":") && (incomingSegments[0] == existingSegments[0]) { - return m, errMsg + return errMsg } } } - return m, nil -} - -// Param gets the path/url parameter from the specified Context. -func Param(ctx context.Context, param string) string { - vStr, ok := ctx.Value(muxContextKey(param)).(string) - if !ok { - return "" - } - return vStr + return nil } From 12ecc649585af4251a213edbc1d96910417a5e42 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:07:29 +0300 Subject: [PATCH 09/21] g --- internal/mx/mx.go | 30 +++++++++++++++++- internal/mx/route.go | 73 -------------------------------------------- 2 files changed, 29 insertions(+), 74 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index 1bc542e8..e2cd0e15 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -88,9 +88,15 @@ func New(opt config.Opts, notFoundHandler http.Handler, routes ...Route) (Muxer, } } + // Try and detect conflicting routes. + if err := detectConflict(m); err != nil { + return Muxer{}, err + } + return m, nil } +// TODO: remove error return from this method. func (m Muxer) addPattern(method, pattern string, originalHandler, wrappingHandler http.Handler) error { return m.router.handle(method, pattern, originalHandler, wrappingHandler) } @@ -127,12 +133,19 @@ func (m Muxer) Resolve(path string) Route { // Users of ong should not use this method. Instead, pass all your routes when calling [New] func (m Muxer) AddRoute(rt Route) error { // AddRoute should only be used internally by ong. - return m.addPattern( + m.addPattern( rt.method, rt.pattern, rt.originalHandler, middleware.All(rt.originalHandler, m.opt), ) + + // Try and detect conflicting routes. + if err := detectConflict(m); err != nil { + return err + } + + return nil } // Merge combines mxs into m. The resulting muxer uses the opts & notFoundHandler of m. @@ -162,6 +175,21 @@ func Param(ctx context.Context, param string) string { return vStr } +// detectConflict returns an error with a diagnostic message when you try to add a route that would conflict with an already existing one. +// +// The error message looks like: +// +// You are trying to add +// pattern: /post/:id/ +// method: GET +// handler: github.com/myAPp/server/main.loginHandler - /home/server/main.go:351 +// However +// pattern: post/create +// method: GET +// handler: github.com/myAPp/server/main.logoutHandler - /home/server/main.go:345 +// already exists and would conflict. +// +// / func detectConflict(m Muxer) error { for k := range m.router.routes { incoming := m.router.routes[k] // TODO: rename this to candidate. diff --git a/internal/mx/route.go b/internal/mx/route.go index eb0a6bc8..07040447 100644 --- a/internal/mx/route.go +++ b/internal/mx/route.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net/http" - "path" "reflect" "runtime" "strings" @@ -152,11 +151,6 @@ func (r *router) handle(method, pattern string, originalHandler, wrappingHandler pattern = "/" + pattern } - // Try and detect conflict before adding a new route. - if err := r.detectConflict(method, pattern, originalHandler); err != nil { - return err - } - rt := Route{ method: strings.ToUpper(method), pattern: pattern, @@ -182,73 +176,6 @@ func (r *router) serveHTTP(w http.ResponseWriter, req *http.Request) { r.notFoundHandler.ServeHTTP(w, req) } -// detectConflict returns an error with a diagnostic message when you try to add a route that would conflict with an already existing one. -// -// The error message looks like: -// -// You are trying to add -// pattern: /post/:id/ -// method: GET -// handler: github.com/myAPp/server/main.loginHandler - /home/server/main.go:351 -// However -// pattern: post/create -// method: GET -// handler: github.com/myAPp/server/main.logoutHandler - /home/server/main.go:345 -// already exists and would conflict. -// -// / -func (r *router) detectConflict(method, pattern string, originalHandler http.Handler) error { - // Conflicting routes are a bad thing. - // They can be a source of bugs and confusion. - // see: https://www.alexedwards.net/blog/which-go-router-should-i-use - - incomingSegments := pathSegments(pattern) - for _, rt := range r.routes { - existingSegments := rt.segments - sameLen := len(incomingSegments) == len(existingSegments) - if !sameLen { - // no conflict - continue - } - - errMsg := fmt.Errorf(` -You are trying to add - pattern: %s - method: %s - handler: %v -However - pattern: %s - method: %s - handler: %v -already exists and would conflict`, - pattern, - strings.ToUpper(method), - getfunc(originalHandler), - path.Join(rt.segments...), - strings.ToUpper(rt.method), - getfunc(rt.originalHandler), - ) - - if len(existingSegments) == 1 && existingSegments[0] == "*" && len(incomingSegments) > 0 { - return errMsg - } - - if pattern == rt.pattern { - return errMsg - } - - if strings.Contains(pattern, ":") && (incomingSegments[0] == existingSegments[0]) { - return errMsg - } - - if strings.Contains(rt.pattern, ":") && (incomingSegments[0] == existingSegments[0]) { - return errMsg - } - } - - return nil -} - func getfunc(handler http.Handler) string { fn := runtime.FuncForPC(reflect.ValueOf(handler).Pointer()) file, line := fn.FileLine(fn.Entry()) From 31d746ccf46fbb0ec80d94a3eb078191f6969ff2 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:08:28 +0300 Subject: [PATCH 10/21] g --- internal/mx/mx_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index 7d9dc985..e769018f 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -431,7 +431,6 @@ func TestMux(t *testing.T) { m, err := mux1.Merge([]Muxer{mux2}) attest.Ok(t, err) - fmt.Println("\n\t m:: ", m, fmt.Sprintf("%p", m.router.notFoundHandler)) attest.Equal(t, m.opt, mux1.opt) attest.Equal(t, fmt.Sprintf("%p", m.router.notFoundHandler), fmt.Sprintf("%p", mux1.router.notFoundHandler)) From cc4bd0b28801ee5e3be861f9cb3a029bd95d633b Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:09:13 +0300 Subject: [PATCH 11/21] g --- internal/mx/mx_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index e769018f..d2f3e3ef 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -457,7 +457,7 @@ func TestMux(t *testing.T) { rStr := errM.Error() attest.Subsequence(t, rStr, "would conflict") attest.Subsequence(t, rStr, "ong/internal/mx/mx_test.go:28") // location where `someMuxHandler` is declared. - attest.Subsequence(t, rStr, "ong/internal/mx/mx_test.go:450") // location where the other handler is declared. + attest.Subsequence(t, rStr, "ong/internal/mx/mx_test.go:449") // location where the other handler is declared. }) }) } From 6f3e27028f0fe97635aee48b8c5e0036a405aa8d Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:11:24 +0300 Subject: [PATCH 12/21] g --- internal/mx/mx_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index d2f3e3ef..ff0a4a85 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -204,7 +204,7 @@ func TestMux(t *testing.T) { msg := "hello world" uri1 := "/api/hi" - uri2 := "/api/:someId" // This conflicts with uri1 + uri2 := "api/:someId" // This conflicts with uri1 method := MethodGet rt1, err := NewRoute( From 6a06287c8da48d87e3b4f5a69d7f395d1ae6de1c Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:21:23 +0300 Subject: [PATCH 13/21] g --- internal/mx/mx_test.go | 77 ++++++++++++++++++ internal/mx/route_test.go | 162 +++++++++++++++++++------------------- 2 files changed, 158 insertions(+), 81 deletions(-) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index ff0a4a85..fedae488 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -462,6 +462,83 @@ func TestMux(t *testing.T) { }) } +func TestConflicts(t *testing.T) { + t.Parallel() + + l := log.New(context.Background(), &bytes.Buffer{}, 500) + + t.Run("conflicts detected", func(t *testing.T) { + t.Parallel() + + msg1 := "firstRoute" + msg2 := "secondRoute" + + rt1, err := NewRoute("/post/create", http.MethodGet, firstRoute(msg1)) + attest.Ok(t, err) + + rt2, err := NewRoute("/post/:id", http.MethodGet, secondRoute(msg2)) + attest.Ok(t, err) + + _, errH := New( + config.WithOpts("localhost", 443, tst.SecretKey(), config.DirectIpStrategy, l), + nil, + rt1, + rt2, + ) + attest.Error(t, errH) + }) + + t.Run("different http methods same path conflicts detected", func(t *testing.T) { + t.Parallel() + r := newRouter(nil) + + msg1 := "firstRoute" + msg2 := "secondRoute" + err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) + attest.Ok(t, err) + + // This one returns with a conflict message. + errH := r.handle(http.MethodGet, "/post/", secondRoute(msg2), secondRoute(msg2)) + attest.Error(t, errH) + + // This one returns with a conflict message. + errB := r.handle(http.MethodDelete, "post/", secondRoute(msg2), secondRoute(msg2)) + attest.Error(t, errB) + + // This one returns with a conflict message. + errC := r.handle(http.MethodPut, "post", secondRoute(msg2), secondRoute(msg2)) + attest.Error(t, errC) + }) + + t.Run("no conflict", func(t *testing.T) { + t.Parallel() + r := newRouter(nil) + + msg1 := "firstRoute-one" + msg2 := "secondRoute-two" + err := r.handle(http.MethodGet, "/w00tw00t.at.blackhats.romanian.anti-sec:)", firstRoute(msg1), firstRoute(msg1)) + attest.Ok(t, err) + + // This one should not conflict. + errH := r.handle(http.MethodGet, "/index.php", secondRoute(msg2), secondRoute(msg2)) + attest.Ok(t, errH) + }) + + t.Run("http MethodAll conflicts with all other methods", func(t *testing.T) { + t.Parallel() + r := newRouter(nil) + + msg1 := "firstRoute" + msg2 := "secondRoute" + err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) + attest.Ok(t, err) + + // This one returns with a conflict message. + errB := r.handle(MethodAll, "post/", secondRoute(msg2), secondRoute(msg2)) + attest.Error(t, errB) + }) +} + func TestMuxFlexiblePattern(t *testing.T) { t.Parallel() diff --git a/internal/mx/route_test.go b/internal/mx/route_test.go index b861f069..fa067b3e 100644 --- a/internal/mx/route_test.go +++ b/internal/mx/route_test.go @@ -3,7 +3,6 @@ package mx import ( "context" "fmt" - "io" "net/http" "net/http/httptest" "testing" @@ -318,86 +317,87 @@ func secondRoute(msg string) http.HandlerFunc { } } -func TestConflicts(t *testing.T) { - t.Parallel() - - t.Run("conflicts detected", func(t *testing.T) { - t.Parallel() - r := newRouter(nil) - - msg1 := "firstRoute" - msg2 := "secondRoute" - err := r.handle(http.MethodGet, "/post/create", firstRoute(msg1), firstRoute(msg1)) - attest.Ok(t, err) - - // This one returns with a conflict message. - errH := r.handle(http.MethodGet, "/post/:id", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errH) - - rec := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/post/create", nil) - r.serveHTTP(rec, req) - - res := rec.Result() - defer res.Body.Close() - - rb, err := io.ReadAll(res.Body) - attest.Ok(t, err) - - attest.Equal(t, res.StatusCode, http.StatusOK) - attest.Equal(t, string(rb), msg1) - }) - - t.Run("different http methods same path conflicts detected", func(t *testing.T) { - t.Parallel() - r := newRouter(nil) - - msg1 := "firstRoute" - msg2 := "secondRoute" - err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) - attest.Ok(t, err) - - // This one returns with a conflict message. - errH := r.handle(http.MethodGet, "/post/", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errH) - - // This one returns with a conflict message. - errB := r.handle(http.MethodDelete, "post/", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errB) - - // This one returns with a conflict message. - errC := r.handle(http.MethodPut, "post", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errC) - }) - - t.Run("no conflict", func(t *testing.T) { - t.Parallel() - r := newRouter(nil) - - msg1 := "firstRoute-one" - msg2 := "secondRoute-two" - err := r.handle(http.MethodGet, "/w00tw00t.at.blackhats.romanian.anti-sec:)", firstRoute(msg1), firstRoute(msg1)) - attest.Ok(t, err) - - // This one should not conflict. - errH := r.handle(http.MethodGet, "/index.php", secondRoute(msg2), secondRoute(msg2)) - attest.Ok(t, errH) - }) - - t.Run("http MethodAll conflicts with all other methods", func(t *testing.T) { - t.Parallel() - r := newRouter(nil) - - msg1 := "firstRoute" - msg2 := "secondRoute" - err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) - attest.Ok(t, err) - - // This one returns with a conflict message. - errB := r.handle(MethodAll, "post/", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errB) - }) -} +// TODO: remove +// func TestConflicts(t *testing.T) { +// t.Parallel() + +// t.Run("conflicts detected", func(t *testing.T) { +// t.Parallel() +// r := newRouter(nil) + +// msg1 := "firstRoute" +// msg2 := "secondRoute" +// err := r.handle(http.MethodGet, "/post/create", firstRoute(msg1), firstRoute(msg1)) +// attest.Ok(t, err) + +// // This one returns with a conflict message. +// errH := r.handle(http.MethodGet, "/post/:id", secondRoute(msg2), secondRoute(msg2)) +// attest.Error(t, errH) + +// rec := httptest.NewRecorder() +// req := httptest.NewRequest(http.MethodGet, "/post/create", nil) +// r.serveHTTP(rec, req) + +// res := rec.Result() +// defer res.Body.Close() + +// rb, err := io.ReadAll(res.Body) +// attest.Ok(t, err) + +// attest.Equal(t, res.StatusCode, http.StatusOK) +// attest.Equal(t, string(rb), msg1) +// }) + +// t.Run("different http methods same path conflicts detected", func(t *testing.T) { +// t.Parallel() +// r := newRouter(nil) + +// msg1 := "firstRoute" +// msg2 := "secondRoute" +// err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) +// attest.Ok(t, err) + +// // This one returns with a conflict message. +// errH := r.handle(http.MethodGet, "/post/", secondRoute(msg2), secondRoute(msg2)) +// attest.Error(t, errH) + +// // This one returns with a conflict message. +// errB := r.handle(http.MethodDelete, "post/", secondRoute(msg2), secondRoute(msg2)) +// attest.Error(t, errB) + +// // This one returns with a conflict message. +// errC := r.handle(http.MethodPut, "post", secondRoute(msg2), secondRoute(msg2)) +// attest.Error(t, errC) +// }) + +// t.Run("no conflict", func(t *testing.T) { +// t.Parallel() +// r := newRouter(nil) + +// msg1 := "firstRoute-one" +// msg2 := "secondRoute-two" +// err := r.handle(http.MethodGet, "/w00tw00t.at.blackhats.romanian.anti-sec:)", firstRoute(msg1), firstRoute(msg1)) +// attest.Ok(t, err) + +// // This one should not conflict. +// errH := r.handle(http.MethodGet, "/index.php", secondRoute(msg2), secondRoute(msg2)) +// attest.Ok(t, errH) +// }) + +// t.Run("http MethodAll conflicts with all other methods", func(t *testing.T) { +// t.Parallel() +// r := newRouter(nil) + +// msg1 := "firstRoute" +// msg2 := "secondRoute" +// err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) +// attest.Ok(t, err) + +// // This one returns with a conflict message. +// errB := r.handle(MethodAll, "post/", secondRoute(msg2), secondRoute(msg2)) +// attest.Error(t, errB) +// }) +// } func TestNotFound(t *testing.T) { t.Parallel() From e436f44045e68c97e6f98efa479a3bdb45e54bbc Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:24:29 +0300 Subject: [PATCH 14/21] g --- internal/mx/mx_test.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index fedae488..c0531bb1 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -490,24 +490,23 @@ func TestConflicts(t *testing.T) { t.Run("different http methods same path conflicts detected", func(t *testing.T) { t.Parallel() - r := newRouter(nil) msg1 := "firstRoute" msg2 := "secondRoute" - err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) - attest.Ok(t, err) - // This one returns with a conflict message. - errH := r.handle(http.MethodGet, "/post/", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errH) + rt1, err := NewRoute("/post", http.MethodGet, firstRoute(msg1)) + attest.Ok(t, err) - // This one returns with a conflict message. - errB := r.handle(http.MethodDelete, "post/", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errB) + rt2, err := NewRoute("post/", http.MethodDelete, secondRoute(msg2)) + attest.Ok(t, err) - // This one returns with a conflict message. - errC := r.handle(http.MethodPut, "post", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errC) + _, errH := New( + config.WithOpts("localhost", 443, tst.SecretKey(), config.DirectIpStrategy, l), + nil, + rt1, + rt2, + ) + attest.Error(t, errH) }) t.Run("no conflict", func(t *testing.T) { From 124a9a44118fd2ecd2e66d20619a4a407ece728b Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:27:54 +0300 Subject: [PATCH 15/21] g --- internal/mx/mx_test.go | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index c0531bb1..47bf4772 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -511,30 +511,44 @@ func TestConflicts(t *testing.T) { t.Run("no conflict", func(t *testing.T) { t.Parallel() - r := newRouter(nil) msg1 := "firstRoute-one" msg2 := "secondRoute-two" - err := r.handle(http.MethodGet, "/w00tw00t.at.blackhats.romanian.anti-sec:)", firstRoute(msg1), firstRoute(msg1)) + + rt1, err := NewRoute("/w00tw00t.at.blackhats.romanian.anti-sec:)", http.MethodGet, firstRoute(msg1)) + attest.Ok(t, err) + + rt2, err := NewRoute("/index.php", http.MethodGet, secondRoute(msg2)) attest.Ok(t, err) - // This one should not conflict. - errH := r.handle(http.MethodGet, "/index.php", secondRoute(msg2), secondRoute(msg2)) + _, errH := New( + config.WithOpts("localhost", 443, tst.SecretKey(), config.DirectIpStrategy, l), + nil, + rt1, + rt2, + ) attest.Ok(t, errH) }) t.Run("http MethodAll conflicts with all other methods", func(t *testing.T) { t.Parallel() - r := newRouter(nil) msg1 := "firstRoute" msg2 := "secondRoute" - err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) + + rt1, err := NewRoute("/post", http.MethodGet, firstRoute(msg1)) + attest.Ok(t, err) + + rt2, err := NewRoute("post/", MethodAll, secondRoute(msg2)) attest.Ok(t, err) - // This one returns with a conflict message. - errB := r.handle(MethodAll, "post/", secondRoute(msg2), secondRoute(msg2)) - attest.Error(t, errB) + _, errH := New( + config.WithOpts("localhost", 443, tst.SecretKey(), config.DirectIpStrategy, l), + nil, + rt1, + rt2, + ) + attest.Error(t, errH) }) } From dce70089abb86983931d84e0d7c5df4f20b26454 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:32:23 +0300 Subject: [PATCH 16/21] g --- internal/mx/mx.go | 10 ++-- internal/mx/route.go | 4 +- internal/mx/route_test.go | 97 ++------------------------------------- 3 files changed, 10 insertions(+), 101 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index e2cd0e15..4187b1ec 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -78,14 +78,12 @@ func New(opt config.Opts, notFoundHandler http.Handler, routes ...Route) (Muxer, mid = middleware.All } - if err := m.addPattern( + m.addPattern( rt.method, rt.pattern, rt.originalHandler, mid(rt.originalHandler, opt), - ); err != nil { - return Muxer{}, err - } + ) } // Try and detect conflicting routes. @@ -97,8 +95,8 @@ func New(opt config.Opts, notFoundHandler http.Handler, routes ...Route) (Muxer, } // TODO: remove error return from this method. -func (m Muxer) addPattern(method, pattern string, originalHandler, wrappingHandler http.Handler) error { - return m.router.handle(method, pattern, originalHandler, wrappingHandler) +func (m Muxer) addPattern(method, pattern string, originalHandler, wrappingHandler http.Handler) { + m.router.handle(method, pattern, originalHandler, wrappingHandler) } // ServeHTTP implements a http.Handler diff --git a/internal/mx/route.go b/internal/mx/route.go index 07040447..c504e8f2 100644 --- a/internal/mx/route.go +++ b/internal/mx/route.go @@ -139,7 +139,7 @@ func pathSegments(p string) []string { // handle adds a handler with the specified method and pattern. // Pattern can contain path segments such as: /item/:id which is // accessible via the Param function. -func (r *router) handle(method, pattern string, originalHandler, wrappingHandler http.Handler) error { +func (r *router) handle(method, pattern string, originalHandler, wrappingHandler http.Handler) { if !strings.HasSuffix(pattern, "/") { // this will make the mux send requests for; // - localhost:80/check @@ -159,8 +159,6 @@ func (r *router) handle(method, pattern string, originalHandler, wrappingHandler wrappingHandler: wrappingHandler, } r.routes = append(r.routes, rt) - - return nil } // serveHTTP routes the incoming http.Request based on method and path extracting path parameters as it goes. diff --git a/internal/mx/route_test.go b/internal/mx/route_test.go index fa067b3e..358589e2 100644 --- a/internal/mx/route_test.go +++ b/internal/mx/route_test.go @@ -247,11 +247,10 @@ func TestRouter(t *testing.T) { match := false var ctx context.Context - err := r.handle(tt.RouteMethod, tt.RoutePattern, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.handle(tt.RouteMethod, tt.RoutePattern, nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { match = true ctx = r.Context() })) - attest.Ok(t, err) req, err := http.NewRequest(tt.Method, tt.Path, nil) attest.Ok(t, err) @@ -284,10 +283,9 @@ func TestMultipleRoutesDifferentMethods(t *testing.T) { r := newRouter(nil) var match string - err := r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { match = r.Method })) - attest.Ok(t, err) req, err := http.NewRequest(http.MethodGet, "/path", nil) attest.Ok(t, err) @@ -317,88 +315,6 @@ func secondRoute(msg string) http.HandlerFunc { } } -// TODO: remove -// func TestConflicts(t *testing.T) { -// t.Parallel() - -// t.Run("conflicts detected", func(t *testing.T) { -// t.Parallel() -// r := newRouter(nil) - -// msg1 := "firstRoute" -// msg2 := "secondRoute" -// err := r.handle(http.MethodGet, "/post/create", firstRoute(msg1), firstRoute(msg1)) -// attest.Ok(t, err) - -// // This one returns with a conflict message. -// errH := r.handle(http.MethodGet, "/post/:id", secondRoute(msg2), secondRoute(msg2)) -// attest.Error(t, errH) - -// rec := httptest.NewRecorder() -// req := httptest.NewRequest(http.MethodGet, "/post/create", nil) -// r.serveHTTP(rec, req) - -// res := rec.Result() -// defer res.Body.Close() - -// rb, err := io.ReadAll(res.Body) -// attest.Ok(t, err) - -// attest.Equal(t, res.StatusCode, http.StatusOK) -// attest.Equal(t, string(rb), msg1) -// }) - -// t.Run("different http methods same path conflicts detected", func(t *testing.T) { -// t.Parallel() -// r := newRouter(nil) - -// msg1 := "firstRoute" -// msg2 := "secondRoute" -// err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) -// attest.Ok(t, err) - -// // This one returns with a conflict message. -// errH := r.handle(http.MethodGet, "/post/", secondRoute(msg2), secondRoute(msg2)) -// attest.Error(t, errH) - -// // This one returns with a conflict message. -// errB := r.handle(http.MethodDelete, "post/", secondRoute(msg2), secondRoute(msg2)) -// attest.Error(t, errB) - -// // This one returns with a conflict message. -// errC := r.handle(http.MethodPut, "post", secondRoute(msg2), secondRoute(msg2)) -// attest.Error(t, errC) -// }) - -// t.Run("no conflict", func(t *testing.T) { -// t.Parallel() -// r := newRouter(nil) - -// msg1 := "firstRoute-one" -// msg2 := "secondRoute-two" -// err := r.handle(http.MethodGet, "/w00tw00t.at.blackhats.romanian.anti-sec:)", firstRoute(msg1), firstRoute(msg1)) -// attest.Ok(t, err) - -// // This one should not conflict. -// errH := r.handle(http.MethodGet, "/index.php", secondRoute(msg2), secondRoute(msg2)) -// attest.Ok(t, errH) -// }) - -// t.Run("http MethodAll conflicts with all other methods", func(t *testing.T) { -// t.Parallel() -// r := newRouter(nil) - -// msg1 := "firstRoute" -// msg2 := "secondRoute" -// err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1)) -// attest.Ok(t, err) - -// // This one returns with a conflict message. -// errB := r.handle(MethodAll, "post/", secondRoute(msg2), secondRoute(msg2)) -// attest.Error(t, errB) -// }) -// } - func TestNotFound(t *testing.T) { t.Parallel() @@ -407,10 +323,9 @@ func TestNotFound(t *testing.T) { r := newRouter(nil) var match string - err := r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { match = r.Method })) - attest.Ok(t, err) req, err := http.NewRequest(http.MethodGet, "/path", nil) attest.Ok(t, err) @@ -427,10 +342,9 @@ func TestNotFound(t *testing.T) { r := newRouter(nil) var match string - err := r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { match = r.Method })) - attest.Ok(t, err) req, err := http.NewRequest(http.MethodGet, "/not-found-path", nil) attest.Ok(t, err) @@ -451,10 +365,9 @@ func TestNotFound(t *testing.T) { }) r := newRouter(notFoundHandler) - err := r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + r.handle(MethodAll, "/path", nil, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { match = r.Method })) - attest.Ok(t, err) req, err := http.NewRequest(http.MethodGet, "/not-found-path", nil) attest.Ok(t, err) From 47543b93180b68b047b7fdd7fd2e672a05297cca Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:34:18 +0300 Subject: [PATCH 17/21] g --- internal/mx/mx.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index 4187b1ec..ec4a26f8 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -94,7 +94,6 @@ func New(opt config.Opts, notFoundHandler http.Handler, routes ...Route) (Muxer, return m, nil } -// TODO: remove error return from this method. func (m Muxer) addPattern(method, pattern string, originalHandler, wrappingHandler http.Handler) { m.router.handle(method, pattern, originalHandler, wrappingHandler) } @@ -152,7 +151,6 @@ func (m Muxer) Merge(mxs []Muxer) (Muxer, error) { return m, nil } - // TODO: detect conflicts. for _, v := range mxs { m.router.routes = append(m.router.routes, v.router.routes...) } @@ -196,8 +194,6 @@ func detectConflict(m Muxer) error { for _, rt := range m.router.routes { if pattern == rt.pattern && (slices.Equal(incoming.segments, rt.segments)) && (getfunc(incoming.originalHandler) == getfunc(rt.originalHandler)) { - // TODO: this is bad, we should not include the one for incoming. - // Is this enough?? continue } From 7e81d49b48085d11d6fff633ef79ed890fe18fea Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:35:32 +0300 Subject: [PATCH 18/21] g --- internal/mx/mx.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index ec4a26f8..43e82440 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -188,12 +188,12 @@ func Param(ctx context.Context, param string) string { // / func detectConflict(m Muxer) error { for k := range m.router.routes { - incoming := m.router.routes[k] // TODO: rename this to candidate. - pattern := incoming.pattern + candidate := m.router.routes[k] + pattern := candidate.pattern incomingSegments := pathSegments(pattern) for _, rt := range m.router.routes { - if pattern == rt.pattern && (slices.Equal(incoming.segments, rt.segments)) && (getfunc(incoming.originalHandler) == getfunc(rt.originalHandler)) { + if pattern == rt.pattern && (slices.Equal(candidate.segments, rt.segments)) && (getfunc(candidate.originalHandler) == getfunc(rt.originalHandler)) { continue } @@ -215,8 +215,8 @@ However handler: %v already exists and would conflict`, pattern, - strings.ToUpper(incoming.method), - getfunc(incoming.originalHandler), + strings.ToUpper(candidate.method), + getfunc(candidate.originalHandler), path.Join(rt.segments...), strings.ToUpper(rt.method), getfunc(rt.originalHandler), From 1713e641ea2aa078e4f2df631477a1555e1d8d1a Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:47:33 +0300 Subject: [PATCH 19/21] g --- internal/mx/mx.go | 2 +- internal/mx/mx_test.go | 4 ++-- mux/mux.go | 20 ++++++++++++---- mux/mux_test.go | 52 ++++++++++++++++++++++++++++++++---------- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/internal/mx/mx.go b/internal/mx/mx.go index 43e82440..8222c012 100644 --- a/internal/mx/mx.go +++ b/internal/mx/mx.go @@ -146,7 +146,7 @@ func (m Muxer) AddRoute(rt Route) error { } // Merge combines mxs into m. The resulting muxer uses the opts & notFoundHandler of m. -func (m Muxer) Merge(mxs []Muxer) (Muxer, error) { +func (m Muxer) Merge(mxs ...Muxer) (Muxer, error) { if len(mxs) < 1 { return m, nil } diff --git a/internal/mx/mx_test.go b/internal/mx/mx_test.go index 47bf4772..cd648e1b 100644 --- a/internal/mx/mx_test.go +++ b/internal/mx/mx_test.go @@ -429,7 +429,7 @@ func TestMux(t *testing.T) { mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2, rt3) attest.Ok(t, err) - m, err := mux1.Merge([]Muxer{mux2}) + m, err := mux1.Merge(mux2) attest.Ok(t, err) attest.Equal(t, m.opt, mux1.opt) @@ -452,7 +452,7 @@ func TestMux(t *testing.T) { mux2, err := New(config.WithOpts(domain, httpsPort, tst.SecretKey(), config.DirectIpStrategy, l), nil, rt2, rt3) attest.Ok(t, err) - _, errM := mux1.Merge([]Muxer{mux2}) + _, errM := mux1.Merge(mux2) attest.Error(t, errM) rStr := errM.Error() attest.Subsequence(t, rStr, "would conflict") diff --git a/mux/mux.go b/mux/mux.go index 9ec1094c..99b986bc 100644 --- a/mux/mux.go +++ b/mux/mux.go @@ -24,11 +24,6 @@ const ( MethodTrace = mx.MethodTrace ) -// func Merge(m []Muxer) Muxer { - -// nw:=New(m[0].internalMux) -// } - // Muxer is a HTTP request multiplexer. // // It matches the URL of each incoming request against a list of registered @@ -95,6 +90,21 @@ func (m Muxer) Unwrap() mx.Muxer { return m.internalMux } +// Merge combines mxs into m. The resulting muxer uses the opts & notFoundHandler of m. +func (m Muxer) Merge(mxs ...Muxer) (Muxer, error) { + mi := []mx.Muxer{} + for _, v := range mxs { + mi = append(mi, v.internalMux) + } + + mm, err := m.internalMux.Merge(mi...) + if err != nil { + return Muxer{}, err + } + + return Muxer{internalMux: mm}, nil +} + // Param gets the path/url parameter from the specified Context. // It returns an empty string if the parameter was not found. func Param(ctx context.Context, param string) string { diff --git a/mux/mux_test.go b/mux/mux_test.go index 3ce99b2d..a238db30 100644 --- a/mux/mux_test.go +++ b/mux/mux_test.go @@ -52,19 +52,47 @@ func TestNew(t *testing.T) { }) } -// func TestMergeMux(t *testing.T) { -// l := log.New(context.Background(), &bytes.Buffer{}, 500) +func TestMerge(t *testing.T) { + t.Parallel() -// rt1 := []Route{ -// NewRoute("/home", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), -// NewRoute("/health/", MethodAll, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), -// } + l := log.New(context.Background(), &bytes.Buffer{}, 500) + + t.Run("okay", func(t *testing.T) { + t.Parallel() + + rt1 := []Route{ + NewRoute("/home", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), + NewRoute("/health/", MethodAll, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), + } + + rt2 := []Route{ + NewRoute("/uri2", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), + } + + mx1 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt1...) + mx2 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt2...) -// rt2 := []Route{ -// NewRoute("/uri2", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), -// } + _, err := mx1.Merge(mx2) + attest.Ok(t, err) + }) + + t.Run("conflict", func(t *testing.T) { + t.Parallel() + + rt1 := []Route{ + NewRoute("/home", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), + NewRoute("/health/", MethodAll, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), + } + + rt2 := []Route{ + NewRoute("/uri2", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), + NewRoute("health", MethodPost, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})), + } -// mx1 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt1...) -// mx2 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt2...) + mx1 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt1...) + mx2 := New(config.DevOpts(l, "secretKey12@34String"), nil, rt2...) -// } + _, err := mx1.Merge(mx2) + attest.Error(t, err) + }) +} From 03c98bf693d1b06cdf9a14f271102c5208256e27 Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:48:08 +0300 Subject: [PATCH 20/21] g --- mux/mux_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/mux/mux_test.go b/mux/mux_test.go index a238db30..99171653 100644 --- a/mux/mux_test.go +++ b/mux/mux_test.go @@ -94,5 +94,6 @@ func TestMerge(t *testing.T) { _, err := mx1.Merge(mx2) attest.Error(t, err) + attest.Subsequence(t, err.Error(), "would conflict") }) } From 3fc3213124623afefeb070d355dcf08c7b20067a Mon Sep 17 00:00:00 2001 From: komuw Date: Tue, 22 Oct 2024 23:51:12 +0300 Subject: [PATCH 21/21] g --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 140d4bfe..2ae16e21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,12 @@ Most recent version is listed first. +# v0.1.13 +- ong/mux: add ability to merge Muxers: https://github.com/komuw/ong/pull/482 + # v0.1.12 - ong/mux: add flexible pattern that allows a handler to serve almost all request URIs: https://github.com/komuw/ong/pull/481 -- + # v0.1.11 - ong/cry: use constant parameters for argon key generation: https://github.com/komuw/ong/pull/477