From 1f9b339170a79bda7276f16dc0f7e63d645a1662 Mon Sep 17 00:00:00 2001
From: Mark Phelps <209477+markphelps@users.noreply.github.com>
Date: Tue, 21 Mar 2023 10:30:58 -0400
Subject: [PATCH] feat: check for protected or flags existing when deleting a
 namespace (#1422)

* feat: add namespaces server impl

* chore: spacing

* chore: rm ability to set protected

* feat: check for protected or flags existing when deleting a namespace

* chore: fix test

* chore: add test for non-existing delete
---
 internal/server/namespace.go      | 25 +++++++++
 internal/server/namespace_test.go | 90 +++++++++++++++++++++++++++++--
 2 files changed, 112 insertions(+), 3 deletions(-)

diff --git a/internal/server/namespace.go b/internal/server/namespace.go
index 1c68822c3f..c34b448440 100644
--- a/internal/server/namespace.go
+++ b/internal/server/namespace.go
@@ -81,8 +81,33 @@ func (s *Server) UpdateNamespace(ctx context.Context, r *flipt.UpdateNamespaceRe
 // DeleteNamespace deletes a namespace
 func (s *Server) DeleteNamespace(ctx context.Context, r *flipt.DeleteNamespaceRequest) (*empty.Empty, error) {
 	s.logger.Debug("delete namespace", zap.Stringer("request", r))
+	namespace, err := s.store.GetNamespace(ctx, r.Key)
+	if err != nil {
+		return nil, err
+	}
+
+	// if namespace is not found then nothing to do
+	if namespace == nil {
+		return &empty.Empty{}, nil
+	}
+
+	if namespace.Protected {
+		return nil, errors.ErrInvalidf("namespace %q is protected", r.Key)
+	}
+
+	// if any flags exist under the namespace then it cannot be deleted
+	count, err := s.store.CountFlags(ctx, r.Key)
+	if err != nil {
+		return nil, err
+	}
+
+	if count > 0 {
+		return nil, errors.ErrInvalidf("namespace %q cannot be deleted; flags must be deleted first", r.Key)
+	}
+
 	if err := s.store.DeleteNamespace(ctx, r); err != nil {
 		return nil, err
 	}
+
 	return &empty.Empty{}, nil
 }
diff --git a/internal/server/namespace_test.go b/internal/server/namespace_test.go
index 0f130817d6..7be8c0d5ff 100644
--- a/internal/server/namespace_test.go
+++ b/internal/server/namespace_test.go
@@ -153,7 +153,7 @@ func TestCreateNamespace(t *testing.T) {
 			store:  store,
 		}
 		req = &flipt.CreateNamespaceRequest{
-			Key:         "key",
+			Key:         "foo",
 			Name:        "name",
 			Description: "desc",
 		}
@@ -180,7 +180,7 @@ func TestUpdateNamespace(t *testing.T) {
 			store:  store,
 		}
 		req = &flipt.UpdateNamespaceRequest{
-			Key:         "key",
+			Key:         "foo",
 			Name:        "name",
 			Description: "desc",
 		}
@@ -207,10 +207,16 @@ func TestDeleteNamespace(t *testing.T) {
 			store:  store,
 		}
 		req = &flipt.DeleteNamespaceRequest{
-			Key: "key",
+			Key: "foo",
 		}
 	)
 
+	store.On("GetNamespace", mock.Anything, "foo").Return(&flipt.Namespace{
+		Key: req.Key,
+	}, nil)
+
+	store.On("CountFlags", mock.Anything, "foo").Return(uint64(0), nil)
+
 	store.On("DeleteNamespace", mock.Anything, req).Return(nil)
 
 	got, err := s.DeleteNamespace(context.TODO(), req)
@@ -218,3 +224,81 @@ func TestDeleteNamespace(t *testing.T) {
 
 	assert.NotNil(t, got)
 }
+
+func TestDeleteNamespace_NonExistent(t *testing.T) {
+	var (
+		store  = &storeMock{}
+		logger = zaptest.NewLogger(t)
+		s      = &Server{
+			logger: logger,
+			store:  store,
+		}
+		req = &flipt.DeleteNamespaceRequest{
+			Key: "foo",
+		}
+	)
+
+	var ns *flipt.Namespace
+	store.On("GetNamespace", mock.Anything, "foo").Return(ns, nil) // mock library does not like nil
+
+	store.AssertNotCalled(t, "CountFlags")
+	store.AssertNotCalled(t, "DeleteNamespace")
+
+	got, err := s.DeleteNamespace(context.TODO(), req)
+	require.NoError(t, err)
+
+	assert.NotNil(t, got)
+}
+
+func TestDeleteNamespace_Protected(t *testing.T) {
+	var (
+		store  = &storeMock{}
+		logger = zaptest.NewLogger(t)
+		s      = &Server{
+			logger: logger,
+			store:  store,
+		}
+		req = &flipt.DeleteNamespaceRequest{
+			Key: "foo",
+		}
+	)
+
+	store.On("GetNamespace", mock.Anything, "foo").Return(&flipt.Namespace{
+		Key:       req.Key,
+		Protected: true,
+	}, nil)
+
+	store.On("CountFlags", mock.Anything, "foo").Return(uint64(0), nil)
+
+	store.AssertNotCalled(t, "DeleteNamespace")
+
+	got, err := s.DeleteNamespace(context.TODO(), req)
+	assert.EqualError(t, err, "namespace \"foo\" is protected")
+	assert.Nil(t, got)
+}
+
+func TestDeleteNamespace_HasFlags(t *testing.T) {
+	var (
+		store  = &storeMock{}
+		logger = zaptest.NewLogger(t)
+		s      = &Server{
+			logger: logger,
+			store:  store,
+		}
+		req = &flipt.DeleteNamespaceRequest{
+			Key: "foo",
+		}
+	)
+
+	store.On("GetNamespace", mock.Anything, "foo").Return(&flipt.Namespace{
+		Key: req.Key,
+	}, nil)
+
+	store.On("CountFlags", mock.Anything, "foo").Return(uint64(1), nil)
+
+	store.AssertNotCalled(t, "DeleteNamespace")
+
+	got, err := s.DeleteNamespace(context.TODO(), req)
+	assert.EqualError(t, err, "namespace \"foo\" cannot be deleted; flags must be deleted first")
+	assert.Nil(t, got)
+}