Correct GetCollectionsBySchemaID to return multiple cols

sourcenetwork · Oct 19, 2023 · d4be441 · d4be441
1 parent 780df33
commit d4be441
Show file tree

Hide file tree

Showing 11 changed files with 92 additions and 68 deletions.
diff --git a/cli/collection.go b/cli/collection.go
@@ -43,34 +43,17 @@ func MakeCollectionCommand(cfg *config.Config) *cobra.Command {
 			store := mustGetStoreContext(cmd)
 
 			var col client.Collection
+			var cols []client.Collection
 			switch {
 			case versionID != "":
-				var cols []client.Collection
 				cols, err = store.GetCollectionsByVersionID(cmd.Context(), versionID)
-				if err != nil {
-					return err
-				}
-				if name != "" {
-					versionCols := cols
-					cols = nil
-					for _, c := range versionCols {
-						if c.Name() == name {
-							cols = append(cols, c)
-							break
-						}
-					}
-				}
-				if len(cols) != 1 {
-					// If more than one collection matches the given criteria we cannot set the context collection
-					return nil
-				}
-				col = cols[0]
 
 			case schemaID != "":
-				col, err = store.GetCollectionBySchemaID(cmd.Context(), schemaID)
+				cols, err = store.GetCollectionsBySchemaID(cmd.Context(), schemaID)
 
 			case name != "":
 				col, err = store.GetCollectionByName(cmd.Context(), name)
+				cols = []client.Collection{col}
 
 			default:
 				return nil
@@ -79,6 +62,24 @@ func MakeCollectionCommand(cfg *config.Config) *cobra.Command {
 			if err != nil {
 				return err
 			}
+
+			if name != "" {
+				fetchedCols := cols
+				cols = nil
+				for _, c := range fetchedCols {
+					if c.Name() == name {
+						cols = append(cols, c)
+						break
+					}
+				}
+			}
+
+			if len(cols) != 1 {
+				// If more than one collection matches the given criteria we cannot set the context collection
+				return nil
+			}
+			col = cols[0]
+
 			if tx, ok := cmd.Context().Value(txContextKey).(datastore.Txn); ok {
 				col = col.WithTxn(tx)
 			}

diff --git a/client/db.go b/client/db.go
@@ -147,10 +147,10 @@ type Store interface {
 	// If no matching collection is found an error will be returned.
 	GetCollectionByName(context.Context, CollectionName) (Collection, error)
 
-	// GetCollectionBySchemaID attempts to retrieve a collection matching the given schema ID.
+	// GetCollectionsBySchemaID attempts to retrieve all collections using the given schema ID.
 	//
-	// If no matching collection is found an error will be returned.
-	GetCollectionBySchemaID(context.Context, string) (Collection, error)
+	// If no matching collection is found an empty set will be returned.
+	GetCollectionsBySchemaID(context.Context, string) ([]Collection, error)
 
 	// GetCollectionsByVersionID attempts to retrieve all collections using the given schema version ID.
 	//

diff --git a/client/errors.go b/client/errors.go
@@ -116,3 +116,10 @@ func NewErrCollectionNotFoundForSchemaVersion(schemaVersionID string) error {
 		errors.NewKV("SchemaVersionID", schemaVersionID),
 	)
 }
+
+func NewErrCollectionNotFoundForSchema(schemaID string) error {
+	return errors.New(
+		errCollectionNotFound,
+		errors.NewKV("SchemaID", schemaID),
+	)
+}
diff --git a/client/mocks/db.go b/client/mocks/db.go
diff --git a/db/txn_db.go b/db/txn_db.go
@@ -79,11 +79,13 @@ func (db *explicitTxnDB) GetCollectionByName(ctx context.Context, name string) (
 	return db.getCollectionByName(ctx, db.txn, name)
 }
 
-// GetCollectionBySchemaID returns an existing collection using the schema hash ID.
-func (db *implicitTxnDB) GetCollectionBySchemaID(
+// GetCollectionsBySchemaID attempts to retrieve all collections using the given schema ID.
+//
+// If no matching collection is found an empty set will be returned.
+func (db *implicitTxnDB) GetCollectionsBySchemaID(
 	ctx context.Context,
 	schemaID string,
-) (client.Collection, error) {
+) ([]client.Collection, error) {
 	txn, err := db.NewTxn(ctx, true)
 	if err != nil {
 		return nil, err
@@ -94,27 +96,23 @@ func (db *implicitTxnDB) GetCollectionBySchemaID(
 	if err != nil {
 		return nil, err
 	}
-	if len(cols) == 0 {
-		return nil, NewErrFailedToGetCollection(schemaID, err)
-	}
 
-	return cols[0], nil
+	return cols, nil
 }
 
-// GetCollectionBySchemaID returns an existing collection using the schema hash ID.
-func (db *explicitTxnDB) GetCollectionBySchemaID(
+// GetCollectionsBySchemaID attempts to retrieve all collections using the given schema ID.
+//
+// If no matching collection is found an empty set will be returned.
+func (db *explicitTxnDB) GetCollectionsBySchemaID(
 	ctx context.Context,
 	schemaID string,
-) (client.Collection, error) {
+) ([]client.Collection, error) {
 	cols, err := db.getCollectionsBySchemaID(ctx, db.txn, schemaID)
 	if err != nil {
 		return nil, err
 	}
-	if len(cols) == 0 {
-		return nil, NewErrFailedToGetCollection(schemaID, err)
-	}
 
-	return cols[0], nil
+	return cols, nil
 }
 
 // GetCollectionsByVersionID attempts to retrieve all collections using the given schema version ID.

diff --git a/http/client.go b/http/client.go
@@ -186,19 +186,23 @@ func (c *Client) GetCollectionByName(ctx context.Context, name client.Collection
 	return &Collection{c.http, definition}, nil
 }
 
-func (c *Client) GetCollectionBySchemaID(ctx context.Context, schemaId string) (client.Collection, error) {
+func (c *Client) GetCollectionsBySchemaID(ctx context.Context, schemaId string) ([]client.Collection, error) {
 	methodURL := c.http.baseURL.JoinPath("collections")
 	methodURL.RawQuery = url.Values{"schema_id": []string{schemaId}}.Encode()
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, methodURL.String(), nil)
 	if err != nil {
 		return nil, err
 	}
-	var definition client.CollectionDefinition
-	if err := c.http.requestJson(req, &definition); err != nil {
+	var descriptions []client.CollectionDefinition
+	if err := c.http.requestJson(req, &descriptions); err != nil {
 		return nil, err
 	}
-	return &Collection{c.http, definition}, nil
+	collections := make([]client.Collection, len(descriptions))
+	for i, d := range descriptions {
+		collections[i] = &Collection{c.http, d}
+	}
+	return collections, nil
 }
 
 func (c *Client) GetCollectionsByVersionID(ctx context.Context, versionId string) ([]client.Collection, error) {

diff --git a/http/handler_store.go b/http/handler_store.go
@@ -118,12 +118,16 @@ func (s *storeHandler) GetCollection(rw http.ResponseWriter, req *http.Request)
 		}
 		responseJSON(rw, http.StatusOK, col.Definition())
 	case req.URL.Query().Has("schema_id"):
-		col, err := store.GetCollectionBySchemaID(req.Context(), req.URL.Query().Get("schema_id"))
+		cols, err := store.GetCollectionsBySchemaID(req.Context(), req.URL.Query().Get("schema_id"))
 		if err != nil {
 			responseJSON(rw, http.StatusBadRequest, errorResponse{err})
 			return
 		}
-		responseJSON(rw, http.StatusOK, col.Definition())
+		colDesc := make([]client.CollectionDefinition, len(cols))
+		for i, col := range cols {
+			colDesc[i] = col.Definition()
+		}
+		responseJSON(rw, http.StatusOK, colDesc)
 	case req.URL.Query().Has("version_id"):
 		cols, err := store.GetCollectionsByVersionID(req.Context(), req.URL.Query().Get("version_id"))
 		if err != nil {

diff --git a/net/peer_collection.go b/net/peer_collection.go
@@ -31,11 +31,11 @@ func (p *Peer) AddP2PCollections(ctx context.Context, collectionIDs []string) er
 	// first let's make sure the collections actually exists
 	storeCollections := []client.Collection{}
 	for _, col := range collectionIDs {
-		storeCol, err := p.db.WithTxn(txn).GetCollectionBySchemaID(p.ctx, col)
+		storeCol, err := p.db.WithTxn(txn).GetCollectionsBySchemaID(p.ctx, col)
 		if err != nil {
 			return err
 		}
-		storeCollections = append(storeCollections, storeCol)
+		storeCollections = append(storeCollections, storeCol...)
 	}
 
 	// Ensure we can add all the collections to the store on the transaction
@@ -93,11 +93,11 @@ func (p *Peer) RemoveP2PCollections(ctx context.Context, collectionIDs []string)
 	// first let's make sure the collections actually exists
 	storeCollections := []client.Collection{}
 	for _, col := range collectionIDs {
-		storeCol, err := p.db.WithTxn(txn).GetCollectionBySchemaID(p.ctx, col)
+		storeCol, err := p.db.WithTxn(txn).GetCollectionsBySchemaID(p.ctx, col)
 		if err != nil {
 			return err
 		}
-		storeCollections = append(storeCollections, storeCol)
+		storeCollections = append(storeCollections, storeCol...)
 	}
 
 	// Ensure we can remove all the collections to the store on the transaction

diff --git a/net/server.go b/net/server.go
@@ -260,10 +260,16 @@ func (s *server) PushLog(ctx context.Context, req *pb.PushLogRequest) (*pb.PushL
 		defer txn.Discard(ctx)
 		store := s.db.WithTxn(txn)
 
-		col, err := store.GetCollectionBySchemaID(ctx, schemaID)
+		// Currently a schema is the best way we have to link a push log request to a collection,
+		// this will change with https://github.com/sourcenetwork/defradb/issues/1085
+		cols, err := store.GetCollectionsBySchemaID(ctx, schemaID)
 		if err != nil {
 			return nil, errors.Wrap(fmt.Sprintf("Failed to get collection from schemaID %s", schemaID), err)
 		}
+		if len(cols) == 0 {
+			return nil, client.NewErrCollectionNotFoundForSchema(schemaID)
+		}
+		col := cols[0]
 
 		// Create a new DAG service with the current transaction
 		var getter format.NodeGetter = s.peer.newDAGSyncerTxn(txn)

diff --git a/tests/clients/cli/wrapper.go b/tests/clients/cli/wrapper.go
@@ -225,19 +225,23 @@ func (w *Wrapper) GetCollectionByName(ctx context.Context, name client.Collectio
 	return &Collection{w.cmd, definition}, nil
 }
 
-func (w *Wrapper) GetCollectionBySchemaID(ctx context.Context, schemaId string) (client.Collection, error) {
+func (w *Wrapper) GetCollectionsBySchemaID(ctx context.Context, schemaId string) ([]client.Collection, error) {
 	args := []string{"client", "collection", "describe"}
 	args = append(args, "--schema", schemaId)
 
 	data, err := w.cmd.execute(ctx, args)
 	if err != nil {
 		return nil, err
 	}
-	var definition client.CollectionDefinition
-	if err := json.Unmarshal(data, &definition); err != nil {
+	var colDesc []client.CollectionDefinition
+	if err := json.Unmarshal(data, &colDesc); err != nil {
 		return nil, err
 	}
-	return &Collection{w.cmd, definition}, nil
+	cols := make([]client.Collection, len(colDesc))
+	for i, v := range colDesc {
+		cols[i] = &Collection{w.cmd, v}
+	}
+	return cols, err
 }
 
 func (w *Wrapper) GetCollectionsByVersionID(ctx context.Context, versionId string) ([]client.Collection, error) {

diff --git a/tests/clients/http/wrapper.go b/tests/clients/http/wrapper.go
@@ -115,8 +115,8 @@ func (w *Wrapper) GetCollectionByName(ctx context.Context, name client.Collectio
 	return w.client.GetCollectionByName(ctx, name)
 }
 
-func (w *Wrapper) GetCollectionBySchemaID(ctx context.Context, schemaId string) (client.Collection, error) {
-	return w.client.GetCollectionBySchemaID(ctx, schemaId)
+func (w *Wrapper) GetCollectionsBySchemaID(ctx context.Context, schemaId string) ([]client.Collection, error) {
+	return w.client.GetCollectionsBySchemaID(ctx, schemaId)
 }
 
 func (w *Wrapper) GetCollectionsByVersionID(ctx context.Context, versionId string) ([]client.Collection, error) {