Skip to content

Commit d03944b

Browse files
committed
fix: resolve path parameter when route has trailing slash
fix: trigger listeners when reading file from reference before main file fix: deadlock race condition in ConfigWatcher improve: log messages for HTTP/HTTPS server fix: logs/dashboard when HTTP request does not match any API
1 parent 704e5da commit d03944b

File tree

18 files changed

+383
-109
lines changed

18 files changed

+383
-109
lines changed

config/dynamic/config.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ package dynamic
22

33
import (
44
"bytes"
5-
"github.com/Masterminds/sprig"
65
"mokapi/sortedmap"
76
"strings"
87
"sync"
98
"text/template"
9+
10+
"github.com/Masterminds/sprig"
1011
)
1112

1213
type Event int
@@ -31,13 +32,21 @@ type Validator interface {
3132
Validate() error
3233
}
3334

35+
type SourceType int
36+
37+
const (
38+
SourceMain SourceType = iota
39+
SourceReference
40+
)
41+
3442
type Config struct {
35-
Info ConfigInfo
36-
Raw []byte
37-
Data interface{}
38-
Refs Refs
39-
Listeners Listeners
40-
Scope Scope
43+
Info ConfigInfo
44+
Raw []byte
45+
Data interface{}
46+
Refs Refs
47+
Listeners Listeners
48+
Scope Scope
49+
SourceType SourceType
4150
}
4251

4352
type Refs struct {

config/dynamic/provider/file/file.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"context"
66
"crypto/sha256"
77
"fmt"
8-
"github.com/fsnotify/fsnotify"
9-
log "github.com/sirupsen/logrus"
108
"io"
119
"io/fs"
1210
"math"
@@ -20,6 +18,9 @@ import (
2018
"strings"
2119
"sync"
2220
"time"
21+
22+
"github.com/fsnotify/fsnotify"
23+
log "github.com/sirupsen/logrus"
2324
)
2425

2526
const mokapiIgnoreFile = ".mokapiignore"
@@ -82,6 +83,7 @@ func (p *Provider) Read(u *url.URL) (*dynamic.Config, error) {
8283
if err != nil {
8384
return nil, err
8485
}
86+
config.SourceType = dynamic.SourceReference
8587

8688
p.watchPath(file)
8789
return config, nil

config/dynamic/provider/http/http.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"crypto/tls"
66
"crypto/x509"
77
"fmt"
8-
log "github.com/sirupsen/logrus"
98
"hash/fnv"
109
"io"
1110
"mokapi/config/dynamic"
@@ -16,6 +15,8 @@ import (
1615
"os"
1716
"sync"
1817
"time"
18+
19+
log "github.com/sirupsen/logrus"
1920
)
2021

2122
type Provider struct {
@@ -81,6 +82,9 @@ func New(config static.HttpProvider) *Provider {
8182

8283
func (p *Provider) Read(u *url.URL) (*dynamic.Config, error) {
8384
c, _, err := p.readUrl(u)
85+
if c != nil {
86+
c.SourceType = dynamic.SourceReference
87+
}
8488
return c, err
8589
}
8690

config/dynamic/provider/npm/npm.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@ package npm
33
import (
44
"context"
55
"fmt"
6-
log "github.com/sirupsen/logrus"
76
"mokapi/config/dynamic"
87
"mokapi/config/dynamic/provider/file"
98
"mokapi/config/static"
109
"mokapi/safe"
1110
"net/url"
1211
"path/filepath"
1312
"strings"
13+
14+
log "github.com/sirupsen/logrus"
1415
)
1516

1617
type Provider struct {

lib/http.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ func GetUrl(r *http.Request) string {
1010
return r.URL.String()
1111
}
1212
var sb strings.Builder
13-
if strings.HasPrefix(r.Proto, "HTTPS") {
13+
if r.TLS != nil {
1414
sb.WriteString("https://")
1515
} else {
1616
sb.WriteString("http://")

providers/openapi/handler.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,14 @@ func (h *operationHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) *H
249249
op, errOpResolve := findOperation(r.Method, requestPath, h.config.Paths)
250250
if op != nil {
251251
params := append(op.Path.Parameters, op.Parameters...)
252+
route := op.Path.Path
253+
if len(route) > 1 {
254+
// there is no official specification for trailing slash. For ease of use, mokapi considers it equivalent
255+
route = strings.TrimRight(route, "/")
256+
}
257+
252258
var rp parameter.RequestParameters
253-
rp, errOpResolve = parameter.FromRequest(params, op.Path.Path, r)
259+
rp, errOpResolve = parameter.FromRequest(params, route, r)
254260

255261
if errOpResolve == nil {
256262
r = r.WithContext(parameter.NewContext(r.Context(), rp))
@@ -280,7 +286,7 @@ func (h *operationHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) *H
280286
}
281287
}
282288

283-
if ctx, err := NewLogEventContext(
289+
/*if ctx, err := NewLogEventContext(
284290
r,
285291
false,
286292
h.eh,
@@ -289,12 +295,12 @@ func (h *operationHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) *H
289295
log.Errorf("unable to log http event: %v", err)
290296
} else {
291297
r = r.WithContext(ctx)
292-
}
298+
}*/
293299

294300
// Not reading the request body can cause a couple of problems.
295301
// Go’s HTTP server uses connection pooling by default.
296302
// If you don’t read and fully consume (or close) the request body, the remaining unread bytes will stay in the TCP buffer.
297-
_, _ = io.Copy(io.Discard, r.Body)
303+
//_, _ = io.Copy(io.Discard, r.Body)
298304

299305
if errOpResolve == nil {
300306
return newHttpErrorf(http.StatusNotFound, "no matching endpoint found: %v %v", strings.ToUpper(r.Method), lib.GetUrl(r))

providers/openapi/handler_requestbody_test.go

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -198,29 +198,6 @@ func TestResponseHandler_ServeHTTP_ResponseBody(t *testing.T) {
198198
check: func(t *testing.T, r *engine2.EventRequest) {
199199
},
200200
},
201-
{
202-
name: "send request body to undefined endpoint",
203-
config: openapitest.NewConfig("3.0.0",
204-
openapitest.WithServer("http://localhost", ""),
205-
),
206-
fn: func(t *testing.T, handler openapi.Handler) {
207-
spy := &spyBody{Reader: bytes.NewBufferString(`{"foo": "abc","bar": 12}`)}
208-
209-
r := httptest.NewRequest("post", "http://localhost/foo", spy)
210-
r.Header.Set("Content-Type", "application/json")
211-
rr := httptest.NewRecorder()
212-
213-
err := handler.ServeHTTP(rr, r)
214-
require.Equal(t, &openapi.HttpError{
215-
StatusCode: http.StatusNotFound,
216-
Message: "no matching endpoint found: POST http://localhost/foo",
217-
}, err)
218-
219-
require.True(t, spy.readCalled, "server needs to read body")
220-
},
221-
check: func(t *testing.T, r *engine2.EventRequest) {
222-
},
223-
},
224201
}
225202

226203
for _, tc := range testcases {

providers/openapi/handler_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,31 @@ func TestHandler_Event(t *testing.T) {
860860
return nil
861861
},
862862
},
863+
{
864+
name: "path parameter with trailing slash in route",
865+
test: func(t *testing.T, h http.HandlerFunc, c *openapi.Config) {
866+
op := openapitest.NewOperation(
867+
openapitest.WithResponse(http.StatusOK,
868+
openapitest.WithContent("application/json", openapitest.NewContent()),
869+
))
870+
openapitest.AppendPath("/foo/{id}/", c,
871+
openapitest.WithOperation(http.MethodPost, op),
872+
openapitest.WithPathParam("id", openapitest.WithParamSchema(schematest.New("string"))),
873+
)
874+
r := httptest.NewRequest("post", "http://localhost/foo/123", strings.NewReader(`{ "foo": "bar" }`))
875+
r.Header.Set("Content-Type", "application/json")
876+
rr := httptest.NewRecorder()
877+
h(rr, r)
878+
require.Equal(t, http.StatusOK, rr.Code)
879+
require.Equal(t, `"123"`, rr.Body.String())
880+
},
881+
event: func(event string, args ...interface{}) []*common.Action {
882+
req := args[0].(*common.EventRequest)
883+
res := args[1].(*common.EventResponse)
884+
res.Data = req.Path["id"]
885+
return nil
886+
},
887+
},
863888
}
864889

865890
t.Parallel()

providers/openapi/log.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func NewLogEventContext(r *http.Request, deprecated bool, eh events.Handler, tra
5151
Request: &HttpRequestLog{
5252
Method: r.Method,
5353
Url: lib.GetUrl(r),
54-
ContentType: r.Header.Get("content-type"),
54+
ContentType: r.Header.Get("Content-Type"),
5555
},
5656
Response: &HttpResponseLog{Headers: make(map[string]string)},
5757
Deprecated: deprecated,

providers/openapi/parameter/path_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package parameter_test
22

33
import (
4+
"context"
45
"mokapi/providers/openapi/parameter"
56
"mokapi/providers/openapi/schema/schematest"
67
"net/http"
@@ -268,6 +269,43 @@ func TestParsePath(t *testing.T) {
268269
require.Equal(t, map[string]interface{}{"role": "admin", "firstName": "Alex"}, result[parameter.Path]["foo"].Value)
269270
},
270271
},
272+
{
273+
name: "path parameter and base path",
274+
param: &parameter.Parameter{
275+
Name: "foo",
276+
Type: parameter.Path,
277+
Schema: schematest.New("string"),
278+
Style: "",
279+
Explode: explode(false),
280+
},
281+
route: "/{foo}",
282+
request: func() *http.Request {
283+
r := httptest.NewRequest(http.MethodGet, "https://foo.bar/mokapi/foo/bar", nil)
284+
return r.WithContext(context.WithValue(r.Context(), "servicePath", "/mokapi/foo"))
285+
},
286+
test: func(t *testing.T, result parameter.RequestParameters, err error) {
287+
require.NoError(t, err)
288+
require.Equal(t, "bar", result[parameter.Path]["foo"].Value)
289+
},
290+
},
291+
{
292+
name: "path parameter and trailing slash in request",
293+
param: &parameter.Parameter{
294+
Name: "foo",
295+
Type: parameter.Path,
296+
Schema: schematest.New("string"),
297+
Style: "",
298+
Explode: explode(false),
299+
},
300+
route: "/{foo}",
301+
request: func() *http.Request {
302+
return httptest.NewRequest(http.MethodGet, "https://foo.bar/bar/", nil)
303+
},
304+
test: func(t *testing.T, result parameter.RequestParameters, err error) {
305+
require.NoError(t, err)
306+
require.Equal(t, "bar", result[parameter.Path]["foo"].Value)
307+
},
308+
},
271309
}
272310

273311
for _, tc := range testcases {

0 commit comments

Comments
 (0)