Skip to content
This repository has been archived by the owner on Apr 12, 2023. It is now read-only.

Add directory soft delete #52

Merged
merged 3 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
21 changes: 21 additions & 0 deletions api/v1/typeshelpers.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v1

import (
"encoding/json"
"errors"

"github.com/google/uuid"
Expand Down Expand Up @@ -33,4 +34,24 @@ func (did DirectoryID) String() string {
return uuid.UUID(did).String()
}

func (did DirectoryID) MarshalJSON() ([]byte, error) {
return json.Marshal(did.String())
}

func (did *DirectoryID) UnmarshalJSON(b []byte) error {
var strUUID string
if err := json.Unmarshal(b, &strUUID); err != nil {
return err
}

u, err := uuid.Parse(strUUID)
if err != nil {
return err
}

*did = DirectoryID(u)

return nil
}

type DirectoryMetadata map[string]string
6 changes: 4 additions & 2 deletions app/v1/callback/callback.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ func (s *AppStorageWithCallback) CreateDirectory(ctx context.Context, d *apiv1.D
return d, nil
}

func (s *AppStorageWithCallback) DeleteDirectory(ctx context.Context, id apiv1.DirectoryID) error {
func (s *AppStorageWithCallback) DeleteDirectory(
ctx context.Context, id apiv1.DirectoryID,
) ([]*apiv1.Directory, error) {
if err := s.cfg.DeleteDirectory(ctx, id); err != nil {
return err
return nil, err
}
return s.impl.DeleteDirectory(ctx, id)
}
Expand Down
2 changes: 1 addition & 1 deletion app/v1/controllerimpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (c *controller) persistIfUpToDate(ctx context.Context, dir apiv1.DirectoryI
func (c *controller) persistDirectory(ctx context.Context, d *apiv1.Directory) error {
// handle deletion
if d.IsDeleted() {
err := c.store.DeleteDirectory(ctx, d.Id)
_, err := c.store.DeleteDirectory(ctx, d.Id)
if err != nil {
return err
}
Expand Down
36 changes: 27 additions & 9 deletions app/v1/sql/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,23 +77,41 @@ func (s *sqlstorage) CreateDirectory(ctx context.Context, d *apiv1.Directory) (*
return d, nil
}

func (s *sqlstorage) DeleteDirectory(ctx context.Context, id apiv1.DirectoryID) error {
func (s *sqlstorage) DeleteDirectory(ctx context.Context, id apiv1.DirectoryID) ([]*apiv1.Directory, error) {
// soft delete directory
res, err := s.db.ExecContext(ctx, "UPDATE tracked_directories SET deleted_at = NOW() WHERE id = $1", id)
var affected []*apiv1.Directory

deleteQuery := `
UPDATE tracked_directories
SET deleted_at = NOW()
WHERE
deleted_at IS NULL
AND id = $1
RETURNING id, deleted_at`

rows, err := s.db.QueryContext(ctx, deleteQuery, id)
if err != nil {
return fmt.Errorf("error deleting directory: %w", err)
return nil, fmt.Errorf("error deleting directory: %w", err)
}

rows, err := res.RowsAffected()
if err != nil {
return fmt.Errorf("error getting rows affected: %w", err)
defer rows.Close()

for rows.Next() {
var d apiv1.Directory

err := rows.Scan(&d.Id, &d.DeletedAt)
if err != nil {
return nil, fmt.Errorf("error scanning directory: %w", err)
}

affected = append(affected, &d)
}

if rows != 1 {
return fmt.Errorf("expected 1 row affected, got %d", rows)
if len(affected) != 1 {
return affected, fmt.Errorf("expected 1 row affected, got %d", len(affected))
}

return nil
return affected, nil
}

// compareDeletedAt compares the observed deleted at time with the expected deleted at time.
Expand Down
26 changes: 26 additions & 0 deletions client/v1/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,32 @@ func (c *httpClient) ListRoots(ctx context.Context) (*v1.DirectoryList, error) {
return &dirList, nil
}

func (c *httpClient) DeleteDirectory(ctx context.Context, id v1.DirectoryID) (*v1.DirectoryList, error) {
path, err := url.JoinPath("/api/v1/directories", id.String())
if err != nil {
return nil, fmt.Errorf("error getting directory: %w", err)
}

resp, err := c.DoRaw(ctx, http.MethodDelete, path, nil)
if err != nil {
return nil, fmt.Errorf("error getting directory: %w", err)
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("error getting directory: %s", resp.Status)
}

var dirList v1.DirectoryList
err = dirList.Parse(resp.Body)

if err != nil {
return nil, fmt.Errorf("error parsing response: %w", err)
}

return &dirList, nil
}

func (c *httpClient) GetDirectory(ctx context.Context, id v1.DirectoryID) (*v1.DirectoryFetch, error) {
path, err := url.JoinPath("/api/v1/directories", id.String())
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions client/v1/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type ReadOnlyClient interface {
type Client interface {
ReadOnlyClient
CreateDirectory(c context.Context, r *v1.CreateDirectoryRequest, parent v1.DirectoryID) (*v1.DirectoryFetch, error)
DeleteDirectory(c context.Context, id v1.DirectoryID) (*v1.DirectoryList, error)
}

// RootClient allows for instantiating a client
Expand Down
40 changes: 40 additions & 0 deletions internal/httpsrv/treemanager/treemanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func newHandler(logger *zap.Logger, s *common.Server) *gin.Engine {

r.GET("/api/v1/directories/:id", getDirectory(s))
r.POST("/api/v1/directories/:id", createDirectory(s))
r.DELETE("/api/v1/directories/:id", deleteDirectory(s))

r.GET("/api/v1/directories/:id/children", listChildren(s))
r.GET("/api/v1/directories/:id/parents", listParents(s))
Expand Down Expand Up @@ -183,6 +184,45 @@ func createDirectory(s *common.Server) gin.HandlerFunc {
}
}

func deleteDirectory(s *common.Server) gin.HandlerFunc {
return func(c *gin.Context) {
idstr := c.Param("id")

id, err := v1.ParseDirectoryID(idstr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{
"error": "invalid id",
})
return
}

affected, err := s.T.DeleteDirectory(c, id)
if errors.Is(err, storage.ErrDirectoryNotFound) {
c.JSON(http.StatusNotFound, gin.H{
"error": "directory not found",
})
return
} else if err != nil {
s.L.Error("error deleting directory", zap.Error(err))
c.JSON(http.StatusInternalServerError, gin.H{
"error": "internal server error",
})
return
}

affectedIDs := make([]v1.DirectoryID, len(affected))

for i, d := range affected {
affectedIDs[i] = d.Id
}

c.JSON(http.StatusOK, &v1.DirectoryList{
Directories: affectedIDs,
Version: v1.APIVersion,
})
}
}

func listChildren(s *common.Server) gin.HandlerFunc {
return func(c *gin.Context) {
idstr := c.Param("id")
Expand Down
21 changes: 21 additions & 0 deletions internal/httpsrv/treemanager/treemanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,27 @@ func TestDirectoryNotFound(t *testing.T) {
integration.DirectoryNotFoundTest(t, cli)
}

func TestDeleteDirectory(t *testing.T) {
t.Parallel()

skt := testutils.NewUnixsocketPath(t)
srv := newTestServer(t, skt, nil)

defer func() {
err := srv.Shutdown()
assert.NoError(t, err, "error shutting down server")
}()

go testutils.RunTestServer(t, srv)

srvAddr := getStubServerAddress(t, skt)
cli := testutils.NewTestClient(t, skt, srvAddr)

testutils.WaitForServer(t, cli)

integration.DeleteDirectoryTest(t, cli)
}

func TestErrorDoesntLeakInfo(t *testing.T) {
t.Parallel()

Expand Down
71 changes: 59 additions & 12 deletions storage/crdb/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (t *Driver) CreateRoot(ctx context.Context, d *v1.Directory) (*v1.Directory
func (t *Driver) ListRoots(ctx context.Context) ([]v1.DirectoryID, error) {
var roots []v1.DirectoryID

q := t.formatQuery("SELECT id FROM directories %[1]s WHERE parent_id IS NULL")
q := t.formatQuery("SELECT id FROM directories %[1]s WHERE parent_id IS NULL AND deleted_at IS NULL")

rows, err := t.db.QueryContext(ctx, q)
if err != nil {
Expand Down Expand Up @@ -106,21 +106,61 @@ func (t *Driver) CreateDirectory(ctx context.Context, d *v1.Directory) (*v1.Dire
return d, nil
}

func (t *Driver) DeleteDirectory(ctx context.Context, id v1.DirectoryID) error {
// TODO(jaosorior): Implement soft-delete.
return nil
func (t *Driver) DeleteDirectory(ctx context.Context, id v1.DirectoryID) ([]*v1.Directory, error) {
var affected []*v1.Directory

q := t.formatQuery(`WITH RECURSIVE get_children AS (
SELECT id, parent_id FROM directories
WHERE id = $1 AND deleted_at IS NULL AND parent_id IS NOT NULL

UNION

SELECT d.id, d.parent_id FROM directories d
INNER JOIN get_children gc ON d.parent_id = gc.id
WHERE d.deleted_at IS NULL
)
UPDATE directories
SET deleted_at = NOW()
WHERE
deleted_at IS NULL
AND id IN (SELECT id FROM get_children)
RETURNING id, name, metadata, created_at, updated_at, deleted_at, parent_id %[1]s`)

rows, err := t.db.QueryContext(ctx, q, id)
if err != nil {
return nil, fmt.Errorf("error querying directory: %w", err)
}
defer rows.Close()

for rows.Next() {
var d v1.Directory

err := rows.Scan(&d.Id, &d.Name, &d.Metadata, &d.CreatedAt, &d.UpdatedAt, &d.DeletedAt, &d.Parent)
if err != nil {
return nil, fmt.Errorf("error scanning directory: %w", err)
}

affected = append(affected, &d)
}

// If no rows were affected, the directory wasn't found.
if len(affected) == 0 {
return nil, storage.ErrDirectoryNotFound
}

return affected, nil
}

// GetDirectoryByID returns a directory by its ID.
// Note that this call does not give out parent information.
func (t *Driver) GetDirectory(ctx context.Context, id v1.DirectoryID) (*v1.Directory, error) {
var d v1.Directory

q := t.formatQuery(`SELECT id, name, metadata, created_at, updated_at, parent_id FROM directories %[1]s
WHERE id = $1`)
q := t.formatQuery(`SELECT id, name, metadata, created_at, updated_at, deleted_at, parent_id FROM directories %[1]s
WHERE id = $1 AND deleted_at IS NULL`)

err := t.db.QueryRowContext(ctx, q,
id).Scan(&d.Id, &d.Name, &d.Metadata, &d.CreatedAt, &d.UpdatedAt, &d.Parent)
id).Scan(&d.Id, &d.Name, &d.Metadata, &d.CreatedAt, &d.UpdatedAt, &d.DeletedAt, &d.Parent)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, storage.ErrDirectoryNotFound
Expand All @@ -137,12 +177,13 @@ func (t *Driver) GetParents(ctx context.Context, child v1.DirectoryID) ([]v1.Dir
// TODO(jaosorior): What's more efficient? A single recursive query or multiple queries?
// Should we instead recurse in-code and do multiple queries?
q := t.formatQuery(`WITH RECURSIVE get_parents AS (
SELECT id, parent_id FROM directories WHERE id = $1
SELECT id, parent_id FROM directories WHERE id = $1 AND deleted_at IS NULL

UNION

SELECT d.id, d.parent_id FROM directories d
INNER JOIN get_parents gp ON d.id = gp.parent_id
WHERE d.deleted_at IS NULL
)
SELECT id FROM get_parents %[1]s`)

Expand Down Expand Up @@ -185,13 +226,13 @@ func (t *Driver) GetParentsUntilAncestor(
// Should we instead recurse in-code and do multiple queries?
q := t.formatQuery(`WITH RECURSIVE get_parents AS (
SELECT id, parent_id FROM directories
WHERE id = $1
WHERE id = $1 AND deleted_at IS NULL

UNION

SELECT d.id, d.parent_id FROM directories d
INNER JOIN get_parents gp ON d.id = gp.parent_id
WHERE gp.id != $2
WHERE gp.id != $2 AND d.deleted_at IS NULL
) SELECT id FROM get_parents %[1]s`)

rows, err := t.db.QueryContext(ctx, q, child, ancestor)
Expand Down Expand Up @@ -223,12 +264,13 @@ func (t *Driver) GetChildren(ctx context.Context, parent v1.DirectoryID) ([]v1.D

q := t.formatQuery(`WITH RECURSIVE get_children AS (
SELECT id, parent_id FROM directories
WHERE parent_id = $1
WHERE id = $1 AND deleted_at IS NULL

UNION

SELECT d.id, d.parent_id FROM directories d
INNER JOIN get_children gc ON d.parent_id = gc.id
WHERE d.deleted_at IS NULL
)
SELECT id FROM get_children %[1]s`)

Expand All @@ -247,7 +289,12 @@ SELECT id FROM get_children %[1]s`)
children = append(children, did)
}

return children, nil
if len(children) == 0 {
return nil, storage.ErrDirectoryNotFound
}

// skip the first element, which is the parent
return children[1:], nil
}

// Note that this assumes that queries only take one
Expand Down
Loading