Handle namespace validation at the route level rather than at data manager

docs/reference/config.md

@@ -581,13 +581,6 @@ nav_order: 2
 |default|The default namespace - must be in the predefined list|`string`|`<nil>`
 |predefined|A list of namespaces to ensure exists, without requiring a broadcast from the network|List `string`|`<nil>`
 
-## namespaces.cache
-
-|Key|Description|Type|Default Value|
-|---|-----------|----|-------------|
-|size|The size of the cache|[`BytesSize`](https://pkg.go.dev/github.com/docker/go-units#BytesSize)|`<nil>`
-|ttl|The time to live (TTL) for the cache|[`time.Duration`](https://pkg.go.dev/time#Duration)|`<nil>`
-
 ## namespaces.predefined[]
 
 |Key|Description|Type|Default Value|

internal/apiserver/route_spi_get_ops.go

@@ -34,11 +34,11 @@ var spiGetOps = &ffapi.Route{
 	JSONInputValue:  nil,
 	JSONOutputValue: func() interface{} { return []*core.Operation{} },
 	JSONOutputCodes: []int{http.StatusOK},
+	Tag:             routeTagNonDefaultNamespace,
 	Extensions: &coreExtensions{
 		FilterFactory: database.OperationQueryFactory,
 		CoreJSONHandler: func(r *ffapi.APIRequest, cr *coreRequest) (output interface{}, err error) {
-			or := cr.mgr.Orchestrator(r.PP["ns"])
-			return filterResult(or.GetOperations(cr.ctx, cr.filter))
+			return filterResult(cr.or.GetOperations(cr.ctx, cr.filter))
 		},
 	},
 }

internal/apiserver/server.go

@@ -200,7 +200,11 @@ func (as *apiServer) swaggerGenerator(routes []*ffapi.Route, apiBaseURL string)
 func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL string) func(req *http.Request) (*openapi3.T, error) {
 	return func(req *http.Request) (*openapi3.T, error) {
 		vars := mux.Vars(req)
-		cm := mgr.Orchestrator(vars["ns"]).Contracts()
+		or := mgr.Orchestrator(vars["ns"])
+		if or == nil {
+			return nil, i18n.NewError(req.Context(), coremsgs.MsgNamespaceDoesNotExist)
+		}
+		cm := or.Contracts()
 		api, err := cm.GetContractAPI(req.Context(), apiBaseURL, vars["apiName"])
 		if err != nil {
 			return nil, err
@@ -218,17 +222,22 @@ func (as *apiServer) contractSwaggerGenerator(mgr namespace.Manager, apiBaseURL
 	}
 }
 
-func getOrchestrator(mgr namespace.Manager, tag string, r *ffapi.APIRequest) orchestrator.Orchestrator {
-	if tag == routeTagDefaultNamespace {
-		return mgr.Orchestrator(config.GetString(coreconfig.NamespacesDefault))
-	}
-	if tag == routeTagNonDefaultNamespace {
+func getOrchestrator(ctx context.Context, mgr namespace.Manager, tag string, r *ffapi.APIRequest) (or orchestrator.Orchestrator, err error) {
+	switch tag {
+	case routeTagDefaultNamespace:
+		or = mgr.Orchestrator(config.GetString(coreconfig.NamespacesDefault))
+	case routeTagNonDefaultNamespace:
 		vars := mux.Vars(r.Req)
 		if ns, ok := vars["ns"]; ok {
-			return mgr.Orchestrator(ns)
+			or = mgr.Orchestrator(ns)
 		}
+	default:
+		return nil, nil
+	}
+	if or == nil {
+		return nil, i18n.NewError(ctx, coremsgs.MsgNamespaceDoesNotExist)
 	}
-	return nil
+	return or, nil
 }
 
 func (as *apiServer) routeHandler(hf *ffapi.HandlerFactory, mgr namespace.Manager, apiBaseURL string, route *ffapi.Route) http.HandlerFunc {
@@ -244,9 +253,14 @@ func (as *apiServer) routeHandler(hf *ffapi.HandlerFactory, mgr namespace.Manage
 			}
 		}
 
+		or, err := getOrchestrator(r.Req.Context(), mgr, route.Tag, r)
+		if err != nil {
+			return nil, err
+		}
+
 		cr := &coreRequest{
 			mgr:        mgr,
-			or:         getOrchestrator(mgr, route.Tag, r),
+			or:         or,
 			ctx:        r.Req.Context(),
 			filter:     filter,
 			apiBaseURL: apiBaseURL,
@@ -255,9 +269,14 @@ func (as *apiServer) routeHandler(hf *ffapi.HandlerFactory, mgr namespace.Manage
 	}
 	if ce.CoreFormUploadHandler != nil {
 		route.FormUploadHandler = func(r *ffapi.APIRequest) (output interface{}, err error) {
+			or, err := getOrchestrator(r.Req.Context(), mgr, route.Tag, r)
+			if err != nil {
+				return nil, err
+			}
+
 			cr := &coreRequest{
 				mgr:        mgr,
-				or:         getOrchestrator(mgr, route.Tag, r),
+				or:         or,
 				ctx:        r.Req.Context(),
 				apiBaseURL: apiBaseURL,
 			}

internal/apiserver/server_test.go

@@ -17,10 +17,12 @@
 package apiserver
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"mime/multipart"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -50,7 +52,9 @@ func newTestServer() (*namespacemocks.Manager, *orchestratormocks.Orchestrator,
 	InitConfig()
 	mgr := &namespacemocks.Manager{}
 	o := &orchestratormocks.Orchestrator{}
-	mgr.On("Orchestrator", mock.Anything).Return(o)
+	mgr.On("Orchestrator", "default").Return(o).Maybe()
+	mgr.On("Orchestrator", "mynamespace").Return(o).Maybe()
+	mgr.On("Orchestrator", "ns1").Return(o).Maybe()
 	as := &apiServer{
 		apiTimeout:     5 * time.Second,
 		maxFilterLimit: 100,
@@ -322,6 +326,21 @@ func TestContractAPISwaggerJSONGetFFIFail(t *testing.T) {
 	assert.Equal(t, 500, res.StatusCode)
 }
 
+func TestContractAPISwaggerJSONBadNamespace(t *testing.T) {
+	mgr, o, as := newTestServer()
+	r := as.createMuxRouter(context.Background(), mgr)
+	mcm := &contractmocks.Manager{}
+	o.On("Contracts").Return(mcm)
+	s := httptest.NewServer(r)
+	defer s.Close()
+
+	mgr.On("Orchestrator", "BAD").Return(nil)
+
+	res, err := http.Get(fmt.Sprintf("http://%s/api/v1/namespaces/BAD/apis/my-api/api/swagger.json", s.Listener.Addr()))
+	assert.NoError(t, err)
+	assert.Equal(t, 404, res.StatusCode)
+}
+
 func TestContractAPISwaggerUI(t *testing.T) {
 	_, r := newTestAPIServer()
 	s := httptest.NewServer(r)
@@ -333,3 +352,44 @@ func TestContractAPISwaggerUI(t *testing.T) {
 	b, _ := ioutil.ReadAll(res.Body)
 	assert.Regexp(t, "html", string(b))
 }
+
+func TestJSONBadNamespace(t *testing.T) {
+	mgr, _, as := newTestServer()
+	r := as.createMuxRouter(context.Background(), mgr)
+	s := httptest.NewServer(r)
+	defer s.Close()
+
+	mgr.On("Orchestrator", "BAD").Return(nil)
+
+	var b bytes.Buffer
+	req := httptest.NewRequest("GET", "/api/v1/namespaces/BAD/apis", &b)
+	req.Header.Set("Content-Type", "application/json; charset=utf-8")
+	res := httptest.NewRecorder()
+
+	r.ServeHTTP(res, req)
+
+	assert.Equal(t, 404, res.Result().StatusCode)
+}
+
+func TestFormDataBadNamespace(t *testing.T) {
+	mgr, _, as := newTestServer()
+	r := as.createMuxRouter(context.Background(), mgr)
+	s := httptest.NewServer(r)
+	defer s.Close()
+
+	mgr.On("Orchestrator", "BAD").Return(nil)
+
+	var b bytes.Buffer
+	w := multipart.NewWriter(&b)
+	writer, err := w.CreateFormFile("file", "filename.ext")
+	assert.NoError(t, err)
+	writer.Write([]byte(`some data`))
+	w.Close()
+	req := httptest.NewRequest("POST", "/api/v1/namespaces/BAD/data", &b)
+	req.Header.Set("Content-Type", w.FormDataContentType())
+	res := httptest.NewRecorder()
+
+	r.ServeHTTP(res, req)
+
+	assert.Equal(t, 404, res.Result().StatusCode)
+}

internal/assets/manager.go

@@ -84,7 +84,7 @@ type assetManager struct {
 	keyNormalization int
 }
 
-func NewAssetManager(ctx context.Context, ns string, di database.Plugin, im identity.Manager, sa syncasync.Bridge, bm broadcast.Manager, pm privatemessaging.Manager, ti map[string]tokens.Plugin, mm metrics.Manager, om operations.Manager, txHelper txcommon.Helper) (Manager, error) {
+func NewAssetManager(ctx context.Context, ns string, di database.Plugin, ti map[string]tokens.Plugin, im identity.Manager, sa syncasync.Bridge, bm broadcast.Manager, pm privatemessaging.Manager, mm metrics.Manager, om operations.Manager, txHelper txcommon.Helper) (Manager, error) {
 	if di == nil || im == nil || sa == nil || bm == nil || pm == nil || ti == nil || mm == nil || om == nil {
 		return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "AssetManager")
 	}

internal/assets/manager_test.go

@@ -63,7 +63,7 @@ func newTestAssetsCommon(t *testing.T, metrics bool) (*assetManager, func()) {
 	mom.On("RegisterHandler", mock.Anything, mock.Anything, mock.Anything)
 	mti.On("Name").Return("ut").Maybe()
 	ctx, cancel := context.WithCancel(context.Background())
-	a, err := NewAssetManager(ctx, "ns1", mdi, mim, msa, mbm, mpm, map[string]tokens.Plugin{"magic-tokens": mti}, mm, mom, txHelper)
+	a, err := NewAssetManager(ctx, "ns1", mdi, map[string]tokens.Plugin{"magic-tokens": mti}, mim, msa, mbm, mpm, mm, mom, txHelper)
 	rag := mdi.On("RunAsGroup", mock.Anything, mock.Anything).Maybe()
 	rag.RunFn = func(a mock.Arguments) {
 		rag.ReturnArguments = mock.Arguments{a[1].(func(context.Context) error)(a[0].(context.Context))}

internal/broadcast/datatype.go

@@ -35,9 +35,6 @@ func (bm *broadcastManager) BroadcastDatatype(ctx context.Context, datatype *cor
 	if err := datatype.Validate(ctx, false); err != nil {
 		return nil, err
 	}
-	if err := bm.data.VerifyNamespaceExists(ctx, datatype.Namespace); err != nil {
-		return nil, err
-	}
 	datatype.Hash = datatype.Value.Hash()
 
 	// Verify the data type is now all valid, before we broadcast it

internal/broadcast/datatype_test.go

@@ -40,25 +40,10 @@ func TestBroadcastDatatypeBadType(t *testing.T) {
 	assert.Regexp(t, "FF00111.*validator", err)
 }
 
-func TestBroadcastDatatypeNSGetFail(t *testing.T) {
-	bm, cancel := newTestBroadcast(t)
-	defer cancel()
-	mdm := bm.data.(*datamocks.Manager)
-	mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(fmt.Errorf("pop"))
-	_, err := bm.BroadcastDatatype(context.Background(), &core.Datatype{
-		Name:      "name1",
-		Namespace: "ns1",
-		Version:   "0.0.1",
-		Value:     fftypes.JSONAnyPtr(`{}`),
-	}, false)
-	assert.EqualError(t, err, "pop")
-}
-
 func TestBroadcastDatatypeBadValue(t *testing.T) {
 	bm, cancel := newTestBroadcast(t)
 	defer cancel()
 	mdm := bm.data.(*datamocks.Manager)
-	mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil)
 	mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(nil)
 	mim := bm.identity.(*identitymanagermocks.Manager)
 	mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil)
@@ -79,7 +64,6 @@ func TestBroadcastUpsertFail(t *testing.T) {
 
 	mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil)
 	mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(fmt.Errorf("pop"))
-	mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil)
 	mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(nil)
 
 	_, err := bm.BroadcastDatatype(context.Background(), &core.Datatype{
@@ -103,7 +87,6 @@ func TestBroadcastDatatypeInvalid(t *testing.T) {
 
 	mim.On("ResolveInputIdentity", mock.Anything, mock.Anything).Return(nil)
 	mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil)
-	mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil)
 	mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(fmt.Errorf("pop"))
 
 	_, err := bm.BroadcastDatatype(context.Background(), &core.Datatype{
@@ -122,7 +105,6 @@ func TestBroadcastOk(t *testing.T) {
 	mim := bm.identity.(*identitymanagermocks.Manager)
 
 	mim.On("ResolveInputSigningIdentity", mock.Anything, mock.Anything).Return(nil)
-	mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil)
 	mdm.On("CheckDatatype", mock.Anything, mock.Anything).Return(nil)
 	mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(nil)
 

internal/broadcast/manager.go

@@ -76,7 +76,7 @@ type broadcastManager struct {
 	operations            operations.Manager
 }
 
-func NewBroadcastManager(ctx context.Context, ns string, di database.Plugin, im identity.Manager, dm data.Manager, bi blockchain.Plugin, dx dataexchange.Plugin, si sharedstorage.Plugin, ba batch.Manager, sa syncasync.Bridge, bp batchpin.Submitter, mm metrics.Manager, om operations.Manager) (Manager, error) {
+func NewBroadcastManager(ctx context.Context, ns string, di database.Plugin, bi blockchain.Plugin, dx dataexchange.Plugin, si sharedstorage.Plugin, im identity.Manager, dm data.Manager, ba batch.Manager, sa syncasync.Bridge, bp batchpin.Submitter, mm metrics.Manager, om operations.Manager) (Manager, error) {
 	if di == nil || im == nil || dm == nil || bi == nil || dx == nil || si == nil || ba == nil || mm == nil || om == nil {
 		return nil, i18n.NewError(ctx, coremsgs.MsgInitializationNilDepError, "BroadcastManager")
 	}

internal/broadcast/manager_test.go

@@ -76,7 +76,7 @@ func newTestBroadcastCommon(t *testing.T, metricsEnabled bool) (*broadcastManage
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())
-	b, err := NewBroadcastManager(ctx, "ns1", mdi, mim, mdm, mbi, mdx, mpi, mba, msa, mbp, mmi, mom)
+	b, err := NewBroadcastManager(ctx, "ns1", mdi, mbi, mdx, mpi, mim, mdm, mba, msa, mbp, mmi, mom)
 	assert.NoError(t, err)
 	return b.(*broadcastManager), cancel
 }

internal/broadcast/message.go

@@ -114,10 +114,6 @@ func (s *broadcastSender) resolveAndSend(ctx context.Context, method sendMethod)
 func (s *broadcastSender) resolve(ctx context.Context) error {
 	msg := s.msg.Message
 
-	if err := s.mgr.data.VerifyNamespaceExists(ctx, msg.Header.Namespace); err != nil {
-		return err
-	}
-
 	// Resolve the sending identity
 	if msg.Header.Type != core.MessageTypeDefinition || msg.Header.Tag != core.SystemTagIdentityClaim {
 		if err := s.mgr.identity.ResolveInputSigningIdentity(ctx, &msg.Header.SignerRef); err != nil {

internal/broadcast/message_test.go

@@ -39,7 +39,6 @@ func TestBroadcastMessageOk(t *testing.T) {
 	mim := bm.identity.(*identitymanagermocks.Manager)
 
 	ctx := context.Background()
-	mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil)
 	mdm.On("ResolveInlineData", ctx, mock.Anything).Return(nil)
 	mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil)
@@ -72,7 +71,6 @@ func TestBroadcastMessageWaitConfirmOk(t *testing.T) {
 	mim := bm.identity.(*identitymanagermocks.Manager)
 
 	ctx := context.Background()
-	mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil)
 	mdm.On("ResolveInlineData", ctx, mock.Anything).Return(nil)
 	mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil)
 
@@ -119,7 +117,6 @@ func TestBroadcastMessageTooLarge(t *testing.T) {
 	mim := bm.identity.(*identitymanagermocks.Manager)
 
 	ctx := context.Background()
-	mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil)
 	mdm.On("ResolveInlineData", ctx, mock.Anything).Run(
 		func(args mock.Arguments) {
 			newMsg := args[1].(*data.NewMessage)
@@ -155,7 +152,6 @@ func TestBroadcastMessageBadInput(t *testing.T) {
 	mim := bm.identity.(*identitymanagermocks.Manager)
 
 	ctx := context.Background()
-	mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil)
 	mdm.On("ResolveInlineData", ctx, mock.Anything).Return(fmt.Errorf("pop"))
 	mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil)
 
@@ -175,8 +171,6 @@ func TestBroadcastMessageBadIdentity(t *testing.T) {
 
 	ctx := context.Background()
 	mim := bm.identity.(*identitymanagermocks.Manager)
-	mdm := bm.data.(*datamocks.Manager)
-	mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil)
 	mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(fmt.Errorf("pop"))
 
 	_, err := bm.BroadcastMessage(ctx, &core.MessageInOut{
@@ -186,7 +180,6 @@ func TestBroadcastMessageBadIdentity(t *testing.T) {
 	}, false)
 	assert.Regexp(t, "FF10206", err)
 
-	mdm.AssertExpectations(t)
 	mim.AssertExpectations(t)
 }
 
@@ -197,7 +190,6 @@ func TestBroadcastPrepare(t *testing.T) {
 	mim := bm.identity.(*identitymanagermocks.Manager)
 
 	ctx := context.Background()
-	mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(nil)
 	mdm.On("ResolveInlineData", ctx, mock.Anything).Return(nil)
 	mim.On("ResolveInputSigningIdentity", ctx, mock.Anything).Return(nil)
 
@@ -222,32 +214,3 @@ func TestBroadcastPrepare(t *testing.T) {
 
 	mdm.AssertExpectations(t)
 }
-
-func TestBroadcastPrepareBadNamespace(t *testing.T) {
-	bm, cancel := newTestBroadcast(t)
-	defer cancel()
-	mdm := bm.data.(*datamocks.Manager)
-
-	ctx := context.Background()
-	mdm.On("VerifyNamespaceExists", ctx, "ns1").Return(fmt.Errorf("pop"))
-
-	msg := &core.MessageInOut{
-		Message: core.Message{
-			Header: core.MessageHeader{
-				SignerRef: core.SignerRef{
-					Author: "did:firefly:org/abcd",
-					Key:    "0x12345",
-				},
-			},
-		},
-		InlineData: core.InlineData{
-			{Value: fftypes.JSONAnyPtr(`{"hello": "world"}`)},
-		},
-	}
-	sender := bm.NewBroadcast(msg)
-	err := sender.Prepare(ctx)
-
-	assert.EqualError(t, err, "pop")
-
-	mdm.AssertExpectations(t)
-}