add directory soft delete

Implements soft deletions for directories.
Deleting a parent directory also deletes all child directories.

Signed-off-by: Mike Mason <mimason@equinix.com>

app/v1/callback/callback.go

@@ -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)
 }

app/v1/controllerimpl.go

@@ -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
 		}

app/v1/sql/storage.go

@@ -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.

client/v1/impl.go

@@ -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 {

client/v1/interface.go

@@ -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

internal/httpsrv/treemanager/treemanager.go

@@ -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))
@@ -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")

internal/httpsrv/treemanager/treemanager_test.go

@@ -259,6 +259,48 @@ 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 TestDeleteDirectoryNotFound(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.DeleteDirectoryNotFoundTest(t, cli)
+// }
+
 func TestErrorDoesntLeakInfo(t *testing.T) {
 	t.Parallel()
 

storage/crdb/driver/driver.go

@@ -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 {
@@ -106,21 +106,63 @@ 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 parent_id = $1
+
+		UNION
+
+		SELECT d.id, d.parent_id FROM directories d
+		INNER JOIN get_children gc ON d.parent_id = gc.id
+	)
+	UPDATE directories
+	SET deleted_at = NOW()
+	WHERE
+		deleted_at IS NULL
+		AND (
+			id = $1
+			OR 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
@@ -137,12 +179,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`)
 
@@ -185,13 +228,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)
@@ -223,12 +266,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`)
 
@@ -247,7 +291,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