Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Change GetCollectionBySchemaFoo funcs to return many #1984

Merged
21 changes: 20 additions & 1 deletion cli/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,26 @@ func MakeCollectionCommand(cfg *config.Config) *cobra.Command {
var col client.Collection
switch {
case versionID != "":
col, err = store.GetCollectionByVersionID(cmd.Context(), 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)
Expand Down
6 changes: 3 additions & 3 deletions client/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ type Store interface {
// If no matching collection is found an error will be returned.
GetCollectionBySchemaID(context.Context, string) (Collection, error)

// GetCollectionBySchemaID attempts to retrieve a collection matching the given schema version ID.
// GetCollectionsByVersionID attempts to retrieve all collections using the given schema version ID.
//
// If no matching collection is found an error will be returned.
GetCollectionByVersionID(context.Context, string) (Collection, error)
// If no matching collections are found an empty set will be returned.
GetCollectionsByVersionID(context.Context, string) ([]Collection, error)

// GetAllCollections returns all the collections and their descriptions that currently exist within
// this [Store].
Expand Down
9 changes: 9 additions & 0 deletions client/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const (
errUninitializeProperty string = "invalid state, required property is uninitialized"
errMaxTxnRetries string = "reached maximum transaction reties"
errRelationOneSided string = "relation must be defined on both schemas"
errCollectionNotFound string = "collection not found"
)

// Errors returnable from this package.
Expand All @@ -45,6 +46,7 @@ var (
ErrInvalidDocKeyVersion = errors.New("invalid DocKey version")
ErrMaxTxnRetries = errors.New(errMaxTxnRetries)
ErrRelationOneSided = errors.New(errRelationOneSided)
ErrCollectionNotFound = errors.New(errCollectionNotFound)
)

// NewErrFieldNotExist returns an error indicating that the given field does not exist.
Expand Down Expand Up @@ -107,3 +109,10 @@ func NewErrRelationOneSided(fieldName string, typeName string) error {
errors.NewKV("Type", typeName),
)
}

func NewErrCollectionNotFoundForSchemaVersion(schemaVersionID string) error {
return errors.New(
errCollectionNotFound,
errors.NewKV("SchemaVersionID", schemaVersionID),
)
}
28 changes: 14 additions & 14 deletions client/mocks/db.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 20 additions & 12 deletions db/txn_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,12 @@ func (db *explicitTxnDB) GetCollectionBySchemaID(
return cols[0], nil
}

// GetCollectionByVersionID returns an existing collection using the schema version hash ID.
func (db *implicitTxnDB) GetCollectionByVersionID(
// GetCollectionsByVersionID attempts to retrieve all collections using the given schema version ID.
//
// If no matching collections are found an empty set will be returned.
func (db *implicitTxnDB) GetCollectionsByVersionID(
ctx context.Context, schemaVersionID string,
) (client.Collection, error) {
) ([]client.Collection, error) {
txn, err := db.NewTxn(ctx, true)
if err != nil {
return nil, err
Expand All @@ -131,26 +133,32 @@ func (db *implicitTxnDB) GetCollectionByVersionID(
if err != nil {
return nil, err
}
if len(cols) == 0 {
return nil, NewErrFailedToGetCollection(schemaVersionID, err)

collections := make([]client.Collection, len(cols))
for i, col := range cols {
collections[i] = col
}

return cols[0], nil
return collections, nil
}

// GetCollectionByVersionID returns an existing collection using the schema version hash ID.
func (db *explicitTxnDB) GetCollectionByVersionID(
// GetCollectionsByVersionID attempts to retrieve all collections using the given schema version ID.
//
// If no matching collections are found an empty set will be returned.
func (db *explicitTxnDB) GetCollectionsByVersionID(
ctx context.Context, schemaVersionID string,
) (client.Collection, error) {
) ([]client.Collection, error) {
cols, err := db.getCollectionsByVersionID(ctx, db.txn, schemaVersionID)
if err != nil {
return nil, err
}
if len(cols) == 0 {
return nil, NewErrFailedToGetCollection(schemaVersionID, err)

collections := make([]client.Collection, len(cols))
for i, col := range cols {
collections[i] = col
}

return cols[0], nil
return collections, nil
}

// GetAllCollections gets all the currently defined collections.
Expand Down
12 changes: 8 additions & 4 deletions http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,23 @@ func (c *Client) GetCollectionBySchemaID(ctx context.Context, schemaId string) (
return &Collection{c.http, definition}, nil
}

func (c *Client) GetCollectionByVersionID(ctx context.Context, versionId string) (client.Collection, error) {
func (c *Client) GetCollectionsByVersionID(ctx context.Context, versionId string) ([]client.Collection, error) {
methodURL := c.http.baseURL.JoinPath("collections")
methodURL.RawQuery = url.Values{"version_id": []string{versionId}}.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) GetAllCollections(ctx context.Context) ([]client.Collection, error) {
Expand Down
8 changes: 6 additions & 2 deletions http/handler_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,16 @@ func (s *storeHandler) GetCollection(rw http.ResponseWriter, req *http.Request)
}
responseJSON(rw, http.StatusOK, col.Definition())
case req.URL.Query().Has("version_id"):
col, err := store.GetCollectionByVersionID(req.Context(), req.URL.Query().Get("version_id"))
cols, err := store.GetCollectionsByVersionID(req.Context(), req.URL.Query().Get("version_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)
default:
cols, err := store.GetAllCollections(req.Context())
if err != nil {
Expand Down
20 changes: 16 additions & 4 deletions planner/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,17 @@ func (n *dagScanNode) dagBlockToNodeDoc(block blocks.Block) (core.Doc, []*ipld.L
fieldName = nil

default:
c, err := n.planner.db.GetCollectionByVersionID(n.planner.ctx, schemaVersionId)
cols, err := n.planner.db.GetCollectionsByVersionID(n.planner.ctx, schemaVersionId)
if err != nil {
return core.Doc{}, nil, err
}
if len(cols) == 0 {
return core.Doc{}, nil, client.NewErrCollectionNotFoundForSchemaVersion(schemaVersionId)
}

field, ok := c.Schema().GetField(fieldName.(string))
// Because we only care about the schema, we can safely take the first - the schema is the same
// for all in the set.
field, ok := cols[0].Schema().GetField(fieldName.(string))
if !ok {
return core.Doc{}, nil, client.NewErrFieldNotExist(fieldName.(string))
}
Expand All @@ -353,13 +358,20 @@ func (n *dagScanNode) dagBlockToNodeDoc(block blocks.Block) (core.Doc, []*ipld.L
n.commitSelect.DocumentMapping.SetFirstOfName(&commit,
request.DockeyFieldName, string(dockey))

collection, err := n.planner.db.GetCollectionByVersionID(n.planner.ctx, schemaVersionId)
cols, err := n.planner.db.GetCollectionsByVersionID(n.planner.ctx, schemaVersionId)
if err != nil {
return core.Doc{}, nil, err
}
if len(cols) == 0 {
return core.Doc{}, nil, client.NewErrCollectionNotFoundForSchemaVersion(schemaVersionId)
}

// WARNING: This will become incorrect once we allow multiple collections to share the same schema
// todo: link to tickets.
// We should instead fetch the collection be global collection ID:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: by global...

Copy link
Contributor Author

@AndrewSisley AndrewSisley Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I managed to leave half the todo text in there too..., thanks for spotting Fred :)

  • Fix warning text

// https://github.com/sourcenetwork/defradb/issues/1085
n.commitSelect.DocumentMapping.SetFirstOfName(&commit,
request.CollectionIDFieldName, int64(collection.ID()))
request.CollectionIDFieldName, int64(cols[0].ID()))

heads := make([]*ipld.Link, 0)

Expand Down
12 changes: 8 additions & 4 deletions tests/clients/cli/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,23 @@ func (w *Wrapper) GetCollectionBySchemaID(ctx context.Context, schemaId string)
return &Collection{w.cmd, definition}, nil
}

func (w *Wrapper) GetCollectionByVersionID(ctx context.Context, versionId string) (client.Collection, error) {
func (w *Wrapper) GetCollectionsByVersionID(ctx context.Context, versionId string) ([]client.Collection, error) {
args := []string{"client", "collection", "describe"}
args = append(args, "--version", versionId)

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) GetAllCollections(ctx context.Context) ([]client.Collection, error) {
Expand Down
4 changes: 2 additions & 2 deletions tests/clients/http/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ func (w *Wrapper) GetCollectionBySchemaID(ctx context.Context, schemaId string)
return w.client.GetCollectionBySchemaID(ctx, schemaId)
}

func (w *Wrapper) GetCollectionByVersionID(ctx context.Context, versionId string) (client.Collection, error) {
return w.client.GetCollectionByVersionID(ctx, versionId)
func (w *Wrapper) GetCollectionsByVersionID(ctx context.Context, versionId string) ([]client.Collection, error) {
return w.client.GetCollectionsByVersionID(ctx, versionId)
}

func (w *Wrapper) GetAllCollections(ctx context.Context) ([]client.Collection, error) {
Expand Down