Skip to content

Commit

Permalink
fix: Update GetCollections behaviour (#2378)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #2371 

## Description

This PR updates the behaviour of `getCollections` to return inactive
collections if a specific version is requested even if the
`--get-inactive` flag hasn't been set.
  • Loading branch information
fredcarle authored Mar 6, 2024
1 parent b45ad88 commit fb3ce47
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 4 deletions.
4 changes: 2 additions & 2 deletions cli/collection_describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ Example: view all collections
Example: view collection by name
defradb client collection describe --name User
Example: view collection by schema id
Example: view collection by schema root id
defradb client collection describe --schema bae123
Example: view collection by version id
Example: view collection by version id. This will also return inactive collections
defradb client collection describe --version bae123
`,
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
3 changes: 3 additions & 0 deletions client/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ type Store interface {

// GetCollections returns all collections and their descriptions matching the given options
// that currently exist within this [Store].
//
// Inactive collections are not returned by default unless a specific schema version ID
// is provided.
GetCollections(context.Context, CollectionFetchOptions) ([]Collection, error)

// GetSchemaByVersionID returns the schema description for the schema version of the
Expand Down
8 changes: 6 additions & 2 deletions db/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,8 +709,11 @@ func (db *db) getCollectionByName(ctx context.Context, txn datastore.Txn, name s
return cols[0], nil
}

// GetCollections returns all collections and their descriptions matching the given options
// getCollections returns all collections and their descriptions matching the given options
// that currently exist within this [Store].
//
// Inactive collections are not returned by default unless a specific schema version ID
// is provided.
func (db *db) getCollections(
ctx context.Context,
txn datastore.Txn,
Expand Down Expand Up @@ -763,7 +766,8 @@ func (db *db) getCollections(
continue
}
}
if !options.IncludeInactive.Value() && !col.Name.HasValue() {
// By default, we don't return inactive collections unless a specific version is requested.
if !options.IncludeInactive.Value() && !col.Name.HasValue() && !options.SchemaVersionID.HasValue() {
continue
}

Expand Down
40 changes: 40 additions & 0 deletions tests/integration/schema/updates/with_schema_branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,3 +543,43 @@ func TestSchemaUpdates_WithBranchingSchemaAndSetActiveSchemaToOtherBranchThenPat
}
testUtils.ExecuteTestCase(t, test)
}

func TestSchemaUpdates_WithBranchingSchemaAndGetCollectionAtVersion(t *testing.T) {
schemaVersion1ID := "bafkreiebcgze3rs6j3g7gu65dwskdg5fn3qby5c6nqffhbdkcy2l5bbvp4"

test := testUtils.TestCase{
Description: `Test schema update, with branching schema toggling between branches and gets the
collection at a specific version`,
Actions: []any{
testUtils.SchemaUpdate{
Schema: `
type Users {
name: String
}
`,
},
testUtils.SchemaPatch{
// The second schema version will not be set as the active version, leaving the initial version active
SetAsDefaultVersion: immutable.Some(true),
Patch: `
[
{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "email", "Kind": 11} }
]
`,
},
testUtils.GetCollections{
FilterOptions: client.CollectionFetchOptions{
SchemaVersionID: immutable.Some(schemaVersion1ID),
},
ExpectedResults: []client.CollectionDescription{
{
// The original collection version is present, it has no source and is inactive (has no name).
ID: 1,
SchemaVersionID: schemaVersion1ID,
},
},
},
},
}
testUtils.ExecuteTestCase(t, test)
}

0 comments on commit fb3ce47

Please sign in to comment.