Simplify BwC for UUIDs in RepositoryData

plugins/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3BlobStoreRepositoryTests.java

@@ -147,7 +147,7 @@ public void testEnforcedCooldownPeriod() throws IOException {
         final BlobStoreRepository repository = (BlobStoreRepository) repositoriesService.repository(repoName);
         final RepositoryData repositoryData = getRepositoryData(repository);
         final RepositoryData modifiedRepositoryData = repositoryData.withVersions(Collections.singletonMap(fakeOldSnapshot,
-            SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.minimumCompatibilityVersion())).withoutRepositoryUUID().withoutClusterUUID();
+            SnapshotsService.SHARD_GEN_IN_REPO_DATA_VERSION.minimumCompatibilityVersion())).withoutUUIDs();
         final BytesReference serialized = BytesReference.bytes(modifiedRepositoryData.snapshotsToXContent(XContentFactory.jsonBuilder(),
             SnapshotsService.OLD_SNAPSHOT_FORMAT));
         PlainActionFuture.get(f -> repository.threadPool().generic().execute(ActionRunnable.run(f, () ->

qa/repository-multi-version/src/test/java/org/elasticsearch/upgrades/MultiVersionRepositoryAccessIT.java

@@ -197,7 +197,7 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException {
             // 7.12.0+ will try to load RepositoryData during repo creation if verify is true, which is impossible in case of version
             // incompatibility in the downgrade test step. We verify that it is impossible here and then create the repo using verify=false
             // to check behavior on other operations below.
-            final boolean verify = TEST_STEP != TestStep.STEP3_OLD_CLUSTER || SnapshotsService.includesClusterUUID(minNodeVersion)
+            final boolean verify = TEST_STEP != TestStep.STEP3_OLD_CLUSTER || SnapshotsService.includesUUIDs(minNodeVersion)
                     || minNodeVersion.before(Version.V_7_12_0);
             if (verify == false) {
                 expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> createRepository(client, repoName, false, true));
@@ -223,7 +223,7 @@ public void testUpgradeMovesRepoToNewMetaVersion() throws IOException {
                     ensureSnapshotRestoreWorks(client, repoName, "snapshot-2", shards);
                 }
             } else {
-                if (SnapshotsService.includesClusterUUID(minNodeVersion) == false) {
+                if (SnapshotsService.includesUUIDs(minNodeVersion) == false) {
                     assertThat(TEST_STEP, is(TestStep.STEP3_OLD_CLUSTER));
                     expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> listSnapshots(repoName));
                     expectThrowsAnyOf(EXPECTED_BWC_EXCEPTIONS, () -> deleteSnapshot(client, repoName, "snapshot-1"));

server/src/main/java/org/elasticsearch/cluster/metadata/RepositoryMetadata.java

@@ -84,7 +84,7 @@ public String type() {
     /**
      * Return the repository UUID, if set and known. The repository UUID is stored in the repository and typically populated here when the
      * repository is registered or when we write to it. It may not be set if the repository is maintaining support for versions before
-     * {@link SnapshotsService#REPOSITORY_UUID_IN_REPO_DATA_VERSION}. It may not be known if the repository was registered with {@code
+     * {@link SnapshotsService#UUIDS_IN_REPO_DATA_VERSION}. It may not be known if the repository was registered with {@code
      * ?verify=false} and has had no subsequent writes. Consumers may, if desired, try and fill in a missing value themselves by retrieving
      * the {@link RepositoryData} and calling {@link org.elasticsearch.repositories.RepositoriesService#updateRepositoryUuidInMetadata}.
      *
@@ -129,7 +129,7 @@ public long pendingGeneration() {
 
     public RepositoryMetadata(StreamInput in) throws IOException {
         name = in.readString();
-        if (in.getVersion().onOrAfter(SnapshotsService.REPOSITORY_UUID_IN_REPO_DATA_VERSION)) {
+        if (in.getVersion().onOrAfter(SnapshotsService.UUIDS_IN_REPO_DATA_VERSION)) {
             uuid = in.readString();
         } else {
             uuid = RepositoryData.MISSING_UUID;
@@ -148,7 +148,7 @@ public RepositoryMetadata(StreamInput in) throws IOException {
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeString(name);
-        if (out.getVersion().onOrAfter(SnapshotsService.REPOSITORY_UUID_IN_REPO_DATA_VERSION)) {
+        if (out.getVersion().onOrAfter(SnapshotsService.UUIDS_IN_REPO_DATA_VERSION)) {
             out.writeString(uuid);
         }
         out.writeString(type);

server/src/main/java/org/elasticsearch/repositories/RepositoryData.java

@@ -164,7 +164,9 @@ private RepositoryData(
         this.shardGenerations = shardGenerations;
         this.indexMetaDataGenerations = indexMetaDataGenerations;
         this.snapshotVersions = snapshotVersions;
-        this.clusterUUID = clusterUUID;
+        this.clusterUUID = Objects.requireNonNull(clusterUUID);
+        assert uuid.equals(MISSING_UUID) == clusterUUID.equals(MISSING_UUID) : "Either repository- and cluster UUID must both be missing" +
+                " or neither of them must be missing but saw [" + uuid + "][" + clusterUUID + "]";
         assert indices.values().containsAll(shardGenerations.indices()) : "ShardGenerations contained indices "
                 + shardGenerations.indices() + " but snapshots only reference indices " + indices.values();
         assert indexSnapshots.values().stream().noneMatch(snapshotIdList -> Set.copyOf(snapshotIdList).size() != snapshotIdList.size()) :
@@ -215,15 +217,15 @@ public ShardGenerations shardGenerations() {
 
     /**
      * @return The UUID of this repository, or {@link RepositoryData#MISSING_UUID} if this repository has no UUID because it still
-     * supports access from versions earlier than {@link SnapshotsService#REPOSITORY_UUID_IN_REPO_DATA_VERSION}.
+     * supports access from versions earlier than {@link SnapshotsService#UUIDS_IN_REPO_DATA_VERSION}.
      */
     public String getUuid() {
         return uuid;
     }
 
     /**
      * @return the cluster UUID of the cluster that wrote this instance to the repository or {@link RepositoryData#MISSING_UUID} if this
-     * instance was written by a cluster older than {@link SnapshotsService#CLUSTER_UUID_IN_REPO_DATA_VERSION}.
+     * instance was written by a cluster older than {@link SnapshotsService#UUIDS_IN_REPO_DATA_VERSION}.
      */
     public String getClusterUUID() {
         return clusterUUID;
@@ -404,28 +406,10 @@ public RepositoryData withGenId(long newGeneration) {
     }
 
     /**
-     * Make a copy of this instance with the given UUID and all other fields unchanged.
-     */
-    public RepositoryData withUuid(String uuid) {
-        assert this.uuid.equals(MISSING_UUID) : this.uuid;
-        assert uuid.equals(MISSING_UUID) == false;
-        return new RepositoryData(
-                uuid,
-                genId,
-                snapshotIds,
-                snapshotStates,
-                snapshotVersions,
-                indices,
-                indexSnapshots,
-                shardGenerations,
-                indexMetaDataGenerations,
-                clusterUUID);
-    }
-
-    /**
-     * For test purposes, make a copy of this instance with the UUID removed and all other fields unchanged, as if from an older version.
+     * For test purposes, make a copy of this instance with the cluster- and repository UUIDs removed and all other fields unchanged,
+     * as if from an older version.
      */
-    public RepositoryData withoutRepositoryUUID() {
+    public RepositoryData withoutUUIDs() {
         return new RepositoryData(
                 MISSING_UUID,
                 genId,
@@ -436,31 +420,13 @@ public RepositoryData withoutRepositoryUUID() {
                 indexSnapshots,
                 shardGenerations,
                 indexMetaDataGenerations,
-                clusterUUID);
-    }
-
-    /**
-     * For test purposes, make a copy of this instance with the cluster UUID removed and all other fields unchanged, as if from an older
-     * version.
-     */
-    public RepositoryData withoutClusterUUID() {
-        return new RepositoryData(
-                uuid,
-                genId,
-                snapshotIds,
-                snapshotStates,
-                snapshotVersions,
-                indices,
-                indexSnapshots,
-                shardGenerations,
-                indexMetaDataGenerations,
                 MISSING_UUID);
     }
 
     public RepositoryData withClusterUuid(String clusterUUID) {
         assert clusterUUID.equals(MISSING_UUID) == false;
         return new RepositoryData(
-                uuid,
+                uuid.equals(MISSING_UUID) ? UUIDs.randomBase64UUID() : uuid,
                 genId,
                 snapshotIds,
                 snapshotStates,
@@ -472,24 +438,6 @@ public RepositoryData withClusterUuid(String clusterUUID) {
                 clusterUUID);
     }
 
-    /**
-     * For test purposes, make a copy of this instance with the cluster UUID removed and all other fields unchanged,
-     * as if from an older version.
-     */
-    public RepositoryData withoutClusterUuid() {
-        return new RepositoryData(
-                uuid,
-                genId,
-                snapshotIds,
-                snapshotStates,
-                snapshotVersions,
-                indices,
-                indexSnapshots,
-                shardGenerations,
-                indexMetaDataGenerations,
-                MISSING_UUID);
-    }
-
     /**
      * Remove snapshots and remove any indices that no longer exist in the repository due to the deletion of the snapshots.
      *
@@ -647,24 +595,20 @@ public XContentBuilder snapshotsToXContent(
             final Version repoMetaVersion,
             boolean permitMissingUuid) throws IOException {
 
-        final boolean shouldWriteClusterId = SnapshotsService.includesClusterUUID(repoMetaVersion);
-        final boolean shouldWriteRepoUuid = SnapshotsService.includesRepositoryUuid(repoMetaVersion);
+        final boolean shouldWriteUUIDS = SnapshotsService.includesUUIDs(repoMetaVersion);
         final boolean shouldWriteIndexGens = SnapshotsService.useIndexGenerations(repoMetaVersion);
         final boolean shouldWriteShardGens = SnapshotsService.useShardGenerations(repoMetaVersion);
 
-        assert Boolean.compare(shouldWriteClusterId, shouldWriteRepoUuid) <= 0;
-        assert Boolean.compare(shouldWriteRepoUuid, shouldWriteIndexGens) <= 0;
+        assert Boolean.compare(shouldWriteUUIDS, shouldWriteIndexGens) <= 0;
         assert Boolean.compare(shouldWriteIndexGens, shouldWriteShardGens) <= 0;
 
         builder.startObject();
 
         if (shouldWriteShardGens) {
             // Add min version field to make it impossible for older ES versions to deserialize this object
             final Version minVersion;
-            if (shouldWriteClusterId) {
-                minVersion = SnapshotsService.CLUSTER_UUID_IN_REPO_DATA_VERSION;
-            } else if (shouldWriteRepoUuid) {
-                minVersion = SnapshotsService.REPOSITORY_UUID_IN_REPO_DATA_VERSION;
+            if (shouldWriteUUIDS) {
+                minVersion = SnapshotsService.UUIDS_IN_REPO_DATA_VERSION;
             } else if (shouldWriteIndexGens) {
                 minVersion = SnapshotsService.INDEX_GEN_IN_REPO_DATA_VERSION;
             } else {
@@ -673,23 +617,19 @@ public XContentBuilder snapshotsToXContent(
             builder.field(MIN_VERSION, minVersion.toString());
         }
 
-        if (shouldWriteRepoUuid) {
+        if (shouldWriteUUIDS) {
             if (uuid.equals(MISSING_UUID)) {
                 assert permitMissingUuid : "missing uuid";
             } else {
                 builder.field(UUID, uuid);
             }
-        } else {
-            assert uuid.equals(MISSING_UUID) : "lost uuid " + uuid;
-        }
-
-        if (shouldWriteClusterId) {
             if (clusterUUID.equals(MISSING_UUID)) {
                 assert permitMissingUuid : "missing clusterUUID";
             } else {
                 builder.field(CLUSTER_UUID, clusterUUID);
             }
         } else {
+            assert uuid.equals(MISSING_UUID) : "lost uuid " + uuid;
             assert clusterUUID.equals(MISSING_UUID) : "lost clusterUUID " + clusterUUID;
         }
 

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

@@ -138,7 +138,6 @@
 import java.util.stream.Stream;
 
 import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo.canonicalName;
-import static org.elasticsearch.repositories.RepositoryData.MISSING_UUID;
 
 /**
  * BlobStore - based implementation of Snapshot Repository
@@ -1906,17 +1905,13 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS
     }
 
     private RepositoryData updateRepositoryData(RepositoryData repositoryData, Version repositoryMetaversion, long newGen) {
-        if (SnapshotsService.includesClusterUUID(repositoryMetaversion)) {
+        if (SnapshotsService.includesUUIDs(repositoryMetaversion)) {
             final String clusterUUID = clusterService.state().metadata().clusterUUID();
             if (repositoryData.getClusterUUID().equals(clusterUUID) == false) {
                 repositoryData = repositoryData.withClusterUuid(clusterUUID);
             }
         }
-        if (SnapshotsService.includesRepositoryUuid(repositoryMetaversion) && repositoryData.getUuid().equals(MISSING_UUID)) {
-            return repositoryData.withGenId(newGen).withUuid(UUIDs.randomBase64UUID());
-        } else {
-            return repositoryData.withGenId(newGen);
-        }
+        return repositoryData.withGenId(newGen);
     }
 
     /**

server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java

@@ -124,10 +124,7 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus
 
     public static final Version INDEX_GEN_IN_REPO_DATA_VERSION = Version.V_7_9_0;
 
-    public static final Version REPOSITORY_UUID_IN_REPO_DATA_VERSION = Version.V_7_12_0;
-
-    // TODO: fold this into #REPOSITORY_UUID_IN_REPO_DATA_VERSION and remove separate handling of uuid and clusterUUID BwC where possible
-    public static final Version CLUSTER_UUID_IN_REPO_DATA_VERSION = Version.V_7_12_0;
+    public static final Version UUIDS_IN_REPO_DATA_VERSION = Version.V_7_12_0;
 
     public static final Version OLD_SNAPSHOT_FORMAT = Version.V_7_5_0;
 
@@ -1890,23 +1887,13 @@ public static boolean useIndexGenerations(Version repositoryMetaVersion) {
     }
 
     /**
-     * Checks whether the metadata version supports writing a repository uuid to the repository.
-     *
-     * @param repositoryMetaVersion version to check
-     * @return true if version supports writing repository uuid to the repository
-     */
-    public static boolean includesRepositoryUuid(Version repositoryMetaVersion) {
-        return repositoryMetaVersion.onOrAfter(REPOSITORY_UUID_IN_REPO_DATA_VERSION);
-    }
-
-    /**
-     * Checks whether the metadata version supports writing the cluster uuid to the repository.
+     * Checks whether the metadata version supports writing the cluster- and repository-uuid to the repository.
      *
      * @param repositoryMetaVersion version to check
-     * @return true if version supports {@link ShardGenerations}
+     * @return true if version supports writing cluster- and repository-uuid to the repository
      */
-    public static boolean includesClusterUUID(Version repositoryMetaVersion) {
-        return repositoryMetaVersion.onOrAfter(CLUSTER_UUID_IN_REPO_DATA_VERSION);
+    public static boolean includesUUIDs(Version repositoryMetaVersion) {
+        return repositoryMetaVersion.onOrAfter(UUIDS_IN_REPO_DATA_VERSION);
     }
 
     /** Deletes snapshot from repository

server/src/test/java/org/elasticsearch/repositories/RepositoryDataTests.java

@@ -65,8 +65,7 @@ public void testIndicesToUpdateAfterRemovingSnapshot() {
     }
 
     public void testXContent() throws IOException {
-        RepositoryData repositoryData =
-                generateRandomRepoData().withUuid(UUIDs.randomBase64UUID(random())).withClusterUuid(UUIDs.randomBase64UUID(random()));
+        RepositoryData repositoryData = generateRandomRepoData().withClusterUuid(UUIDs.randomBase64UUID(random()));
         XContentBuilder builder = JsonXContent.contentBuilder();
         repositoryData.snapshotsToXContent(builder, Version.CURRENT);
         try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder))) {
@@ -144,7 +143,7 @@ public void testInitIndices() {
         // test that initializing indices works
         Map<IndexId, List<SnapshotId>> indices = randomIndices(snapshotIds);
         RepositoryData newRepoData = new RepositoryData(
-                repositoryData.getUuid(),
+                UUIDs.randomBase64UUID(random()),
                 repositoryData.getGenId(),
                 snapshotIds,
                 snapshotStates,
@@ -198,8 +197,7 @@ public void testGetSnapshotState() {
 
     public void testIndexThatReferencesAnUnknownSnapshot() throws IOException {
         final XContent xContent = randomFrom(XContentType.values()).xContent();
-        final RepositoryData repositoryData =
-                generateRandomRepoData().withUuid(UUIDs.randomBase64UUID()).withClusterUuid(UUIDs.randomBase64UUID(random()));
+        final RepositoryData repositoryData = generateRandomRepoData().withClusterUuid(UUIDs.randomBase64UUID(random()));
 
         XContentBuilder builder = XContentBuilder.builder(xContent);
         repositoryData.snapshotsToXContent(builder, Version.CURRENT);

test/framework/src/main/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java

@@ -148,10 +148,8 @@ protected void disableRepoConsistencyCheck(String reason) {
 
     protected RepositoryData getRepositoryData(String repoName, Version version) {
         final RepositoryData repositoryData = getRepositoryData(repoName);
-        if (SnapshotsService.includesRepositoryUuid(version) == false) {
-            return repositoryData.withoutRepositoryUUID().withoutClusterUUID();
-        } else if (SnapshotsService.includesClusterUUID(version) == false) {
-            return repositoryData.withoutClusterUuid();
+        if (SnapshotsService.includesUUIDs(version) == false) {
+            return repositoryData.withoutUUIDs();
         } else {
             return repositoryData;
         }