This is an automated email from the ASF dual-hosted git repository. frankgh pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
The following commit(s) were added to refs/heads/trunk by this push: new e95786a CASSANDRASC-129: Remove tableId from list snapshot response (#120) e95786a is described below commit e95786a077e1137dcaae206854986987edc6a71e Author: Francisco Guerrero <fran...@apache.org> AuthorDate: Wed May 8 18:04:05 2024 -0700 CASSANDRASC-129: Remove tableId from list snapshot response (#120) Patch by Francisco Guerrero; Reviewed by Yifan Cai for CASSANDRASC-129 --- .../common/response/ListSnapshotFilesResponse.java | 7 --- .../sidecar/client/SidecarClientTest.java | 56 ++++++++++++++++++---- src/main/dist/conf/sidecar.yaml | 15 +++--- .../cluster/instance/InstanceMetadataImpl.java | 8 +++- .../routes/snapshots/ListSnapshotHandler.java | 6 ++- .../apache/cassandra/sidecar/utils/FileUtils.java | 38 +++++++++++++++ .../routes/snapshots/ListSnapshotHandlerTest.java | 15 ++---- .../cassandra/sidecar/utils/FileUtilsTest.java | 40 ++++++++++++++++ 8 files changed, 147 insertions(+), 38 deletions(-) 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 class ListSnapshotFilesResponse 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 @@ public class ListSnapshotFilesResponse @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 @@ public class ListSnapshotFilesResponse this.snapshotName = snapshotName; this.keySpaceName = keySpaceName; this.tableName = tableName; - this.tableId = tableId; this.fileName = fileName; } @@ -93,10 +90,6 @@ public class ListSnapshotFilesResponse { 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 @@ abstract class SidecarClientTest + "?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 @@ abstract class SidecarClientTest 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 @@ abstract class SidecarClientTest 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 @@ abstract class SidecarClientTest 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 @@ abstract class SidecarClientTest 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 @@ cassandra_instances: 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 @@ cassandra_instances: 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 @@ cassandra_instances: 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.ArrayList; 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 @@ public class InstanceMetadataImpl implements InstanceMetadata 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 @@ public class ListSnapshotHandler extends AbstractHandler<SnapshotRequestParam> 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 @@ class ListSnapshotHandlerTest 0, "snapshot1", "keyspace1", - "table1", - "1234", + "table1-1234", "1.db"); ListSnapshotFilesResponse.FileInfo fileInfoNotExpected = new ListSnapshotFilesResponse.FileInfo(11, @@ -131,8 +130,7 @@ class ListSnapshotHandlerTest 0, "snapshot1", "keyspace1", - "table1", - "1234", + "table1-1234", "2.db"); client.get(server.actualPort(), "localhost", testRoute) @@ -159,8 +157,7 @@ class ListSnapshotHandlerTest 0, "snapshot1", "keyspace1", - "table1", - "1234", + "table1-1234", "1.db"), new ListSnapshotFilesResponse.FileInfo(0, "localhost", @@ -168,8 +165,7 @@ class ListSnapshotHandlerTest 0, "snapshot1", "keyspace1", - "table1", - "1234", + "table1-1234", ".index/secondary.db") ); ListSnapshotFilesResponse.FileInfo fileInfoNotExpected = @@ -179,8 +175,7 @@ class ListSnapshotHandlerTest 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"); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org