From fb3151df4890c325cb44dc48a049a77e460e420b Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 29 May 2018 22:17:15 +0200 Subject: [PATCH] SONAR-10691 remove coupling of migrations with ElasticsearchClient --- ...archClient.java => MigrationEsClient.java} | 6 +- ...LoginToSubmitterUuidOnTableCeActivity.java | 8 ++- .../step/ElasticsearchChangeTest.java | 51 ----------------- ...nToSubmitterUuidOnTableCeActivityTest.java | 8 ++- .../server/es/MigrationEsClientImpl.java | 46 +++++++++++++++ .../sonar/server/es/SimpleEsClientImpl.java | 57 ------------------- .../platformlevel/PlatformLevel2.java | 2 + ...st.java => MigrationEsClientImplTest.java} | 41 ++++++------- 8 files changed, 82 insertions(+), 137 deletions(-) rename server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/es/{ElasticsearchClient.java => MigrationEsClient.java} (82%) delete mode 100644 server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/ElasticsearchChangeTest.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/es/MigrationEsClientImpl.java delete mode 100644 server/sonar-server/src/main/java/org/sonar/server/es/SimpleEsClientImpl.java rename server/sonar-server/src/test/java/org/sonar/server/es/{SimpleEsClientImplTest.java => MigrationEsClientImplTest.java} (59%) diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/es/ElasticsearchClient.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/es/MigrationEsClient.java similarity index 82% rename from server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/es/ElasticsearchClient.java rename to server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/es/MigrationEsClient.java index 677118f24a9..0c63c22256e 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/es/ElasticsearchClient.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/es/MigrationEsClient.java @@ -20,10 +20,10 @@ package org.sonar.server.platform.db.migration.es; -public interface ElasticsearchClient { +public interface MigrationEsClient { /** - * This method is reentrant and does not fail if indexName or otherIndexNames does not exist + * This method is re-entrant and does not fail if indexName or otherIndexNames do not exist */ - void deleteIndexes(String... otherIndexNames); + void deleteIndexes(String name, String... otherNames); } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivity.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivity.java index 59eaae83706..073b24ed540 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivity.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivity.java @@ -22,6 +22,7 @@ package org.sonar.server.platform.db.migration.version.v72; import java.sql.SQLException; import org.sonar.db.Database; +import org.sonar.server.platform.db.migration.es.MigrationEsClient; import org.sonar.server.platform.db.migration.sql.RenameColumnsBuilder; import org.sonar.server.platform.db.migration.step.DdlChange; @@ -29,8 +30,11 @@ import static org.sonar.server.platform.db.migration.def.VarcharColumnDef.newVar public class RenameSubmitterLoginToSubmitterUuidOnTableCeActivity extends DdlChange { - public RenameSubmitterLoginToSubmitterUuidOnTableCeActivity(Database db) { + private final MigrationEsClient esClient; + + public RenameSubmitterLoginToSubmitterUuidOnTableCeActivity(Database db, MigrationEsClient esClient) { super(db); + this.esClient = esClient; } @Override @@ -44,6 +48,6 @@ public class RenameSubmitterLoginToSubmitterUuidOnTableCeActivity extends DdlCha .build()) .build()); - context.deleteIndexes("users"); + esClient.deleteIndexes("users"); } } diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/ElasticsearchChangeTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/ElasticsearchChangeTest.java deleted file mode 100644 index 7b1c1456c1a..00000000000 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/step/ElasticsearchChangeTest.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 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.step; - -import java.sql.SQLException; -import org.junit.Test; -import org.mockito.ArgumentCaptor; -import org.sonar.server.platform.db.migration.es.ElasticsearchClient; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; - -public class ElasticsearchChangeTest { - - private ElasticsearchClient client = mock(ElasticsearchClient.class); - - @Test - public void deleteIndice_call() throws SQLException { - ArgumentCaptor indices = ArgumentCaptor.forClass(String.class); - - new ElasticsearchChange(client) { - @Override - protected void execute(Context context) throws SQLException { - context.deleteIndice("a", "b", "c"); - } - }.execute(); - - verify(client, times(1)).deleteIndice(indices.capture()); - assertThat(indices.getAllValues()).containsExactly("a", "b", "c"); - } -} diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivityTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivityTest.java index 988b2bec020..4cf1ee0bf1f 100644 --- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivityTest.java +++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v72/RenameSubmitterLoginToSubmitterUuidOnTableCeActivityTest.java @@ -25,8 +25,11 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.db.CoreDbTester; +import org.sonar.server.platform.db.migration.es.MigrationEsClient; import static java.sql.Types.VARCHAR; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; public class RenameSubmitterLoginToSubmitterUuidOnTableCeActivityTest { @@ -36,7 +39,8 @@ public class RenameSubmitterLoginToSubmitterUuidOnTableCeActivityTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - private RenameSubmitterLoginToSubmitterUuidOnTableCeActivity underTest = new RenameSubmitterLoginToSubmitterUuidOnTableCeActivity(db.database()); + private MigrationEsClient esClient = mock(MigrationEsClient.class); + private RenameSubmitterLoginToSubmitterUuidOnTableCeActivity underTest = new RenameSubmitterLoginToSubmitterUuidOnTableCeActivity(db.database(), esClient); @Test public void rename_column() throws SQLException { @@ -44,6 +48,8 @@ public class RenameSubmitterLoginToSubmitterUuidOnTableCeActivityTest { db.assertColumnDefinition("ce_activity", "submitter_uuid", VARCHAR, 255, true); db.assertColumnDoesNotExist("ce_activity", "submitter_login"); + + verify(esClient).deleteIndexes("users"); } public void migration_is_not_reentrant() throws SQLException { diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/MigrationEsClientImpl.java b/server/sonar-server/src/main/java/org/sonar/server/es/MigrationEsClientImpl.java new file mode 100644 index 00000000000..4d30aea9940 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/es/MigrationEsClientImpl.java @@ -0,0 +1,46 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 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.es; + +import com.google.common.collect.Sets; +import java.util.Arrays; +import java.util.Set; +import java.util.stream.Stream; +import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; +import org.sonar.server.platform.db.migration.es.MigrationEsClient; + +public class MigrationEsClientImpl implements MigrationEsClient { + private final EsClient client; + + public MigrationEsClientImpl(EsClient client) { + this.client = client; + } + + @Override + public void deleteIndexes(String name, String... otherNames) { + GetMappingsResponse mappings = client.nativeClient().admin().indices().prepareGetMappings("_all").get(); + Set existingIndices = Sets.newHashSet(mappings.mappings().keysIt()); + Stream.concat(Stream.of(name), Arrays.stream(otherNames)) + .distinct() + .filter(existingIndices::contains) + .forEach(index -> client.nativeClient().admin().indices().prepareDelete(index).get()); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/SimpleEsClientImpl.java b/server/sonar-server/src/main/java/org/sonar/server/es/SimpleEsClientImpl.java deleted file mode 100644 index 47b32e4b34a..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/es/SimpleEsClientImpl.java +++ /dev/null @@ -1,57 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 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.es; - -import java.util.Arrays; -import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; -import org.elasticsearch.client.Client; -import org.sonar.server.platform.db.migration.es.ElasticsearchClient; - -import static java.util.Objects.requireNonNull; - -public class SimpleEsClientImpl implements ElasticsearchClient { - private final Client nativeClient; - - public SimpleEsClientImpl(Client nativeClient) { - this.nativeClient = requireNonNull(nativeClient); - } - - /** - * This method is reentrant and does not fail if indexName or otherIndexNames does not exist - */ - public void deleteIndexes(String... indexNames) { - if (indexNames.length == 0) { - return; - } - - GetMappingsResponse getMappingsResponse = nativeClient.admin().indices().prepareGetMappings("_all").get(); - String[] allIndexes = getMappingsResponse.mappings().keys().toArray(String.class); - String[] intersection = intersection(indexNames, allIndexes); - nativeClient.admin().indices().prepareDelete(intersection).get(); - } - - private String[] intersection(String[] a, String[] b) { - return Arrays.stream(a) - .distinct() - .filter(x -> Arrays.stream(b).anyMatch(y -> y != null && y.equals(x))) - .toArray(String[]::new); - } -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java index b32ad3d14e9..a5755a839d3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel2.java @@ -26,6 +26,7 @@ import org.sonar.core.platform.PluginLoader; import org.sonar.core.extension.CoreExtensionRepositoryImpl; import org.sonar.core.extension.CoreExtensionsLoader; import org.sonar.server.l18n.ServerI18n; +import org.sonar.server.es.MigrationEsClientImpl; import org.sonar.server.platform.DatabaseServerCompatibility; import org.sonar.server.platform.DefaultServerUpgradeStatus; import org.sonar.server.platform.StartupMetadataProvider; @@ -56,6 +57,7 @@ public class PlatformLevel2 extends PlatformLevel { MigrationConfigurationModule.class, DatabaseVersion.class, DatabaseServerCompatibility.class, + MigrationEsClientImpl.class, new StartupMetadataProvider(), DefaultServerUpgradeStatus.class, diff --git a/server/sonar-server/src/test/java/org/sonar/server/es/SimpleEsClientImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/es/MigrationEsClientImplTest.java similarity index 59% rename from server/sonar-server/src/test/java/org/sonar/server/es/SimpleEsClientImplTest.java rename to server/sonar-server/src/test/java/org/sonar/server/es/MigrationEsClientImplTest.java index 1818be26afe..62a4e498e47 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/es/SimpleEsClientImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/es/MigrationEsClientImplTest.java @@ -19,53 +19,48 @@ */ package org.sonar.server.es; -import org.elasticsearch.client.Client; +import java.util.Iterator; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.junit.Rule; import org.junit.Test; import org.sonar.api.config.internal.MapSettings; +import org.sonar.server.platform.db.migration.es.MigrationEsClient; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; import static org.sonar.server.es.NewIndex.SettingsConfiguration.newBuilder; -public class SimpleEsClientImplTest { +public class MigrationEsClientImplTest { @Rule - public EsTester es = EsTester.createCustom(new FakeIndexDefinition(), + public EsTester es = EsTester.createCustom( new SimpleIndexDefinition("a"), new SimpleIndexDefinition("b"), new SimpleIndexDefinition("c")); - private Client client = es.client().nativeClient(); - private SimpleEsClientImpl underTest = new SimpleEsClientImpl(client); + private MigrationEsClient underTest = new MigrationEsClientImpl(es.client()); @Test - public void call_without_arguments_does_not_generate_an_elasticsearch_call() { - Client client = mock(Client.class); - SimpleEsClientImpl underTest = new SimpleEsClientImpl(client); - underTest.deleteIndexes(); + public void delete_existing_index() { + underTest.deleteIndexes("a"); - verify(client, never()).admin(); + assertThat(loadExistingIndices()) + .doesNotContain("a") + .contains("b", "c"); } @Test - public void delete_known_indice_must_delete_the_index() { - underTest.deleteIndexes("fakes"); + public void ignore_indices_that_do_not_exist() { + underTest.deleteIndexes("a", "xxx", "c"); - assertThat(es.client().nativeClient().admin().indices().prepareGetMappings().get().mappings().get("fakes")).isNull(); + assertThat(loadExistingIndices()) + .doesNotContain("a", "c") + .contains("b"); } - @Test - public void delete_unknown_indice_must_delete_all_existing_indexes() { - underTest.deleteIndexes("a", "xxx", "c"); - - assertThat(es.client().nativeClient().admin().indices().prepareGetMappings().get().mappings().get("a")).isNull(); - assertThat(es.client().nativeClient().admin().indices().prepareGetMappings().get().mappings().get("c")).isNull(); + private Iterator loadExistingIndices() { + return es.client().nativeClient().admin().indices().prepareGetMappings().get().mappings().keysIt(); } - public class SimpleIndexDefinition implements IndexDefinition { + private static class SimpleIndexDefinition implements IndexDefinition { private final String indexName; public SimpleIndexDefinition(String indexName) { -- 2.39.5