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.

Sign up for GitHub

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

refactor: Merge collection UpdateWith and DeleteWith #2531

Merged
merged 8 commits into from
Apr 22, 2024
Merged

refactor: Merge collection UpdateWith and DeleteWith #2531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
68ee852
merge collection UpdateWith and DeleteWith functions
nasdf Apr 19, 2024
a1d39ac
update openapi operation ids
nasdf Apr 19, 2024
3136915
fix bug in cli update collection
nasdf Apr 19, 2024
415e7c1
clean up planner update
nasdf Apr 19, 2024
06fbdc2
allow setting nil values in document
nasdf Apr 19, 2024
73323b2
Merge branch 'develop' into nasdf/refactor/collection-update-delete-with
nasdf Apr 20, 2024
d31f076
change collection update and collection delete cli param checks
nasdf Apr 20, 2024
1b4c429
fix planner update
nasdf Apr 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 10 additions & 27 deletions cli/collection_delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ import (
)

func MakeCollectionDeleteCommand() *cobra.Command {
var argDocIDs []string
var argDocID string
var filter string
var cmd = &cobra.Command{
Use: "delete [-i --identity] [--filter <filter> --docID <docID>]",
Short: "Delete documents by docID or filter.",
Long: `Delete documents by docID or filter and lists the number of documents deleted.

Example: delete by docID(s):
defradb client collection delete --name User --docID bae-123,bae-456
Example: delete by docID:
defradb client collection delete --name User --docID bae-123

Example: delete by docID(s) with identity:
defradb client collection delete -i cosmos1f2djr7dl9vhrk3twt3xwqp09nhtzec9mdkf70j --name User --docID bae-123,bae-456
Example: delete by docID with identity:
defradb client collection delete -i cosmos1f2djr7dl9vhrk3twt3xwqp09nhtzec9mdkf70j --name User --docID bae-123

Example: delete by filter:
defradb client collection delete --name User --filter '{ "_gte": { "points": 100 } }'
Expand All @@ -40,30 +40,13 @@ Example: delete by filter:
}

switch {
case len(argDocIDs) == 1:
docID, err := client.NewDocIDFromString(argDocIDs[0])
case argDocID != "":
docID, err := client.NewDocIDFromString(argDocID)
if err != nil {
return err
}
res, err := col.DeleteWithDocID(cmd.Context(), docID)
if err != nil {
return err
}
return writeJSON(cmd, res)
case len(argDocIDs) > 1:
docIDs := make([]client.DocID, len(argDocIDs))
for i, v := range argDocIDs {
docID, err := client.NewDocIDFromString(v)
if err != nil {
return err
}
docIDs[i] = docID
}
res, err := col.DeleteWithDocIDs(cmd.Context(), docIDs)
if err != nil {
return err
}
return writeJSON(cmd, res)
_, err = col.Delete(cmd.Context(), docID)
return err
case filter != "":
res, err := col.DeleteWithFilter(cmd.Context(), filter)
if err != nil {
Expand All @@ -75,7 +58,7 @@ Example: delete by filter:
}
},
}
cmd.Flags().StringSliceVar(&argDocIDs, "docID", nil, "Document ID")
cmd.Flags().StringVar(&argDocID, "docID", "", "Document ID")
cmd.Flags().StringVar(&filter, "filter", "", "Document filter")
return cmd
}
40 changes: 8 additions & 32 deletions cli/collection_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

func MakeCollectionUpdateCommand() *cobra.Command {
var argDocIDs []string
var argDocID string
var filter string
var updater string
var cmd = &cobra.Command{
Expand All @@ -32,13 +32,13 @@ Example: update by filter:
defradb client collection update --name User \
--filter '{ "_gte": { "points": 100 } }' --updater '{ "verified": true }'

Example: update by docIDs:
Example: update by docID:
defradb client collection update --name User \
--docID bae-123,bae-456 --updater '{ "verified": true }'
--docID bae-123 --updater '{ "verified": true }'

Example: update private docIDs, with identity:
Example: update private docID, with identity:
defradb client collection update -i cosmos1f2djr7dl9vhrk3twt3xwqp09nhtzec9mdkf70j --name User \
--docID bae-123,bae-456 --updater '{ "verified": true }'
--docID bae-123 --updater '{ "verified": true }'
`,
Args: cobra.RangeArgs(0, 1),
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -48,38 +48,14 @@ Example: update private docIDs, with identity:
}

switch {
case len(argDocIDs) == 1 && updater != "":
docID, err := client.NewDocIDFromString(argDocIDs[0])
if err != nil {
return err
}
res, err := col.UpdateWithDocID(cmd.Context(), docID, updater)
if err != nil {
return err
}
return writeJSON(cmd, res)
case len(argDocIDs) > 1 && updater != "":
docIDs := make([]client.DocID, len(argDocIDs))
for i, v := range argDocIDs {
docID, err := client.NewDocIDFromString(v)
if err != nil {
return err
}
docIDs[i] = docID
}
res, err := col.UpdateWithDocIDs(cmd.Context(), docIDs, updater)
if err != nil {
return err
}
return writeJSON(cmd, res)
case filter != "" && updater != "":
res, err := col.UpdateWithFilter(cmd.Context(), filter, updater)
if err != nil {
return err
}
return writeJSON(cmd, res)
case len(argDocIDs) == 1 && len(args) == 1:
docID, err := client.NewDocIDFromString(argDocIDs[0])
case argDocID != "" && len(args) == 1:
docID, err := client.NewDocIDFromString(argDocID)
if err != nil {
return err
}
Expand All @@ -96,7 +72,7 @@ Example: update private docIDs, with identity:
}
},
}
cmd.Flags().StringSliceVar(&argDocIDs, "docID", nil, "Document ID")
cmd.Flags().StringVar(&argDocID, "docID", "", "Document ID")
cmd.Flags().StringVar(&filter, "filter", "", "Document filter")
cmd.Flags().StringVar(&updater, "updater", "", "Document updater")
return cmd
Expand Down
75 changes: 0 additions & 75 deletions client/collection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,6 @@ type Collection interface {
// Will return true if a matching document exists, otherwise will return false.
Exists(ctx context.Context, docID DocID) (bool, error)

// UpdateWith updates a target document using the given updater type.
//
// Target can be a Filter statement, a single DocID, a single document,
// an array of DocIDs, or an array of documents.
// It is recommended to use the respective typed versions of Update
// (e.g. UpdateWithFilter or UpdateWithDocID) over this function if you can.
//
// Returns an ErrInvalidUpdateTarget error if the target type is not supported.
// Returns an ErrInvalidUpdater error if the updater type is not supported.
UpdateWith(
ctx context.Context,
target any,
updater string,
) (*UpdateResult, error)

// UpdateWithFilter updates using a filter to target documents for update.
Copy link
Contributor

@AndrewSisley AndrewSisley Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

suggestion: Just Update and Delete would be better names IMO, especially given that the filtering is optional - they might be calling UpdateWithFilter with no filter.

EDIT: Update and Delete were kept/already exist, ignore this :) Unless you want to unify them like GetCollections and GetSchemas do.

//
// The provided updater must be a string Patch, string Merge Patch, a parsed Patch, or parsed Merge Patch
Expand All @@ -103,44 +88,6 @@ type Collection interface {
updater string,
) (*UpdateResult, error)

// UpdateWithDocID updates using a DocID to target a single document for update.
//
// The provided updater must be a string Patch, string Merge Patch, a parsed Patch, or parsed Merge Patch
// else an ErrInvalidUpdater will be returned.
//
// Returns an ErrDocumentNotFound if a document matching the given DocID is not found.
UpdateWithDocID(
ctx context.Context,
docID DocID,
updater string,
) (*UpdateResult, error)

// UpdateWithDocIDs updates documents matching the given DocIDs.
//
// The provided updater must be a string Patch, string Merge Patch, a parsed Patch, or parsed Merge Patch
// else an ErrInvalidUpdater will be returned.
//
// Returns an ErrDocumentNotFound if a document is not found for any given DocID.
UpdateWithDocIDs(
ctx context.Context,
docIDs []DocID,
updater string,
) (*UpdateResult, error)

// DeleteWith deletes a target document.
//
// Target can be a Filter statement, a single DocID, a single document, an array of DocIDs,
// or an array of documents. It is recommended to use the respective typed versions of Delete
// (e.g. DeleteWithFilter or DeleteWithDocID) over this function if you can.
// This operation will soft-delete documents related to the given DocID and update the composite block
// with a status of `Deleted`.
//
// Returns an ErrInvalidDeleteTarget if the target type is not supported.
DeleteWith(
ctx context.Context,
target any,
) (*DeleteResult, error)

// DeleteWithFilter deletes documents matching the given filter.
//
// This operation will soft-delete documents related to the given filter and update the composite block
Expand All @@ -150,28 +97,6 @@ type Collection interface {
filter any,
) (*DeleteResult, error)

// DeleteWithDocID deletes using a DocID to target a single document for delete.
//
// This operation will soft-delete documents related to the given DocID and update the composite block
// with a status of `Deleted`.
//
// Returns an ErrDocumentNotFound if a document matching the given DocID is not found.
DeleteWithDocID(
ctx context.Context,
docID DocID,
) (*DeleteResult, error)

// DeleteWithDocIDs deletes documents matching the given DocIDs.
//
// This operation will soft-delete documents related to the given DocIDs and update the composite block
// with a status of `Deleted`.
//
// Returns an ErrDocumentNotFound if a document is not found for any given DocID.
DeleteWithDocIDs(
ctx context.Context,
docIDs []DocID,
) (*DeleteResult, error)

// Get returns the document with the given DocID.
//
// Returns an ErrDocumentNotFound if a document matching the given DocID is not found.
Expand Down
3 changes: 0 additions & 3 deletions client/document.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,6 @@ func (doc *Document) setCBOR(t CType, field string, val NormalValue) error {

func (doc *Document) setAndParseObjectType(value map[string]any) error {
for k, v := range value {
if v == nil {
continue
}
err := doc.Set(k, v)
if err != nil {
return err
Expand Down
111 changes: 0 additions & 111 deletions db/collection_delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,74 +15,11 @@ import (

"github.com/sourcenetwork/defradb/acp"
"github.com/sourcenetwork/defradb/client"
"github.com/sourcenetwork/defradb/client/request"
"github.com/sourcenetwork/defradb/core"
"github.com/sourcenetwork/defradb/events"
"github.com/sourcenetwork/defradb/merkle/clock"
)

// DeleteWith deletes a target document.
//
// Target can be a Filter statement, a single DocID, a single document,
// an array of DocIDs, or an array of documents.
//
// If you want more type safety, use the respective typed versions of Delete.
// Eg: DeleteWithFilter or DeleteWithDocID
func (c *collection) DeleteWith(
ctx context.Context,
target any,
) (*client.DeleteResult, error) {
switch t := target.(type) {
case string, map[string]any, *request.Filter:
return c.DeleteWithFilter(ctx, t)
case client.DocID:
return c.DeleteWithDocID(ctx, t)
case []client.DocID:
return c.DeleteWithDocIDs(ctx, t)
default:
return nil, client.ErrInvalidDeleteTarget
}
}

// DeleteWithDocID deletes using a DocID to target a single document for delete.
func (c *collection) DeleteWithDocID(
ctx context.Context,
docID client.DocID,
) (*client.DeleteResult, error) {
ctx, txn, err := ensureContextTxn(ctx, c.db, false)
if err != nil {
return nil, err
}
defer txn.Discard(ctx)

dsKey := c.getPrimaryKeyFromDocID(docID)
res, err := c.deleteWithKey(ctx, dsKey)
if err != nil {
return nil, err
}

return res, txn.Commit(ctx)
}

// DeleteWithDocIDs is the same as DeleteWithDocID but accepts multiple DocIDs as a slice.
func (c *collection) DeleteWithDocIDs(
ctx context.Context,
docIDs []client.DocID,
) (*client.DeleteResult, error) {
ctx, txn, err := ensureContextTxn(ctx, c.db, false)
if err != nil {
return nil, err
}
defer txn.Discard(ctx)

res, err := c.deleteWithIDs(ctx, docIDs, client.Deleted)
if err != nil {
return nil, err
}

return res, txn.Commit(ctx)
}

// DeleteWithFilter deletes using a filter to target documents for delete.
func (c *collection) DeleteWithFilter(
ctx context.Context,
Expand All @@ -102,54 +39,6 @@ func (c *collection) DeleteWithFilter(
return res, txn.Commit(ctx)
}

func (c *collection) deleteWithKey(
ctx context.Context,
key core.PrimaryDataStoreKey,
) (*client.DeleteResult, error) {
// Check the key we have been given to delete with actually has a corresponding
// document (i.e. document actually exists in the collection).
err := c.applyDelete(ctx, key)
if err != nil {
return nil, err
}

// Upon successfull deletion, record a summary.
results := &client.DeleteResult{
Count: 1,
DocIDs: []string{key.DocID},
}

return results, nil
}

func (c *collection) deleteWithIDs(
ctx context.Context,
docIDs []client.DocID,
_ client.DocumentStatus,
) (*client.DeleteResult, error) {
results := &client.DeleteResult{
DocIDs: make([]string, 0),
}

for _, docID := range docIDs {
primaryKey := c.getPrimaryKeyFromDocID(docID)

// Apply the function that will perform the full deletion of this document.
err := c.applyDelete(ctx, primaryKey)
if err != nil {
return nil, err
}

// Add this deleted docID to our list.
results.DocIDs = append(results.DocIDs, docID.String())
}

// Upon successfull deletion, record a summary of how many we deleted.
results.Count = int64(len(results.DocIDs))

return results, nil
}

func (c *collection) deleteWithFilter(
ctx context.Context,
filter any,
Expand Down
Loading
Loading