Fix project.active being nullable

Ensure the field defaults to `true`, both in Java and the database. During upgrade, migrate all values that are currently `null` to `true`. Solidify this change by switching `project.active` from `Boolean` to `boolean`. Adjust logic that previously had to check for `null`. Fixes #4410 Signed-off-by: nscuro <[email protected]>
DependencyTrack · Nov 28, 2024 · 0b14b26 · 0b14b26
1 parent 10f43a6
commit 0b14b26
Show file tree

Hide file tree

Showing 16 changed files with 157 additions and 65 deletions.
diff --git a/dev/docker-compose.mysql.yml b/dev/docker-compose.mysql.yml
@@ -0,0 +1,43 @@
+# This file is part of Dependency-Track.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# SPDX-License-Identifier: Apache-2.0
+# Copyright (c) OWASP Foundation. All Rights Reserved.
+services:
+  apiserver:
+    depends_on:
+    - mysql
+    environment:
+      ALPINE_DATABASE_MODE: "external"
+      ALPINE_DATABASE_URL: "jdbc:mysql://mysql:3306/dtrack?autoReconnect=true&useSSL=false&sessionVariables=sql_mode='ANSI_QUOTES,STRICT_TRANS_TABLES,ONLY_FULL_GROUP_BY,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION'"
+      ALPINE_DATABASE_DRIVER: "com.mysql.cj.jdbc.Driver"
+      ALPINE_DATABASE_USERNAME: "dtrack"
+      ALPINE_DATABASE_PASSWORD: "dtrack"
+
+  mysql:
+    image: mysql:5.7
+    platform: "linux/amd64" # arm64 is not supported
+    environment:
+      MYSQL_DATABASE: "dtrack"
+      MYSQL_RANDOM_ROOT_PASSWORD: "yes"
+      MYSQL_USER: "dtrack"
+      MYSQL_PASSWORD: "dtrack"
+    ports:
+    - "127.0.0.1:3306:3306"
+    volumes:
+    - "mysql-data:/var/lib/mysql"
+    restart: unless-stopped
+
+volumes:
+  mysql-data: { }
diff --git a/src/main/java/org/dependencytrack/model/Project.java b/src/main/java/org/dependencytrack/model/Project.java
@@ -268,9 +268,9 @@ public enum FetchGroup {
     private Double lastInheritedRiskScore;
 
     @Persistent
-    @Column(name = "ACTIVE")
+    @Column(name = "ACTIVE", defaultValue = "true")
     @JsonSerialize(nullsUsing = BooleanDefaultTrueSerializer.class)
-    private Boolean active; // Added in v3.6. Existing records need to be nullable on upgrade.
+    private boolean active = true;
 
     @Persistent
     @Index(name = "PROJECT_IS_LATEST_IDX")
@@ -512,11 +512,11 @@ public void setExternalReferences(List<ExternalReference> externalReferences) {
         this.externalReferences = externalReferences;
     }
 
-    public Boolean isActive() {
+    public boolean isActive() {
         return active;
     }
 
-    public void setActive(Boolean active) {
+    public void setActive(boolean active) {
         this.active = active;
     }
 

diff --git a/src/main/java/org/dependencytrack/model/ProjectVersion.java b/src/main/java/org/dependencytrack/model/ProjectVersion.java
@@ -26,5 +26,5 @@
  * Value object holding UUID and version for a project
  */
 @JsonInclude(JsonInclude.Include.NON_NULL)
-public record ProjectVersion(UUID uuid, String version, Boolean active) implements Serializable {
+public record ProjectVersion(UUID uuid, String version, boolean active) implements Serializable {
 }
diff --git a/src/main/java/org/dependencytrack/notification/NotificationRouter.java b/src/main/java/org/dependencytrack/notification/NotificationRouter.java
@@ -274,7 +274,7 @@ private void limitToProject(
             }
 
             if (isLimitedToTags) {
-                final Predicate<Project> tagMatchPredicate = project -> (project.isActive() == null || project.isActive())
+                final Predicate<Project> tagMatchPredicate = project -> project.isActive()
                         && project.getTags() != null
                         && project.getTags().stream().anyMatch(rule.getTags()::contains);
 
@@ -340,7 +340,7 @@ private boolean checkIfChildrenAreAffected(Project parent, UUID uuid) {
             return false;
         }
         for (Project child : parent.getChildren()) {
-            final boolean isChildActive = child.isActive() == null || child.isActive();
+            final boolean isChildActive = child.isActive();
             if ((child.getUuid().equals(uuid) && isChildActive) || isChild) {
                 return true;
             }

diff --git a/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java b/src/main/java/org/dependencytrack/persistence/PolicyQueryManager.java
@@ -460,7 +460,7 @@ public PaginatedResult getPolicyViolations(boolean includeSuppressed, boolean sh
             filterCriteria.add("(analysis.suppressed == false || analysis.suppressed == null)");
         }
         if (!showInactive) {
-            filterCriteria.add("(project.active == true || project.active == null)");
+            filterCriteria.add("project.active");
         }
         processViolationsFilters(filters, params, filterCriteria);
         if (orderBy == null) {

diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryFilterBuilder.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryFilterBuilder.java
@@ -46,7 +46,7 @@ class ProjectQueryFilterBuilder {
 
     ProjectQueryFilterBuilder excludeInactive(boolean excludeInactive) {
         if (excludeInactive) {
-            filterCriteria.add("(active == true || active == null)");
+            filterCriteria.add("active");
         }
         return this;
     }

diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java
@@ -182,7 +182,7 @@ public List<Project> getAllProjects() {
     public List<Project> getAllProjects(boolean excludeInactive) {
         final Query<Project> query = pm.newQuery(Project.class);
         if (excludeInactive) {
-            query.setFilter("active == true || active == null");
+            query.setFilter("active");
         }
         query.setOrdering("id asc");
         return query.executeList();
@@ -473,9 +473,6 @@ public Project createProject(final Project project, List<Tag> tags, boolean comm
         if (project.getParent() != null && !Boolean.TRUE.equals(project.getParent().isActive())){
             throw new IllegalArgumentException("An inactive Parent cannot be selected as parent");
         }
-        if (project.isActive() == null) {
-            project.setActive(Boolean.TRUE);
-        }
         final Project oldLatestProject = project.isLatest() ? getLatestProjectVersion(project.getName()) : null;
         final Project result = callInTransaction(() -> {
             // Remove isLatest flag from current latest project version, if the new project will be the latest

diff --git a/src/main/java/org/dependencytrack/search/ComponentIndexer.java b/src/main/java/org/dependencytrack/search/ComponentIndexer.java
@@ -114,7 +114,7 @@ private static List<ComponentDocument> fetchNext(final QueryManager qm, final Lo
         final Query<Component> query = qm.getPersistenceManager().newQuery(Component.class);
         var filterParts = new ArrayList<String>();
         var params = new HashMap<String, Object>();
-        filterParts.add("(project.active == null || project.active)");
+        filterParts.add("project.active");
         if (lastId != null) {
             filterParts.add("id > :lastId");
             params.put("lastId", lastId);

diff --git a/src/main/java/org/dependencytrack/search/ProjectIndexer.java b/src/main/java/org/dependencytrack/search/ProjectIndexer.java
@@ -114,7 +114,7 @@ private static List<ProjectDocument> fetchNext(final QueryManager qm, final Long
         final Query<Project> query = qm.getPersistenceManager().newQuery(Project.class);
         var filterParts = new ArrayList<String>();
         var params = new HashMap<String, Object>();
-        filterParts.add("(active == null || active)");
+        filterParts.add("active");
         if (lastId != null) {
             filterParts.add("id > :lastId");
             params.put("lastId", lastId);

diff --git a/src/main/java/org/dependencytrack/tasks/metrics/PortfolioMetricsUpdateTask.java b/src/main/java/org/dependencytrack/tasks/metrics/PortfolioMetricsUpdateTask.java
@@ -175,9 +175,9 @@ private void updateMetrics() throws Exception {
     private List<Project> fetchNextActiveProjectsBatch(final PersistenceManager pm, final Long lastId) {
         final Query<Project> query = pm.newQuery(Project.class);
         if (lastId == null) {
-            query.setFilter("(active == null || active == true)");
+            query.setFilter("active");
         } else {
-            query.setFilter("(active == null || active == true) && id < :lastId");
+            query.setFilter("active && id < :lastId");
             query.setParameters(lastId);
         }
         query.setOrdering("id DESC");

diff --git a/src/main/java/org/dependencytrack/tasks/repositories/RepositoryMetaAnalyzerTask.java b/src/main/java/org/dependencytrack/tasks/repositories/RepositoryMetaAnalyzerTask.java
@@ -294,7 +294,7 @@ protected boolean isCacheCurrent(ComponentAnalysisCache cac, String target) {
 
     private List<Component> fetchNextComponentBatch(final QueryManager qm, final Long lastId) {
         final var filterConditions = new ArrayList<>(List.of(
-                "(project.active == null || project.active)",
+                "project.active",
                 "purl != null"));
         final var filterParams = new HashMap<String, Object>();
         if (lastId != null) {

diff --git a/src/main/java/org/dependencytrack/upgrade/UpgradeItems.java b/src/main/java/org/dependencytrack/upgrade/UpgradeItems.java
@@ -40,6 +40,7 @@ class UpgradeItems {
         UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4100.v4100Updater.class);
         UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4110.v4110Updater.class);
         UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4120.v4120Updater.class);
+        UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4122.v4122Updater.class);
     }
 
     static List<Class<? extends UpgradeItem>> getUpgradeItems() {

diff --git a/src/main/java/org/dependencytrack/upgrade/v4122/v4122Updater.java b/src/main/java/org/dependencytrack/upgrade/v4122/v4122Updater.java
@@ -0,0 +1,84 @@
+/*
+ * This file is part of Dependency-Track.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright (c) OWASP Foundation. All Rights Reserved.
+ */
+package org.dependencytrack.upgrade.v4122;
+
+import alpine.common.logging.Logger;
+import alpine.persistence.AlpineQueryManager;
+import alpine.server.upgrade.AbstractUpgradeItem;
+import alpine.server.util.DbUtil;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+public class v4122Updater extends AbstractUpgradeItem {
+
+    private static final Logger LOGGER = Logger.getLogger(v4122Updater.class);
+
+    @Override
+    public String getSchemaVersion() {
+        return "4.12.2";
+    }
+
+    @Override
+    public void executeUpgrade(final AlpineQueryManager qm, final Connection connection) throws Exception {
+        fixProjectActiveNullValues(connection);
+    }
+
+    private static void fixProjectActiveNullValues(final Connection connection) throws SQLException {
+        LOGGER.info("Setting active flag to true for projects where it's currently null");
+        try (final PreparedStatement ps = connection.prepareStatement("""
+                UPDATE "PROJECT"
+                   SET "ACTIVE" = ?
+                 WHERE "ACTIVE" IS NULL;
+                """)) {
+            ps.setBoolean(1, true);
+
+            final int modifiedProjects = ps.executeUpdate();
+            LOGGER.info("Updated active flag of %d projects".formatted(modifiedProjects));
+        }
+
+        LOGGER.info("Setting default value of the project active flag to true");
+        try (final Statement stmt = connection.createStatement()) {
+            if (DbUtil.isMssql()) {
+                stmt.executeUpdate("""
+                        ALTER TABLE "PROJECT"
+                          ADD DEFAULT 'true'
+                          FOR "ACTIVE";
+                        """);
+            } else if (DbUtil.isMysql()) {
+                stmt.executeUpdate("""
+                         ALTER TABLE "PROJECT"
+                        MODIFY COLUMN "ACTIVE" BIT(1) DEFAULT 1;
+                        """);
+            } else if (DbUtil.isPostgreSQL() || DbUtil.isH2()) {
+                stmt.executeUpdate("""
+                        ALTER TABLE "PROJECT"
+                        ALTER COLUMN "ACTIVE"
+                          SET DEFAULT TRUE;
+                        """);
+            } else {
+                throw new IllegalStateException(
+                        "Unsupported database: " + connection.getMetaData().getDatabaseProductName());
+            }
+        }
+    }
+
+}
diff --git a/src/test/java/org/dependencytrack/model/ProjectTest.java b/src/test/java/org/dependencytrack/model/ProjectTest.java
@@ -55,11 +55,9 @@ public void testProjectPersistActiveFieldDefaultsToTrue() {
         project.setName("Example Project 1");
         project.setDescription("Description 1");
         project.setVersion("1.0");
-        project.setActive(null);
 
         Project persistedProject = qm.createProject(project, null, false);
 
-        Assert.assertNotNull(persistedProject.isActive());
         Assert.assertTrue(persistedProject.isActive());
     }
 }
diff --git a/src/test/java/org/dependencytrack/notification/NotificationRouterTest.java b/src/test/java/org/dependencytrack/notification/NotificationRouterTest.java
@@ -698,43 +698,6 @@ public void testAffectedInactiveChild() {
         Assert.assertEquals(0, rules.size());
     }
 
-    @Test
-    public void testAffectedActiveNullChild() {
-        NotificationPublisher publisher = createSlackPublisher();
-        // Creates a new rule and defines when the rule should be triggered (notifyOn)
-        NotificationRule rule = qm.createNotificationRule("Matching Test Rule", NotificationScope.PORTFOLIO, NotificationLevel.INFORMATIONAL, publisher);
-        Set<NotificationGroup> notifyOn = new HashSet<>();
-        notifyOn.add(NotificationGroup.NEW_VULNERABILITY);
-        rule.setNotifyOn(notifyOn);
-        // Creates a project which will later be matched on
-        List<Project> projects = new ArrayList<>();
-        Project grandParent = qm.createProject("Test Project Grandparent", null, "1.0", null, null, null, true, false);
-        Project parent = qm.createProject("Test Project Parent", null, "1.0", null, grandParent, null, true, false);
-        Project child = qm.createProject("Test Project Child", null, "1.0", null, parent, null, true, false);
-        Project grandChild = qm.createProject("Test Project Grandchild", null, "1.0", null, child, null, true, false);
-        grandChild.setActive(null); // https://github.com/DependencyTrack/dependency-track/issues/3296
-        projects.add(grandParent);
-        rule.setProjects(projects);
-        // Creates a new component
-        Component component = new Component();
-        component.setProject(grandChild);
-        // Creates a new notification
-        Notification notification = new Notification();
-        notification.setScope(NotificationScope.PORTFOLIO.name());
-        notification.setGroup(NotificationGroup.NEW_VULNERABILITY.name());
-        notification.setLevel(NotificationLevel.INFORMATIONAL);
-        // Notification should be limited to only specific projects - Set the projects which are affected by the notification event
-        Set<Project> affectedProjects = new HashSet<>();
-        affectedProjects.add(grandChild);
-        NewVulnerabilityIdentified subject = new NewVulnerabilityIdentified(new Vulnerability(), component, affectedProjects, null);
-        notification.setSubject(subject);
-        // Ok, let's test this
-        NotificationRouter router = new NotificationRouter();
-        List<NotificationRule> rules = router.resolveRules(PublishContext.from(notification), notification);
-        Assert.assertTrue(rule.isNotifyChildren());
-        Assert.assertEquals(1, rules.size());
-    }
-
     @Test
     public void testValidMatchingTagLimitingRule() {
         NotificationPublisher publisher = createSlackPublisher();

diff --git a/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java
@@ -318,7 +318,7 @@ public void getProjectByUuidTest() {
                 .withMatcher("projectUuid", equalTo(project.getUuid().toString()))
                 .withMatcher("parentUuid", equalTo(parentProject.getUuid().toString()))
                 .withMatcher("childUuid", equalTo(childProject.getUuid().toString()))
-                .isEqualTo("""
+                .isEqualTo(/* language=JSON */ """
                         {
                           "name": "acme-app",
                           "version": "1.0.0",
@@ -344,7 +344,8 @@ public void getProjectByUuidTest() {
                           "versions": [
                             {
                               "uuid": "${json-unit.matches:projectUuid}",
-                              "version": "1.0.0"
+                              "version": "1.0.0",
+                              "active": true
                             }
                           ]
                         }
@@ -1012,20 +1013,25 @@ public void updateProjectNotPermittedTest() {
 
     @Test
     public void updateProjectTestIsActiveEqualsNull() {
-        Project project = qm.createProject("ABC", null, "1.0", null, null, null, true, false);
-        project.setDescription("Test project");
-        project.setActive(null);
-        Assert.assertNull(project.isActive());
-        Response response = jersey.target(V1_PROJECT)
+        final Project project = qm.createProject("ABC", null, "1.0", null, null, null, true, false);
+        final Response response = jersey.target(V1_PROJECT)
                 .request()
                 .header(X_API_KEY, apiKey)
-                .post(Entity.entity(project, MediaType.APPLICATION_JSON));
+                .post(Entity.json(/* language=JSON */ """
+                        {
+                          "uuid": "%s",
+                          "name": "ABC",
+                          "version": "1.0",
+                          "description": "Test project"
+                        }
+                        """.formatted(project.getUuid())));
         Assert.assertEquals(200, response.getStatus(), 0);
         JsonObject json = parseJsonObject(response);
         Assert.assertNotNull(json);
         Assert.assertEquals("ABC", json.getString("name"));
         Assert.assertEquals("1.0", json.getString("version"));
         Assert.assertEquals("Test project", json.getString("description"));
+        Assert.assertTrue(json.getBoolean("active"));
     }
 
     @Test