CASSANDRASC-129: Remove tableId from list snapshot response (#120)
Patch by Francisco Guerrero; Reviewed by Yifan Cai for CASSANDRASC-129
diff --git a/client-common/src/main/java/org/apache/cassandra/sidecar/common/response/ListSnapshotFilesResponse.java b/client-common/src/main/java/org/apache/cassandra/sidecar/common/response/ListSnapshotFilesResponse.java
index b418a08..3700d93 100644
--- a/client-common/src/main/java/org/apache/cassandra/sidecar/common/response/ListSnapshotFilesResponse.java
+++ b/client-common/src/main/java/org/apache/cassandra/sidecar/common/response/ListSnapshotFilesResponse.java
@@ -64,7 +64,6 @@
public final String snapshotName;
public final String keySpaceName;
public final String tableName;
- public final String tableId;
public final String fileName;
private String componentDownloadUrl;
@@ -75,7 +74,6 @@
@JsonProperty("snapshotName") String snapshotName,
@JsonProperty("keySpaceName") String keySpaceName,
@JsonProperty("tableName") String tableName,
- @JsonProperty("tableId") String tableId,
@JsonProperty("fileName") String fileName)
{
this.size = size;
@@ -85,7 +83,6 @@
this.snapshotName = snapshotName;
this.keySpaceName = keySpaceName;
this.tableName = tableName;
- this.tableId = tableId;
this.fileName = fileName;
}
@@ -93,10 +90,6 @@
{
if (componentDownloadUrl == null)
{
- String tableName = this.tableId != null
- ? this.tableName + "-" + this.tableId
- : this.tableName;
-
componentDownloadUrl = ApiEndpointsV1.COMPONENTS_ROUTE
.replaceAll(ApiEndpointsV1.KEYSPACE_PATH_PARAM, keySpaceName)
.replaceAll(ApiEndpointsV1.TABLE_PATH_PARAM, tableName)
diff --git a/client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java b/client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java
index 84a5646..69459fe 100644
--- a/client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java
+++ b/client/src/testFixtures/java/org/apache/cassandra/sidecar/client/SidecarClientTest.java
@@ -498,6 +498,50 @@
+ "?includeSecondaryIndexFiles=true");
}
+ /**
+ * CASSANDRASC-94 introduced a new field ({@code tableId}) to the payload when listing snapshots. We
+ * need to make sure the client is able to handle the payload with the additional field (and ignore it).
+ *
+ * @throws Exception when the test fails
+ */
+ @Test
+ public void testListSnapshotFilesPayloadWithTableId() throws Exception
+ {
+ String responseAsString = "{\"snapshotFilesInfo\":[{\"size\":15,\"host\":\"localhost1\",\"port\":2020," +
+ "\"dataDirIndex\":1,\"snapshotName\":\"2023.04.11\",\"keySpaceName\":\"cycling\"," +
+ "\"tableName\":\"cyclist_name\",\"tableId\":\"1234\",\"fileName\":" +
+ "\"nb-203-big-TOC.txt\"}]}";
+ MockResponse response = new MockResponse().setResponseCode(OK.code()).setBody(responseAsString);
+ SidecarInstanceImpl sidecarInstance = instances.get(2);
+ MockWebServer mockWebServer = servers.get(2);
+ mockWebServer.enqueue(response);
+
+ ListSnapshotFilesResponse result = client.listSnapshotFiles(sidecarInstance,
+ "cycling",
+ "cyclist_name",
+ "2023.04.11")
+ .get(30, TimeUnit.SECONDS);
+ assertThat(result).isNotNull();
+ assertThat(result.snapshotFilesInfo()).hasSize(1);
+ ListSnapshotFilesResponse.FileInfo fileInfo = result.snapshotFilesInfo().get(0);
+ assertThat(fileInfo.size).isEqualTo(15);
+ assertThat(fileInfo.host).isEqualTo("localhost1");
+ assertThat(fileInfo.port).isEqualTo(2020);
+ assertThat(fileInfo.dataDirIndex).isEqualTo(1);
+ assertThat(fileInfo.snapshotName).isEqualTo("2023.04.11");
+ assertThat(fileInfo.keySpaceName).isEqualTo("cycling");
+ assertThat(fileInfo.tableName).isEqualTo("cyclist_name");
+ assertThat(fileInfo.fileName).isEqualTo("nb-203-big-TOC.txt");
+
+ assertThat(mockWebServer.getRequestCount()).isEqualTo(1);
+ RecordedRequest request = mockWebServer.takeRequest();
+ assertThat(request.getPath()).isEqualTo(ApiEndpointsV1.SNAPSHOTS_ROUTE
+ .replaceAll(KEYSPACE_PATH_PARAM, "cycling")
+ .replaceAll(ApiEndpointsV1.TABLE_PATH_PARAM, "cyclist_name")
+ .replaceAll(ApiEndpointsV1.SNAPSHOT_PATH_PARAM, "2023.04.11")
+ + "?includeSecondaryIndexFiles=true");
+ }
+
@Test
public void testListSnapshotFilesWithoutSecondaryIndexFiles() throws Exception
{
@@ -914,8 +958,7 @@
server.getPort(), 0,
"2023.04.12",
"cycling",
- "cyclist_name",
- "1234",
+ "cyclist_name-1234",
"nb-1-big-TOC.txt");
client.streamSSTableComponent(sidecarInstance, fileInfo, null, mockStreamConsumer);
expectedPath = fileInfo.componentDownloadUrl();
@@ -1011,8 +1054,7 @@
server.getPort(), 0,
"2023.04.12",
"cycling",
- "cyclist_name",
- "1234",
+ "cyclist_name-1234",
"nb-1-big-TOC.txt");
client.streamSSTableComponent(sidecarInstance, fileInfo, HttpRange.of(10, 20), mockStreamConsumer);
@@ -1114,8 +1156,7 @@
server.getPort(), 0,
"2023.04.12",
"cycling",
- "cyclist_name",
- "1234",
+ "cyclist_name-1234",
"nb-1-big-TOC.txt");
client.streamSSTableComponent(sidecarInstance, fileInfo, null, mockStreamConsumer);
expectedPath = fileInfo.componentDownloadUrl();
@@ -1199,8 +1240,7 @@
server.getPort(), 0,
"2023.04.12",
"cycling",
- "cyclist_name",
- "1234",
+ "cyclist_name-1234",
"nb-1-big-TOC.txt");
client.streamSSTableComponent(sidecarInstance, fileInfo, HttpRange.of(10, 20), mockStreamConsumer);
expectedPath = fileInfo.componentDownloadUrl();
diff --git a/src/main/dist/conf/sidecar.yaml b/src/main/dist/conf/sidecar.yaml
index 31310c7..ecd836c 100644
--- a/src/main/dist/conf/sidecar.yaml
+++ b/src/main/dist/conf/sidecar.yaml
@@ -26,9 +26,8 @@
username: cassandra
password: cassandra
data_dirs:
- - /ccm/test/node1/data0
- - /ccm/test/node1/data1
- staging_dir: /ccm/test/node1/sstable-staging
+ - ~/.ccm/test/node1/data0
+ staging_dir: ~/.ccm/test/node1/sstable-staging
jmx_host: 127.0.0.1
jmx_port: 7100
jmx_ssl_enabled: false
@@ -40,9 +39,8 @@
username: cassandra
password: cassandra
data_dirs:
- - /ccm/test/node2/data0
- - /ccm/test/node2/data1
- staging_dir: /ccm/test/node2/sstable-staging
+ - ~/.ccm/test/node2/data0
+ staging_dir: ~/.ccm/test/node2/sstable-staging
jmx_host: 127.0.0.1
jmx_port: 7200
jmx_ssl_enabled: false
@@ -54,9 +52,8 @@
username: cassandra
password: cassandra
data_dirs:
- - /ccm/test/node3/data0
- - /ccm/test/node3/data1
- staging_dir: /ccm/test/node3/sstable-staging
+ - ~/.ccm/test/node3/data0
+ staging_dir: ~/.ccm/test/node3/sstable-staging
jmx_host: 127.0.0.1
jmx_port: 7300
jmx_ssl_enabled: false
diff --git a/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java b/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java
index 1f90933..21cac7d 100644
--- a/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java
+++ b/src/main/java/org/apache/cassandra/sidecar/cluster/instance/InstanceMetadataImpl.java
@@ -22,12 +22,14 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
+import java.util.stream.Collectors;
import com.codahale.metrics.MetricRegistry;
import org.apache.cassandra.sidecar.cluster.CassandraAdapterDelegate;
import org.apache.cassandra.sidecar.common.DataObjectBuilder;
import org.apache.cassandra.sidecar.metrics.instance.InstanceMetrics;
import org.apache.cassandra.sidecar.metrics.instance.InstanceMetricsImpl;
+import org.apache.cassandra.sidecar.utils.FileUtils;
import org.jetbrains.annotations.Nullable;
/**
@@ -49,8 +51,10 @@
id = builder.id;
host = builder.host;
port = builder.port;
- dataDirs = Collections.unmodifiableList(builder.dataDirs);
- stagingDir = builder.stagingDir;
+ dataDirs = builder.dataDirs.stream()
+ .map(FileUtils::maybeResolveHomeDirectory)
+ .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
+ stagingDir = FileUtils.maybeResolveHomeDirectory(builder.stagingDir);
delegate = builder.delegate;
metrics = builder.metrics;
}
diff --git a/src/main/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandler.java b/src/main/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandler.java
index 0f87817..84db628 100644
--- a/src/main/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandler.java
+++ b/src/main/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandler.java
@@ -218,14 +218,16 @@
int sidecarPort = configuration.port();
ListSnapshotFilesResponse response = new ListSnapshotFilesResponse();
snapshotFileStream.forEach(file -> {
+ String tableNameAndId = file.tableId != null
+ ? request.tableName() + "-" + file.tableId
+ : request.tableName();
response.addSnapshotFile(new ListSnapshotFilesResponse.FileInfo(file.size,
host,
sidecarPort,
file.dataDirectoryIndex,
request.snapshotName(),
request.keyspace(),
- request.tableName(),
- file.tableId,
+ tableNameAndId,
file.name));
});
return Future.succeededFuture(response);
diff --git a/src/main/java/org/apache/cassandra/sidecar/utils/FileUtils.java b/src/main/java/org/apache/cassandra/sidecar/utils/FileUtils.java
new file mode 100644
index 0000000..51172e8
--- /dev/null
+++ b/src/main/java/org/apache/cassandra/sidecar/utils/FileUtils.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.sidecar.utils;
+
+/**
+ * Encompasses utilities for files
+ */
+public class FileUtils
+{
+ /**
+ * Resolves the home directory from the input {@code directory} string when the string begins with {@code ~}.
+ *
+ * @param directory the directory path
+ * @return the resolved directory path, replacing the user's home directory when the string begins with {@code ~}
+ */
+ public static String maybeResolveHomeDirectory(String directory)
+ {
+ if (directory == null || !directory.startsWith("~"))
+ return directory;
+ return System.getProperty("user.home") + directory.substring(1);
+ }
+}
diff --git a/src/test/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandlerTest.java b/src/test/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandlerTest.java
index 7c64d36..f98574f 100644
--- a/src/test/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandlerTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/routes/snapshots/ListSnapshotHandlerTest.java
@@ -121,8 +121,7 @@
0,
"snapshot1",
"keyspace1",
- "table1",
- "1234",
+ "table1-1234",
"1.db");
ListSnapshotFilesResponse.FileInfo fileInfoNotExpected =
new ListSnapshotFilesResponse.FileInfo(11,
@@ -131,8 +130,7 @@
0,
"snapshot1",
"keyspace1",
- "table1",
- "1234",
+ "table1-1234",
"2.db");
client.get(server.actualPort(), "localhost", testRoute)
@@ -159,8 +157,7 @@
0,
"snapshot1",
"keyspace1",
- "table1",
- "1234",
+ "table1-1234",
"1.db"),
new ListSnapshotFilesResponse.FileInfo(0,
"localhost",
@@ -168,8 +165,7 @@
0,
"snapshot1",
"keyspace1",
- "table1",
- "1234",
+ "table1-1234",
".index/secondary.db")
);
ListSnapshotFilesResponse.FileInfo fileInfoNotExpected =
@@ -179,8 +175,7 @@
0,
"snapshot1",
"keyspace1",
- "table1",
- "1234",
+ "table1-1234",
"2.db");
client.get(server.actualPort(), "localhost", testRoute)
diff --git a/src/test/java/org/apache/cassandra/sidecar/utils/FileUtilsTest.java b/src/test/java/org/apache/cassandra/sidecar/utils/FileUtilsTest.java
new file mode 100644
index 0000000..eba7be1
--- /dev/null
+++ b/src/test/java/org/apache/cassandra/sidecar/utils/FileUtilsTest.java
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.cassandra.sidecar.utils;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Unit tests for {@link FileUtils}
+ */
+class FileUtilsTest
+{
+ @Test
+ void testMaybeResolveHomeDirectory()
+ {
+ assertThat(FileUtils.maybeResolveHomeDirectory(null)).isNull();
+ assertThat(FileUtils.maybeResolveHomeDirectory("")).isEqualTo("");
+ assertThat(FileUtils.maybeResolveHomeDirectory("~")).isEqualTo(System.getProperty("user.home"));
+ assertThat(FileUtils.maybeResolveHomeDirectory("~/")).isEqualTo(System.getProperty("user.home") + "/");
+ assertThat(FileUtils.maybeResolveHomeDirectory("~/.ccm")).isEqualTo(System.getProperty("user.home") + "/.ccm");
+ assertThat(FileUtils.maybeResolveHomeDirectory("/dev/null")).isEqualTo("/dev/null");
+ }
+}
