From cd90c20239ba8660ce41dbe221fe6ecc0b9216d8 Mon Sep 17 00:00:00 2001 From: bubbajoe Date: Sun, 2 Jun 2024 09:50:49 +0900 Subject: [PATCH] fix collection/namespace switching, and fix tests --- internal/admin/routes/collection_routes.go | 16 +++++--- internal/admin/routes/module_routes_test.go | 2 +- internal/admin/routes/service_routes_test.go | 2 +- internal/config/configtest/dgate_configs.go | 17 ++++++++ internal/proxy/proxy_documents.go | 4 +- internal/proxy/proxy_handler_test.go | 2 +- internal/proxy/proxy_state_test.go | 42 ++++++++++---------- pkg/modules/dgate/state/state_mod.go | 11 +++-- pkg/resources/document_manager.go | 2 +- 9 files changed, 63 insertions(+), 35 deletions(-) diff --git a/internal/admin/routes/collection_routes.go b/internal/admin/routes/collection_routes.go index 3418cb7..4852807 100644 --- a/internal/admin/routes/collection_routes.go +++ b/internal/admin/routes/collection_routes.go @@ -48,7 +48,10 @@ func ConfigureCollectionAPI(server chi.Router, logger *zap.Logger, cs changestat if oldCollection, ok := rm.GetCollection(collection.Name, collection.NamespaceName); ok { if oldCollection.Type == spec.CollectionTypeDocument { docs, err := dm.GetDocuments( - collection.NamespaceName, collection.Name, 0, 0) + collection.Name, + collection.NamespaceName, + 0, 0, + ) if err != nil { util.JsonError(w, http.StatusInternalServerError, err.Error()) return @@ -142,7 +145,7 @@ func ConfigureCollectionAPI(server chi.Router, logger *zap.Logger, cs changestat util.JsonError(w, http.StatusBadRequest, "offset must be an integer") return } - docs, err := dm.GetDocuments(namespaceName, collectionName, offset, limit) + docs, err := dm.GetDocuments(collectionName, namespaceName, offset, limit) if err != nil { util.JsonError(w, http.StatusInternalServerError, err.Error()) return @@ -194,7 +197,7 @@ func ConfigureCollectionAPI(server chi.Router, logger *zap.Logger, cs changestat return } - document, err := dm.GetDocumentByID(namespaceName, collectionName, documentId) + document, err := dm.GetDocumentByID(collectionName, namespaceName, documentId) if err != nil { util.JsonError(w, http.StatusNotFound, err.Error()) return @@ -348,7 +351,7 @@ func ConfigureCollectionAPI(server chi.Router, logger *zap.Logger, cs changestat util.JsonError(w, http.StatusBadRequest, "document_id is required") return } - document, err := dm.GetDocumentByID(namespaceName, collectionName, documentId) + document, err := dm.GetDocumentByID(collectionName, namespaceName, documentId) if err != nil { util.JsonError(w, http.StatusNotFound, err.Error()) return @@ -386,7 +389,10 @@ func ConfigureCollectionAPI(server chi.Router, logger *zap.Logger, cs changestat } if collection.Type == spec.CollectionTypeDocument { docs, err := dm.GetDocuments( - namespaceName, collectionName, 0, 1) + collectionName, + namespaceName, + 1, 1, + ) if err != nil { util.JsonError(w, http.StatusInternalServerError, err.Error()) return diff --git a/internal/admin/routes/module_routes_test.go b/internal/admin/routes/module_routes_test.go index ed1383d..74db074 100644 --- a/internal/admin/routes/module_routes_test.go +++ b/internal/admin/routes/module_routes_test.go @@ -22,7 +22,7 @@ import ( func TestAdminRoutes_Module(t *testing.T) { namespaces := []string{"default", "test"} for _, ns := range namespaces { - config := configtest.NewTest3DGateConfig() + config := configtest.NewTest4DGateConfig() ps := proxy.NewProxyState(zap.NewNop(), config) mux := chi.NewMux() mux.Route("/api/v1", func(r chi.Router) { diff --git a/internal/admin/routes/service_routes_test.go b/internal/admin/routes/service_routes_test.go index 2e808a1..555959f 100644 --- a/internal/admin/routes/service_routes_test.go +++ b/internal/admin/routes/service_routes_test.go @@ -21,7 +21,7 @@ import ( func TestAdminRoutes_Service(t *testing.T) { namespaces := []string{"default", "test"} for _, ns := range namespaces { - config := configtest.NewTest3DGateConfig() + config := configtest.NewTest4DGateConfig() ps := proxy.NewProxyState(zap.NewNop(), config) mux := chi.NewMux() mux.Route("/api/v1", func(r chi.Router) { diff --git a/internal/config/configtest/dgate_configs.go b/internal/config/configtest/dgate_configs.go index ae5a673..36956f3 100644 --- a/internal/config/configtest/dgate_configs.go +++ b/internal/config/configtest/dgate_configs.go @@ -107,6 +107,23 @@ func NewTest3DGateConfig() *config.DGateConfig { return conf } +func NewTest4DGateConfig() *config.DGateConfig { + conf := NewTestDGateConfig() + conf.DisableDefaultNamespace = false + conf.ProxyConfig = config.DGateProxyConfig{ + Host: "localhost", + Port: 16436, + InitResources: &config.DGateResources{ + Namespaces: []spec.Namespace{ + { + Name: "test", + }, + }, + }, + } + return conf +} + func NewTestDGateConfig_DomainAndNamespaces() *config.DGateConfig { conf := NewTestDGateConfig() conf.ProxyConfig.InitResources.Namespaces = []spec.Namespace{ diff --git a/internal/proxy/proxy_documents.go b/internal/proxy/proxy_documents.go index b67c641..3ccf83a 100644 --- a/internal/proxy/proxy_documents.go +++ b/internal/proxy/proxy_documents.go @@ -15,7 +15,7 @@ func (ps *ProxyState) GetDocuments(collection, namespace string, limit, offset i if _, ok := ps.rm.GetNamespace(namespace); !ok { return nil, spec.ErrNamespaceNotFound(namespace) } - if _, ok := ps.rm.GetCollection(namespace, collection); !ok { + if _, ok := ps.rm.GetCollection(collection, namespace); !ok { return nil, spec.ErrCollectionNotFound(collection) } return ps.store.FetchDocuments(namespace, collection, limit, offset) @@ -26,7 +26,7 @@ func (ps *ProxyState) GetDocumentByID(namespace, collection, docId string) (*spe if _, ok := ps.rm.GetNamespace(namespace); !ok { return nil, spec.ErrNamespaceNotFound(namespace) } - if _, ok := ps.rm.GetCollection(collection, namespace); !ok { + if _, ok := ps.rm.GetCollection(namespace, collection); !ok { return nil, spec.ErrCollectionNotFound(collection) } return ps.store.FetchDocument(namespace, collection, docId) diff --git a/internal/proxy/proxy_handler_test.go b/internal/proxy/proxy_handler_test.go index cacd8c7..de38c67 100644 --- a/internal/proxy/proxy_handler_test.go +++ b/internal/proxy/proxy_handler_test.go @@ -65,6 +65,7 @@ func TestProxyHandler_ReverseProxy(t *testing.T) { modBuf := NewMockModulePool() modBuf.On("Borrow").Return(modExt).Once() modBuf.On("Return", modExt).Return().Once() + modBuf.On("Close").Return().Once() reqCtxProvider.SetModulePool(modBuf) modPool := NewMockModulePool() @@ -178,7 +179,6 @@ func TestProxyHandler_ProxyHandlerError(t *testing.T) { modPool := NewMockModulePool() modPool.On("Borrow").Return(modExt).Once() modPool.On("Return", modExt).Return().Once() - reqCtxProvider := proxy.NewRequestContextProvider(rt, ps) reqCtxProvider.SetModulePool(modPool) reqCtx := reqCtxProvider.CreateRequestContext( diff --git a/internal/proxy/proxy_state_test.go b/internal/proxy/proxy_state_test.go index 0a3d27d..8b26709 100644 --- a/internal/proxy/proxy_state_test.go +++ b/internal/proxy/proxy_state_test.go @@ -267,14 +267,14 @@ func TestProcessChangeLog_Route(t *testing.T) { } func TestProcessChangeLog_Service(t *testing.T) { - conf := configtest.NewTestDGateConfig() + conf := configtest.NewTest4DGateConfig() ps := proxy.NewProxyState(zap.NewNop(), conf) if err := ps.Store().InitStore(); err != nil { t.Fatal(err) } s := &spec.Service{ - Name: "test", + Name: "test123", NamespaceName: "test", URLs: []string{"http://localhost:8080"}, Tags: []string{"test"}, @@ -302,14 +302,14 @@ func TestProcessChangeLog_Service(t *testing.T) { } func TestProcessChangeLog_Module(t *testing.T) { - conf := configtest.NewTestDGateConfig() + conf := configtest.NewTest4DGateConfig() ps := proxy.NewProxyState(zap.NewNop(), conf) if err := ps.Store().InitStore(); err != nil { t.Fatal(err) } m := &spec.Module{ - Name: "test", + Name: "test123", NamespaceName: "test", Payload: "", Tags: []string{"test"}, @@ -337,7 +337,7 @@ func TestProcessChangeLog_Module(t *testing.T) { } func TestProcessChangeLog_Namespace(t *testing.T) { - ps := proxy.NewProxyState(zap.NewNop(), configtest.NewTestDGateConfig()) + ps := proxy.NewProxyState(zap.NewNop(), configtest.NewTest4DGateConfig()) if err := ps.Store().InitStore(); err != nil { t.Fatal(err) } @@ -351,28 +351,30 @@ func TestProcessChangeLog_Namespace(t *testing.T) { if !assert.Nil(t, err, "error should be nil") { return } - namespaces := ps.ResourceManager().GetNamespaces() - assert.Equal(t, 2, len(namespaces), "should have 2 items") - assert.Equal(t, n.Name, namespaces[1].Name, "should have the same name") + ns, ok := ps.ResourceManager().GetNamespace(n.Name) + if !assert.True(t, ok, "should be true") { + return + } + assert.Equal(t, n.Name, ns.Name, "should have the same name") cl = spec.NewChangeLog(n, n.Name, spec.DeleteNamespaceCommand) err = ps.ProcessChangeLog(cl, false) if !assert.Nil(t, err, "error should be nil") { return } - namespaces = ps.ResourceManager().GetNamespaces() - assert.Equal(t, 1, len(namespaces), "should have 0 item") + _, ok = ps.ResourceManager().GetNamespace(n.Name) + assert.False(t, ok, "should be false") } func TestProcessChangeLog_Collection(t *testing.T) { - conf := configtest.NewTestDGateConfig() + conf := configtest.NewTest4DGateConfig() ps := proxy.NewProxyState(zap.NewNop(), conf) if err := ps.Store().InitStore(); err != nil { t.Fatal(err) } c := &spec.Collection{ - Name: "test", + Name: "test123", NamespaceName: "test", // Type: spec.CollectionTypeDocument, Visibility: spec.CollectionVisibilityPrivate, @@ -409,11 +411,11 @@ func TestProcessChangeLog_Document(t *testing.T) { } c := &spec.Collection{ - Name: "test", + Name: "test123", NamespaceName: "test", - // Type: spec.CollectionTypeDocument, - Visibility: spec.CollectionVisibilityPrivate, - Tags: []string{"test"}, + Type: spec.CollectionTypeDocument, + Visibility: spec.CollectionVisibilityPrivate, + Tags: []string{"test"}, } cl := spec.NewChangeLog(c, c.NamespaceName, spec.AddCollectionCommand) @@ -423,9 +425,9 @@ func TestProcessChangeLog_Document(t *testing.T) { } d := &spec.Document{ - ID: "test", + ID: "test123", + CollectionName: "test123", NamespaceName: "test", - CollectionName: "test", Data: "", } @@ -435,7 +437,7 @@ func TestProcessChangeLog_Document(t *testing.T) { return } documents, err := ps.DocumentManager().GetDocuments( - "test", "test", 999, 0, + d.CollectionName, d.NamespaceName, 999, 0, ) if !assert.Nil(t, err, "error should be nil") { return @@ -452,7 +454,7 @@ func TestProcessChangeLog_Document(t *testing.T) { return } documents, err = ps.DocumentManager().GetDocuments( - "test", "test", 999, 0, + d.CollectionName, d.NamespaceName, 999, 0, ) if !assert.Nil(t, err, "error should be nil") { return diff --git a/pkg/modules/dgate/state/state_mod.go b/pkg/modules/dgate/state/state_mod.go index b8fa250..91458d7 100644 --- a/pkg/modules/dgate/state/state_mod.go +++ b/pkg/modules/dgate/state/state_mod.go @@ -71,7 +71,7 @@ func (hp *ResourcesModule) fetchDocument(collection, docId string) *goja.Promise return } doc, err := state.DocumentManager(). - GetDocumentByID(namespace.(string), collection, docId) + GetDocumentByID(collection, namespace.(string), docId) if err != nil { reject(rt.NewGoError(err)) return @@ -87,11 +87,11 @@ func (hp *ResourcesModule) fetchDocuments(args goja.FunctionCall) (*goja.Promise loop := hp.modCtx.EventLoop() rt := hp.modCtx.Runtime() - collection_name := "" + collection := "" if args.Argument(0) == goja.Undefined() { return nil, errors.New("collection name is required") } else { - collection_name = args.Argument(0).String() + collection = args.Argument(0).String() } limit := 0 if args.Argument(1) != goja.Undefined() { @@ -111,7 +111,10 @@ func (hp *ResourcesModule) fetchDocuments(args goja.FunctionCall) (*goja.Promise docPromise, resolve, reject := rt.NewPromise() loop.RunOnLoop(func(rt *goja.Runtime) { doc, err := state.DocumentManager(). - GetDocuments(namespace, collection_name, limit, offset) + GetDocuments( + collection, namespace, + limit, offset, + ) if err != nil { reject(rt.NewGoError(err)) return diff --git a/pkg/resources/document_manager.go b/pkg/resources/document_manager.go index c765215..8889e51 100644 --- a/pkg/resources/document_manager.go +++ b/pkg/resources/document_manager.go @@ -5,6 +5,6 @@ import ( ) type DocumentManager interface { - GetDocumentByID(namespace, collection, id string) (*spec.Document, error) + GetDocumentByID(collection, namespace, id string) (*spec.Document, error) GetDocuments(collection, namespace string, limit, offset int) ([]*spec.Document, error) }