From 2fbe22903bd60c6fa7242e1dba1dc9cd33fd51ef Mon Sep 17 00:00:00 2001 From: bubbajoe Date: Sun, 12 May 2024 14:38:41 +0900 Subject: [PATCH] simplify code and add more test for better coverage --- cmd/dgate-cli/commands/run_cmd_test.go | 13 +- pkg/dgclient/dgclient.go | 48 ++---- pkg/dgclient/dgclient_test.go | 204 +++++++++++++++++++++++++ pkg/modules/extractors/extractors.go | 9 +- pkg/modules/types/goja_value.go | 19 --- pkg/storage/debug_storage.go | 3 - 6 files changed, 234 insertions(+), 62 deletions(-) create mode 100644 pkg/dgclient/dgclient_test.go delete mode 100644 pkg/modules/types/goja_value.go diff --git a/cmd/dgate-cli/commands/run_cmd_test.go b/cmd/dgate-cli/commands/run_cmd_test.go index 56284bc..24f810f 100644 --- a/cmd/dgate-cli/commands/run_cmd_test.go +++ b/cmd/dgate-cli/commands/run_cmd_test.go @@ -162,6 +162,14 @@ func TestCommands_ClientError(t *testing.T) { if assert.NotNil(t, err, "no error on %s:%s", action, resource) { assert.Equal(t, "error", err.Error()) } + if action == "delete" && resource == "document" { + mockClient.On("DeleteAllDocument", "test", "test"). + Return(errors.New("error")) + err = Run(mockClient, version) + if assert.NotNil(t, err, "no error on %s:%s", action, resource) { + assert.Equal(t, "error", err.Error()) + } + } } } } @@ -186,11 +194,6 @@ func (m *mockDGClient) Init(baseUrl string, opts ...dgclient.Options) error { return args.Error(0) } -func (m *mockDGClient) BaseUrl() string { - args := m.Called() - return args.String(0) -} - func (m *mockDGClient) GetRoute(name, namespace string) (*spec.Route, error) { args := m.Called(name, namespace) if args.Get(0) == nil { diff --git a/pkg/dgclient/dgclient.go b/pkg/dgclient/dgclient.go index f73af5b..20a5a62 100644 --- a/pkg/dgclient/dgclient.go +++ b/pkg/dgclient/dgclient.go @@ -1,6 +1,7 @@ package dgclient import ( + "errors" "fmt" "net/http" "net/url" @@ -10,8 +11,6 @@ import ( type DGateClient interface { Init(baseUrl string, opts ...Options) error - BaseUrl() string - DGateNamespaceClient DGateModuleClient DGateRouteClient @@ -42,25 +41,25 @@ func (d *dgateClient) Init(baseUrl string, opts ...Options) error { return err } - if bUrl.Scheme == "" { - bUrl.Scheme = "http" - } else if bUrl.Host == "" { - return url.InvalidHostError("host is empty") + if bUrl.Host == "" { + return errors.New("host is empty") + } else { + d.baseUrl = bUrl } - d.client = http.DefaultClient - d.baseUrl = bUrl - for _, opt := range opts { - opt(d) + if opt != nil { + opt(d) + } + } + if d.client == nil { + d.client = http.DefaultClient + } else if d.client.Transport == nil { + d.client.Transport = http.DefaultTransport } return nil } -func (d *dgateClient) BaseUrl() string { - return d.baseUrl.String() -} - func WithHttpClient(client *http.Client) Options { return func(dc DGateClient) { if d, ok := dc.(*dgateClient); ok { @@ -99,14 +98,8 @@ func (ct *customTransport) RoundTrip(req *http.Request) (*http.Response, error) } func WithBasicAuth(username, password string) Options { - if username == "" || password == "" { - return nil - } return func(dc DGateClient) { if d, ok := dc.(*dgateClient); ok { - if d.client.Transport == nil { - d.client.Transport = http.DefaultTransport - } if ct, ok := d.client.Transport.(*customTransport); ok { ct.Username = username ct.Password = password @@ -124,10 +117,10 @@ func WithBasicAuth(username, password string) Options { func WithFollowRedirect(follow bool) Options { return func(dc DGateClient) { if d, ok := dc.(*dgateClient); ok { - if follow { - return - } d.client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if follow { + return nil + } return http.ErrUseLastResponse } } @@ -135,14 +128,8 @@ func WithFollowRedirect(follow bool) Options { } func WithUserAgent(ua string) Options { - if ua == "" { - return nil - } return func(dc DGateClient) { if d, ok := dc.(*dgateClient); ok { - if d.client.Transport == nil { - d.client.Transport = http.DefaultTransport - } if ct, ok := d.client.Transport.(*customTransport); ok { ct.UserAgent = ua ct.Transport = http.DefaultTransport @@ -159,9 +146,6 @@ func WithUserAgent(ua string) Options { func WithVerboseLogging(on bool) Options { return func(dc DGateClient) { if d, ok := dc.(*dgateClient); ok { - if d.client.Transport == nil { - d.client.Transport = http.DefaultTransport - } if ct, ok := d.client.Transport.(*customTransport); ok { ct.VerboseLog = on } else { diff --git a/pkg/dgclient/dgclient_test.go b/pkg/dgclient/dgclient_test.go new file mode 100644 index 0000000..bb31a81 --- /dev/null +++ b/pkg/dgclient/dgclient_test.go @@ -0,0 +1,204 @@ +package dgclient_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/dgate-io/dgate/pkg/dgclient" + "github.com/stretchr/testify/assert" +) + +func TestDGClient_OptionsWithRedirect(t *testing.T) { + var client = dgclient.NewDGateClient() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/service/test" { + http.Redirect(w, r, "/", http.StatusTemporaryRedirect) + return + } + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"name":"test"}`)) + } + })) + defer server.Close() + + err := client.Init(server.URL, + dgclient.WithHttpClient(server.Client()), + dgclient.WithFollowRedirect(true), + ) + if err != nil { + t.Fatal(err) + } + + _, err = client.GetService("test", "default") + if err != nil { + t.Fatal(err) + } +} + +func TestDGClient_OptionsRedirectError(t *testing.T) { + var client = dgclient.NewDGateClient() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if user, pass, _ := r.BasicAuth(); user != "user" || pass != "password" { + w.WriteHeader(http.StatusUnauthorized) + return + } + if r.URL.Path == "/api/v1/service/test" { + http.Redirect(w, r, "/", http.StatusTemporaryRedirect) + return + } + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"name":"test"}`)) + } + })) + defer server.Close() + + err := client.Init(server.URL, + dgclient.WithHttpClient(server.Client()), + dgclient.WithBasicAuth("user", "password"), + dgclient.WithFollowRedirect(false), + ) + if err != nil { + t.Fatal(err) + } + + _, err = client.GetService("test", "default") + if err == nil { + t.Fatal("expected error") + } +} + +func TestDGClient_OptionsWithBasicAuth(t *testing.T) { + var client = dgclient.NewDGateClient() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if user, pass, _ := r.BasicAuth(); user != "user" || pass != "password" { + w.WriteHeader(http.StatusUnauthorized) + return + } + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"name":"test"}`)) + } + })) + defer server.Close() + + err := client.Init(server.URL, + dgclient.WithHttpClient(server.Client()), + dgclient.WithVerboseLogging(true), + dgclient.WithBasicAuth("user", "password"), + ) + if err != nil { + t.Fatal(err) + } + + _, err = client.GetService("test", "default") + if err != nil { + t.Fatal(err) + } +} + +func TestDGClient_OptionsBasicAuthError(t *testing.T) { + var client = dgclient.NewDGateClient() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if user, pass, _ := r.BasicAuth(); user != "user" || pass != "password" { + w.WriteHeader(http.StatusUnauthorized) + return + } + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"name":"test"}`)) + } + })) + defer server.Close() + + err := client.Init(server.URL, + dgclient.WithHttpClient(server.Client()), + dgclient.WithBasicAuth("user", "wrongpassword"), + dgclient.WithVerboseLogging(true), + ) + if err != nil { + t.Fatal(err) + } + + _, err = client.GetService("test", "default") + if err == nil { + t.Fatal("expected error") + } +} + +func TestDGClient_OptionsWithUserAgent(t *testing.T) { + var client = dgclient.NewDGateClient() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.UserAgent() != "test" { + w.WriteHeader(http.StatusBadRequest) + return + } + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"name":"test"}`)) + } + })) + defer server.Close() + + err := client.Init(server.URL, + dgclient.WithHttpClient(server.Client()), + dgclient.WithVerboseLogging(true), + dgclient.WithUserAgent("test"), + ) + if err != nil { + t.Fatal(err) + } + + _, err = client.GetService("test", "default") + if err != nil { + t.Fatal(err) + } +} + +func TestDGClient_OptionsWithUserAgent2(t *testing.T) { + var client = dgclient.NewDGateClient() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.UserAgent() != "test" { + w.WriteHeader(http.StatusBadRequest) + return + } + if r.Method == http.MethodGet { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"name":"test"}`)) + } + })) + defer server.Close() + + err := client.Init(server.URL, + dgclient.WithHttpClient(server.Client()), + dgclient.WithUserAgent("test"), + dgclient.WithVerboseLogging(true), + ) + if err != nil { + t.Fatal(err) + } + + _, err = client.GetService("test", "default") + if err != nil { + t.Fatal(err) + } +} + +func TestDGClient_Init_ParseURLError(t *testing.T) { + var client = dgclient.NewDGateClient() + err := client.Init("://#/:asdm") + if err == nil { + t.Fatal("expected error") + } +} + +func TestDGClient_Init_EmptyHostError(t *testing.T) { + var client = dgclient.NewDGateClient() + err := client.Init("") + if err == nil { + t.Fatal("expected error") + } + assert.Equal(t, "host is empty", err.Error()) +} diff --git a/pkg/modules/extractors/extractors.go b/pkg/modules/extractors/extractors.go index 4086112..1120400 100644 --- a/pkg/modules/extractors/extractors.go +++ b/pkg/modules/extractors/extractors.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "strings" "time" "github.com/dgate-io/dgate/pkg/eventloop" @@ -83,7 +84,6 @@ func DefaultFetchUpstreamFunction() FetchUpstreamUrlFunc { } } -// const _ goja.AsyncContextTracker = ExtractorContextTracker{""} func ExtractFetchUpstreamFunction( loop *eventloop.EventLoop, ) (fetchUpstream FetchUpstreamUrlFunc, err error) { @@ -101,6 +101,9 @@ func ExtractFetchUpstreamFunction( if goja.IsUndefined(res) || goja.IsNull(res) || upstreamUrlString == "" { return nil, errors.New("fetchUpstream returned an invalid URL") } + if !strings.Contains(upstreamUrlString, "://") { + upstreamUrlString += "http://" + } upstreamUrl, err := url.Parse(upstreamUrlString) if err != nil { return nil, err @@ -121,7 +124,7 @@ func ExtractRequestModifierFunction( if call, ok := goja.AssertFunction(rt.Get("requestModifier")); ok { requestModifier = func(modCtx *types.ModuleContext) error { _, err := RunAndWaitForResult( - rt, call, types.ToValue(rt, modCtx), + rt, call, rt.ToValue(modCtx), ) return err } @@ -137,7 +140,7 @@ func ExtractResponseModifierFunction( responseModifier = func(modCtx *types.ModuleContext, res *http.Response) error { modCtx = types.ModuleContextWithResponse(modCtx, res) _, err := RunAndWaitForResult( - rt, call, types.ToValue(rt, modCtx), + rt, call, rt.ToValue(modCtx), ) return err } diff --git a/pkg/modules/types/goja_value.go b/pkg/modules/types/goja_value.go deleted file mode 100644 index 13e448f..0000000 --- a/pkg/modules/types/goja_value.go +++ /dev/null @@ -1,19 +0,0 @@ -package types - -import "github.com/dop251/goja" - -type Symbolic interface { - symbols(rt *goja.Runtime) map[string]goja.Value -} - -func ToValue(rt *goja.Runtime, v any) goja.Value { - val := rt.ToValue(v) - if obj, ok := val.(*goja.Object); ok { - if sym, ok := v.(Symbolic); ok { - for name, value := range sym.symbols(rt) { - obj.SetSymbol(goja.NewSymbol(name), value) - } - } - } - return val -} diff --git a/pkg/storage/debug_storage.go b/pkg/storage/debug_storage.go index ce45ddb..e7ac502 100644 --- a/pkg/storage/debug_storage.go +++ b/pkg/storage/debug_storage.go @@ -9,9 +9,6 @@ import ( ) type DebugStoreConfig struct { - // Path to the directory where the files will be stored. - // If the directory does not exist, it will be created. - // If the directory exists, it will be used. Logger zerolog.Logger }