From 2e95a6d9729ad591ef16c950453529a704d534db Mon Sep 17 00:00:00 2001 From: Jacek Date: Wed, 26 Aug 2020 10:16:58 +0200 Subject: [PATCH] SONAR-13802 Fix NPE when component favorite orphan selected - db migration - web api fixes --- .../db/migration/version/v85/DbVersion85.java | 1 + .../DropOrphanFavoritesFromProperties.java | 47 +++++++ ...DropOrphanFavoritesFromPropertiesTest.java | 119 ++++++++++++++++++ .../schema.sql | 12 ++ .../ws/ComponentViewerJsonWriter.java | 6 +- .../component/ws/SearchProjectsAction.java | 1 + .../ws/SearchProjectsActionTest.java | 24 +++- 7 files changed, 206 insertions(+), 4 deletions(-) create mode 100644 server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromProperties.java create mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest.java create mode 100644 server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest/schema.sql diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DbVersion85.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DbVersion85.java index ceea70ead6b..9d921d3bd1c 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DbVersion85.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DbVersion85.java @@ -31,6 +31,7 @@ public class DbVersion85 implements DbVersion { .add(4002, "Drop 'project_alm_bindings' table", DropProjectAlmBindings.class) .add(4003, "Drop unused variation values columns in 'project_measures' table", DropUnusedVariationsInProjectMeasures.class) .add(4004, "Drop unused periods in 'snapshots' table", DropUnusedPeriodsInSnapshots.class) + .add(4005, "Drop orphan favorites from 'properties' table", DropOrphanFavoritesFromProperties.class) ; } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromProperties.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromProperties.java new file mode 100644 index 00000000000..25aa6c76bb1 --- /dev/null +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromProperties.java @@ -0,0 +1,47 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.platform.db.migration.version.v85; + +import java.sql.SQLException; +import org.sonar.db.Database; +import org.sonar.server.platform.db.migration.step.DataChange; +import org.sonar.server.platform.db.migration.step.MassUpdate; + +public class DropOrphanFavoritesFromProperties extends DataChange { + + public DropOrphanFavoritesFromProperties(Database db) { + super(db); + } + + @Override + public void execute(Context context) throws SQLException { + MassUpdate massUpdate = context.prepareMassUpdate(); + massUpdate.select("select uuid from properties where prop_key = ? and component_uuid is null") + .setString(1, "favourite"); + massUpdate.update("delete from properties where uuid = ?"); + + massUpdate.execute((row, update) -> { + String propertyUuid = row.getString(1); + update.setString(1, propertyUuid); + return true; + }); + + } +} diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest.java new file mode 100644 index 00000000000..f2e3c5040c9 --- /dev/null +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest.java @@ -0,0 +1,119 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.platform.db.migration.version.v85; + +import java.sql.SQLException; +import java.time.Instant; +import javax.annotation.Nullable; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.core.util.Uuids; +import org.sonar.db.CoreDbTester; +import org.sonar.server.platform.db.migration.step.DataChange; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DropOrphanFavoritesFromPropertiesTest { + + private static final String PROPERTIES_TABLE_NAME = "properties"; + + @Rule + public CoreDbTester dbTester = CoreDbTester.createForSchema(DropOrphanFavoritesFromPropertiesTest.class, "schema.sql"); + + private DataChange underTest = new DropOrphanFavoritesFromProperties(dbTester.database()); + + private static final String COMPONENT_UUID_1 = Uuids.createFast(); + private static final String COMPONENT_UUID_2 = Uuids.createFast(); + private static final String COMPONENT_UUID_3 = Uuids.createFast(); + private static final String COMPONENT_UUID_4 = Uuids.createFast(); + + private static final String USER_UUID_1 = "1"; + private static final String USER_UUID_2 = "2"; + + @Before + public void setup() { + insertProperty("1", USER_UUID_1, COMPONENT_UUID_1, "test"); + insertProperty("2", USER_UUID_2, COMPONENT_UUID_2, "test2"); + insertProperty("3", USER_UUID_2, COMPONENT_UUID_3, "test3"); + insertProperty("4", USER_UUID_2, null, "test3"); + } + + @Test + public void migrate() throws SQLException { + insertProperty("5", USER_UUID_1, COMPONENT_UUID_4, "favourite"); + // properties to remove + insertProperty("6", USER_UUID_2, null, "favourite"); + insertProperty("7", USER_UUID_2, null, "favourite"); + insertProperty("8", USER_UUID_2, null, "favourite"); + + underTest.execute(); + + assertThat(dbTester.countRowsOfTable(PROPERTIES_TABLE_NAME)).isEqualTo(5); + assertThat(dbTester.select("select uuid from properties").stream().map(columns -> columns.get("UUID"))) + .containsOnly("1", "2", "3", "4", "5"); + } + + @Test + public void does_not_fail_when_properties_table_empty() throws SQLException { + dbTester.executeUpdateSql("delete from properties"); + + underTest.execute(); + + assertThat(dbTester.countRowsOfTable(PROPERTIES_TABLE_NAME)).isZero(); + } + + @Test + public void does_not_remove_properties_with_key_other_than_favourite() throws SQLException { + underTest.execute(); + + assertThat(dbTester.countRowsOfTable(PROPERTIES_TABLE_NAME)).isEqualTo(4L); + } + + @Test + public void migration_is_re_entrant() throws SQLException { + insertProperty("5", USER_UUID_1, COMPONENT_UUID_1, "favourite"); + // properties to remove + insertProperty("6", USER_UUID_1, null, "favourite"); + insertProperty("7", USER_UUID_2, null, "favourite"); + + underTest.execute(); + + insertProperty("8", USER_UUID_2, null, "favourite"); + insertProperty("9", USER_UUID_2, null, "favourite"); + + // re-entrant + underTest.execute(); + + assertThat(dbTester.countRowsOfTable(PROPERTIES_TABLE_NAME)).isEqualTo(5); + assertThat(dbTester.select("select uuid from properties").stream().map(columns -> columns.get("UUID"))) + .containsOnly("1", "2", "3", "4", "5"); + } + + private void insertProperty(String uuid, String userUuid, @Nullable String componentUuid, String propKey) { + dbTester.executeInsert(PROPERTIES_TABLE_NAME, + "uuid", uuid, + "user_uuid", userUuid, + "component_uuid", componentUuid, + "prop_key", propKey, + "is_empty", true, + "created_at", Instant.now().toEpochMilli()); + } +} diff --git a/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest/schema.sql b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest/schema.sql new file mode 100644 index 00000000000..eb6cd7f9a5c --- /dev/null +++ b/server/sonar-db-migration/src/test/resources/org/sonar/server/platform/db/migration/version/v85/DropOrphanFavoritesFromPropertiesTest/schema.sql @@ -0,0 +1,12 @@ +CREATE TABLE "PROPERTIES"( + "PROP_KEY" VARCHAR(512) NOT NULL, + "IS_EMPTY" BOOLEAN NOT NULL, + "TEXT_VALUE" VARCHAR(4000), + "CLOB_VALUE" CLOB(2147483647), + "CREATED_AT" BIGINT NOT NULL, + "COMPONENT_UUID" VARCHAR(40), + "UUID" VARCHAR(40) NOT NULL, + "USER_UUID" VARCHAR(255) +); +ALTER TABLE "PROPERTIES" ADD CONSTRAINT "PK_PROPERTIES" PRIMARY KEY("UUID"); +CREATE INDEX "PROPERTIES_KEY" ON "PROPERTIES"("PROP_KEY"); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/ComponentViewerJsonWriter.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/ComponentViewerJsonWriter.java index 7134dfc1bb1..ff7e3127765 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/ComponentViewerJsonWriter.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/ComponentViewerJsonWriter.java @@ -98,10 +98,10 @@ public class ComponentViewerJsonWriter { writeComponentWithoutFav(json, component, session, true); List propertyDtos = dbClient.propertiesDao().selectByQuery(PropertyQuery.builder() - .setKey("favourite") - .setComponentUuid(component.uuid()) + .setKey("favourite") + .setComponentUuid(component.uuid()) .setUserUuid(userSession.getUuid()) - .build(), + .build(), session); boolean isFavourite = propertyDtos.size() == 1; json.prop("fav", isFavourite); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/SearchProjectsAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/SearchProjectsAction.java index 60682508513..600688cbabc 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/SearchProjectsAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/component/ws/SearchProjectsAction.java @@ -360,6 +360,7 @@ public class SearchProjectsAction implements ComponentsWsAction { List favoriteDbUuids = props.stream() .map(PropertyDto::getComponentUuid) + .filter(Objects::nonNull) .collect(MoreCollectors.toList(props.size())); return dbClient.componentDao().selectByUuids(dbSession, favoriteDbUuids).stream() diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ws/SearchProjectsActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ws/SearchProjectsActionTest.java index 97d53d1e022..6d0f216578f 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ws/SearchProjectsActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/component/ws/SearchProjectsActionTest.java @@ -30,6 +30,7 @@ import java.util.Optional; import java.util.function.Consumer; import java.util.stream.IntStream; import java.util.stream.Stream; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -627,6 +628,23 @@ public class SearchProjectsActionTest { assertThat(result.getComponentsList()).extracting(Component::getKey).containsExactly(javaProject.getDbKey(), markDownProject.getDbKey()); } + @Test + public void does_not_fail_on_orphan_favorite() { + userSession.logIn(); + OrganizationDto organization = db.organizations().insert(); + ComponentDto javaProject = insertProject(organization); + ComponentDto markDownProject = insertProject(organization); + ComponentDto sonarQubeProject = insertProject(organization); + Stream.of(javaProject, markDownProject).forEach(this::addFavourite); + + addFavourite((String) null); + + SearchProjectsWsResponse result = call(request.setFilter("isFavorite")); + + assertThat(result.getComponentsCount()).isEqualTo(2); + assertThat(result.getComponentsList()).extracting(Component::getKey).containsExactly(javaProject.getDbKey(), markDownProject.getDbKey()); + } + @Test public void filtering_on_favorites_returns_empty_results_if_not_logged_in() { userSession.anonymous(); @@ -1411,7 +1429,11 @@ public class SearchProjectsActionTest { } private void addFavourite(ComponentDto project) { - dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey("favourite").setComponentUuid(project.uuid()).setUserUuid(userSession.getUuid())); + addFavourite(project.uuid()); + } + + private void addFavourite(@Nullable String componentUuid) { + dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey("favourite").setComponentUuid(componentUuid).setUserUuid(userSession.getUuid())); dbSession.commit(); } -- 2.39.5