delete: add endpoint for deleting namespaces (#2244)

* delete: child jobs are now properly deleted Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com> * delete: add delete namespace endpoint Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com> Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
MarquezProject · Nov 17, 2022 · 0f857c5 · 0f857c5
1 parent 1d8334b
commit 0f857c5
Show file tree

Hide file tree

Showing 31 changed files with 391 additions and 315 deletions.
diff --git a/.github/workflows/headerchecker.yml b/.github/workflows/headerchecker.yml
@@ -31,7 +31,7 @@ jobs:
       - name: Check for headers
         run: |
           ok=1
-          readarray -t files <<<"$(jq -r '.[]' <<<'${{ steps.files.outputs.all }}')"
+          readarray -t files <<<"$(jq -r '.[]' <<<'${{ steps.files.outputs.added_modified }}')"
           for file in ${files[@]}; do 
             if [[ ($file == *".java") ]]; then
               if ! grep -q Copyright "$file"; then

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,15 @@
 
 ## [Unreleased](https://github.com/MarquezProject/marquez/compare/0.27.0...HEAD)
 
+### Added
+* Add possibility to soft-delete namespaces [`#2244`](https://github.com/MarquezProject/marquez/pull/2244) [@mobuchowski](https://github.com/mobuchowski)  
+  *Adds the ability to "hide" inactive namespaces. The namespaces are being undeleted when relevant OL event is received.*
+
+### Fixed
+* Fix bug where job isn't properly deleted [`#2244`](https://github.com/MarquezProject/marquez/pull/2244) [@mobuchowski](https://github.com/mobuchowski)  
+  *It wasn't possible to delete jobs created from events that had `ParentRunFacet`. Now it's possible.*
+
+
 ## [0.27.0](https://github.com/MarquezProject/marquez/compare/0.26.0...0.27.0) - 2022-10-24
 
 ### Added

diff --git a/api/src/main/java/marquez/api/JobResource.java b/api/src/main/java/marquez/api/JobResource.java
@@ -176,7 +176,8 @@ public Response delete(
             .findJobByName(namespaceName.getValue(), jobName.getValue())
             .orElseThrow(() -> new JobNotFoundException(jobName));
 
-    jobService.delete(namespaceName.getValue(), jobName.getValue());
+    // Should be simple name from `jobs_fqn`.
+    jobService.delete(namespaceName.getValue(), job.getSimpleName());
     return Response.ok(job).build();
   }
 

diff --git a/api/src/main/java/marquez/api/NamespaceResource.java b/api/src/main/java/marquez/api/NamespaceResource.java
@@ -15,6 +15,7 @@
 import javax.validation.Valid;
 import javax.validation.constraints.Min;
 import javax.ws.rs.Consumes;
+import javax.ws.rs.DELETE;
 import javax.ws.rs.DefaultValue;
 import javax.ws.rs.GET;
 import javax.ws.rs.PUT;
@@ -77,6 +78,23 @@ public Response list(
     return Response.ok(new Namespaces(namespaces)).build();
   }
 
+  @Timed
+  @ResponseMetered
+  @ExceptionMetered
+  @DELETE
+  @Path("/namespaces/{namespace}")
+  @Produces(APPLICATION_JSON)
+  public Response delete(@PathParam("namespace") NamespaceName name) {
+    final Namespace namespace =
+        namespaceService
+            .findBy(name.getValue())
+            .orElseThrow(() -> new NamespaceNotFoundException(name));
+    datasetService.deleteByNamespaceName(namespace.getName().getValue());
+    jobService.deleteByNamespaceName(namespace.getName().getValue());
+    namespaceService.delete(namespace.getName().getValue());
+    return Response.ok(namespace).build();
+  }
+
   @Value
   static class Namespaces {
     @NonNull

diff --git a/api/src/main/java/marquez/db/Columns.java b/api/src/main/java/marquez/db/Columns.java
@@ -55,6 +55,7 @@ private Columns() {}
   public static final String DATASET_NAME = "dataset_name";
   public static final String FACETS = "facets";
   public static final String TAGS = "tags";
+  public static final String IS_HIDDEN = "is_hidden";
 
   /* NAMESPACE ROW COLUMNS */
   public static final String CURRENT_OWNER_NAME = "current_owner_name";
@@ -197,6 +198,14 @@ public static boolean booleanOrDefault(
     return results.getBoolean(column);
   }
 
+  public static boolean booleanOrThrow(final ResultSet results, final String column)
+      throws SQLException {
+    if (results.getObject(column) == null) {
+      throw new IllegalArgumentException();
+    }
+    return results.getBoolean(column);
+  }
+
   public static int intOrThrow(final ResultSet results, final String column) throws SQLException {
     if (results.getObject(column) == null) {
       throw new IllegalArgumentException();

diff --git a/api/src/main/java/marquez/db/DatasetDao.java b/api/src/main/java/marquez/db/DatasetDao.java
@@ -292,15 +292,24 @@ DatasetRow upsert(
       String name,
       String physicalName);
 
+  @SqlUpdate(
+      """
+    UPDATE datasets d
+    SET is_hidden = true
+    FROM namespaces n
+    WHERE n.uuid=d.namespace_uuid
+    AND n.name=:namespaceName
+  """)
+  void deleteByNamespaceName(String namespaceName);
+
   @SqlQuery(
       """
-  UPDATE datasets
-  SET is_hidden = true
-  FROM dataset_symlinks, namespaces
-  WHERE dataset_symlinks.dataset_uuid = datasets.uuid
-  AND namespaces.uuid = dataset_symlinks.namespace_uuid
-  AND namespaces.name=:namespaceName AND dataset_symlinks.name=:name
-  RETURNING *
+    UPDATE datasets d
+    SET is_hidden = true
+    FROM namespaces n
+    WHERE n.uuid = d.namespace_uuid
+    AND n.name=:namespaceName AND d.name=:name
+    RETURNING *
   """)
   Optional<DatasetRow> delete(String namespaceName, String name);
 

diff --git a/api/src/main/java/marquez/db/JobDao.java b/api/src/main/java/marquez/db/JobDao.java
@@ -88,6 +88,16 @@ SELECT run_uuid, JSON_AGG(e.facets) AS facets
   """)
   void delete(String namespaceName, String name);
 
+  @SqlUpdate(
+      """
+  UPDATE jobs
+  SET is_hidden = true
+  FROM namespaces n
+  WHERE jobs.namespace_uuid = n.uuid
+  AND n.name = :namespaceName
+  """)
+  void deleteByNamespaceName(String namespaceName);
+
   default Optional<Job> findWithRun(String namespaceName, String jobName) {
     Optional<Job> job = findJobByName(namespaceName, jobName);
     job.ifPresent(

diff --git a/api/src/main/java/marquez/db/NamespaceDao.java b/api/src/main/java/marquez/db/NamespaceDao.java
@@ -78,10 +78,20 @@ default Namespace upsertNamespaceMeta(
   @SqlQuery("SELECT * FROM namespaces ORDER BY name LIMIT :limit OFFSET :offset")
   List<Namespace> findAll(int limit, int offset);
 
+  @SqlQuery("UPDATE namespaces SET is_hidden=false WHERE name = :name RETURNING *")
+  NamespaceRow undelete(String name);
+
+  @SqlUpdate("UPDATE namespaces SET is_hidden=true WHERE name = :name")
+  void delete(String name);
+
   default NamespaceRow upsertNamespaceRow(
       UUID uuid, Instant now, String name, String currentOwnerName) {
     doUpsertNamespaceRow(uuid, now, name, currentOwnerName);
-    return findNamespaceByName(name).orElseThrow();
+    NamespaceRow namespaceRow = findNamespaceByName(name).orElseThrow();
+    if (namespaceRow.getIsHidden()) {
+      namespaceRow = undelete(namespaceRow.getName());
+    }
+    return namespaceRow;
   }
 
   /**
@@ -99,40 +109,47 @@ default NamespaceRow upsertNamespaceRow(
    * @param currentOwnerName
    */
   @SqlUpdate(
-      "INSERT INTO namespaces ( "
-          + "uuid, "
-          + "created_at, "
-          + "updated_at, "
-          + "name, "
-          + "current_owner_name "
-          + ") VALUES ("
-          + ":uuid, "
-          + ":now, "
-          + ":now, "
-          + ":name, "
-          + ":currentOwnerName) "
-          + "ON CONFLICT(name) DO NOTHING")
+      """
+    INSERT INTO namespaces (
+      uuid,
+      created_at,
+      updated_at,
+      name,
+      current_owner_name
+      ) VALUES (
+      :uuid,
+      :now,
+      :now,
+      :name,
+      :currentOwnerName)
+      ON CONFLICT(name) DO NOTHING
+  """)
   void doUpsertNamespaceRow(UUID uuid, Instant now, String name, String currentOwnerName);
 
   @SqlQuery(
-      "INSERT INTO namespaces ( "
-          + "uuid, "
-          + "created_at, "
-          + "updated_at, "
-          + "name, "
-          + "current_owner_name, "
-          + "description "
-          + ") VALUES ("
-          + ":uuid, "
-          + ":now, "
-          + ":now, "
-          + ":name, "
-          + ":currentOwnerName, "
-          + ":description "
-          + ") ON CONFLICT(name) DO "
-          + "UPDATE SET "
-          + "updated_at = EXCLUDED.updated_at "
-          + "RETURNING *")
+      """
+    INSERT INTO namespaces (
+      uuid,
+      created_at,
+      updated_at,
+      name,
+      current_owner_name,
+      description,
+      is_hidden
+      ) VALUES (
+      :uuid,
+      :now,
+      :now,
+      :name,
+      :currentOwnerName,
+      :description,
+      false
+      ) ON CONFLICT(name) DO
+      UPDATE SET
+      updated_at = EXCLUDED.updated_at,
+      is_hidden = false
+      RETURNING *
+    """)
   NamespaceRow upsertNamespaceRow(
       UUID uuid, Instant now, String name, String currentOwnerName, String description);
 

diff --git a/api/src/main/java/marquez/db/mappers/NamespaceMapper.java b/api/src/main/java/marquez/db/mappers/NamespaceMapper.java
@@ -5,6 +5,7 @@
 
 package marquez.db.mappers;
 
+import static marquez.db.Columns.booleanOrThrow;
 import static marquez.db.Columns.stringOrNull;
 import static marquez.db.Columns.stringOrThrow;
 import static marquez.db.Columns.timestampOrThrow;
@@ -28,6 +29,7 @@ public Namespace map(@NonNull ResultSet results, @NonNull StatementContext conte
         timestampOrThrow(results, Columns.CREATED_AT),
         timestampOrThrow(results, Columns.UPDATED_AT),
         OwnerName.of(stringOrThrow(results, Columns.CURRENT_OWNER_NAME)),
-        stringOrNull(results, Columns.DESCRIPTION));
+        stringOrNull(results, Columns.DESCRIPTION),
+        booleanOrThrow(results, Columns.IS_HIDDEN));
   }
 }
diff --git a/api/src/main/java/marquez/db/mappers/NamespaceRowMapper.java b/api/src/main/java/marquez/db/mappers/NamespaceRowMapper.java
@@ -5,6 +5,7 @@
 
 package marquez.db.mappers;
 
+import static marquez.db.Columns.booleanOrThrow;
 import static marquez.db.Columns.stringOrNull;
 import static marquez.db.Columns.stringOrThrow;
 import static marquez.db.Columns.timestampOrThrow;
@@ -28,6 +29,7 @@ public NamespaceRow map(@NonNull ResultSet results, @NonNull StatementContext co
         timestampOrThrow(results, Columns.UPDATED_AT),
         stringOrThrow(results, Columns.NAME),
         stringOrNull(results, Columns.DESCRIPTION),
-        stringOrThrow(results, Columns.CURRENT_OWNER_NAME));
+        stringOrThrow(results, Columns.CURRENT_OWNER_NAME),
+        booleanOrThrow(results, Columns.IS_HIDDEN));
   }
 }
diff --git a/api/src/main/java/marquez/db/models/NamespaceRow.java b/api/src/main/java/marquez/db/models/NamespaceRow.java
@@ -20,6 +20,7 @@ public class NamespaceRow {
   @NonNull String name;
   @Nullable String description;
   @NonNull String currentOwnerName;
+  @NonNull Boolean isHidden;
 
   public Optional<String> getDescription() {
     return Optional.ofNullable(description);

diff --git a/api/src/main/java/marquez/service/models/Namespace.java b/api/src/main/java/marquez/service/models/Namespace.java
@@ -20,6 +20,7 @@ public class Namespace {
   @NonNull Instant updatedAt;
   @NonNull OwnerName ownerName;
   @Nullable String description;
+  @NonNull Boolean isHidden;
 
   public Optional<String> getDescription() {
     return Optional.ofNullable(description);

diff --git a/api/src/main/resources/marquez/db/migration/V53__add_hidden_namespace.sql b/api/src/main/resources/marquez/db/migration/V53__add_hidden_namespace.sql
@@ -0,0 +1 @@
+ALTER TABLE namespaces ADD COLUMN is_hidden BOOLEAN DEFAULT FALSE;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		ALTER TABLE namespaces ADD COLUMN is_hidden BOOLEAN DEFAULT FALSE;