From d5077435f5c5ff21f79abef4d5b2f6dd5bef808d Mon Sep 17 00:00:00 2001 From: Thach Le Date: Sat, 22 Jun 2024 16:35:48 +0700 Subject: [PATCH] Remove not readonly commands #2832 (#2871) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove not readonly commands #2832 * Fix comment Add test * Update test * Update cicd.yaml workaround for apt update issue related to Clearsigned file * GitHub issue template polishing and stale issues action (#2833) * GitHub issue template polishing and stale issues action * Addressing feedback on message test and a better label for the stale issues * Implement HEXPIRE, HEXPIREAT, HEXPIRETIME and HPERSIST (#2836) * HEXPIRE implemented with integration tests * Polishing to integration test, added unit test * Move new commands to RedisHashCommands, added HEXPIREAT, HEXPIRETIME and HPERSIST * Make sure we reset the configuration setting after the new hash commands were tested * Broke one test because of wrong configuration setting; the other is unstable, trying to fix it * Polishing imports * Polishin : Copyright change not needed * MAke sure we wait for the check so it does not fail on the slower pipeline (#2844) * Add support for `SPUBLISH` (#2838) * implementation of SPUBLISH * sort methods by name * test cluster spublish with no redirects * use injected cluster client in RedisClusterPubSubConnectionIntegrationTests * Applying code formatter each time we run a Maven build (#2841) * Let's try this again * Add tests and templates * Merge formatting issues + formatter using wrong configuration * Update cicd.yaml the issue related apt repo has been fixed https://github.com/orgs/community/discussions/120966#discussioncomment-9211925 * Add support CLIENT KILL [MAXAGE] (#2782) * Add support CLIENT KILL [MAXAGE] * polish * address review changes * format code * Add support for `SUNSUBSCRIBE` #2759 (#2851) * Add support for `SUNSUBSCRIBE` #2759 * replace junit.Assert with assertj * Mark dnsResolver(DnsResolver) as deprecated. (#2855) * Implement HPEXPIRE, HPEXPIREAT, HPEXPIRETIME, HTTL and HPTTL (#2857) * Fixes to the hash field expiration functions * Implemented HPEXPIRE, HPEXPIREAT, HPEXPIRETIME, HTTL, HPTTL * Format the files * FIELDS keyword was introduced as per the PRD * Extend tests to include HTTL * Repair unit tests, add new ones for the new commands * Disable failing test, becasue it is very unstable * Modify return value to list of long values, fix cluster logic * Polishing * Polishing - addressed Ali's comments * Polishing: Modified by mistake * Polishing : Addressed Marks' comments * XREAD support for reading last message from stream (#2863) * Add last() utility method to the XArgs.StreamOffset * Submitted one more file by mistake * Add a `evalReadOnly` overload that accepts the script as a `String` (#2868) * Add a evalReadOnly overload that accepts the script as a String * Fix @since annotation for 7.0 * Add release drafter workflow (#2820) * Release-drafter, dependabot, OCD polishing * Commitish not needed and pointing to the wrong branch anyway, also wrong version chosen * Pipeline is also very confusing as there are many pipelines, this is the INTEGRATION pipeline * Adding commitish, as it is required for filter-by-commitish * Autolabeling hits issues when using pull_request, using pull_request_target instead * pull_request_target does nothing, trying the other suggestion from https://github.com/release-drafter/release-drafter/issues/1125 * Bump org.apache.maven.plugins:maven-javadoc-plugin from 3.6.3 to 3.7.0 (#2873) Bumps [org.apache.maven.plugins:maven-javadoc-plugin](https://github.com/apache/maven-javadoc-plugin) from 3.6.3 to 3.7.0. - [Release notes](https://github.com/apache/maven-javadoc-plugin/releases) - [Commits](https://github.com/apache/maven-javadoc-plugin/compare/maven-javadoc-plugin-3.6.3...maven-javadoc-plugin-3.7.0) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-javadoc-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump org.codehaus.mojo:flatten-maven-plugin from 1.5.0 to 1.6.0 (#2874) Bumps [org.codehaus.mojo:flatten-maven-plugin](https://github.com/mojohaus/flatten-maven-plugin) from 1.5.0 to 1.6.0. - [Release notes](https://github.com/mojohaus/flatten-maven-plugin/releases) - [Commits](https://github.com/mojohaus/flatten-maven-plugin/compare/1.5.0...1.6.0) --- updated-dependencies: - dependency-name: org.codehaus.mojo:flatten-maven-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump org.apache.maven.plugins:maven-jar-plugin from 3.3.0 to 3.4.1 (#2875) Bumps [org.apache.maven.plugins:maven-jar-plugin](https://github.com/apache/maven-jar-plugin) from 3.3.0 to 3.4.1. - [Release notes](https://github.com/apache/maven-jar-plugin/releases) - [Commits](https://github.com/apache/maven-jar-plugin/compare/maven-jar-plugin-3.3.0...maven-jar-plugin-3.4.1) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-jar-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump org.openjdk.jmh:jmh-generator-annprocess from 1.21 to 1.37 (#2876) Bumps [org.openjdk.jmh:jmh-generator-annprocess](https://github.com/openjdk/jmh) from 1.21 to 1.37. - [Commits](https://github.com/openjdk/jmh/compare/1.21...1.37) --- updated-dependencies: - dependency-name: org.openjdk.jmh:jmh-generator-annprocess dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump org.apache.commons:commons-pool2 from 2.11.1 to 2.12.0 (#2877) Bumps org.apache.commons:commons-pool2 from 2.11.1 to 2.12.0. --- updated-dependencies: - dependency-name: org.apache.commons:commons-pool2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Setting the next release to be 6.4.x as part of #2880 (#2881) * Bump version of lettuce-core to 6.4.0 * Change all @since notations to match the next release 6.4.x and also fixed two typos in the API JavaDoc * Fixed formatting issues * Modify the release acrtion to call the proper maven target for release, make releasing manually available too (#2885) * Format file * Using interface method from java 8 instead of java 9 * Fix test * Revert readwrite command to the list COMMAND INFO READWRITE 1) 1) "readwrite" 2) "1" 3) 1) "loading" 2) "stale" 3) "fast" --------- Signed-off-by: dependabot[bot] Co-authored-by: atakavci <58048133+atakavci@users.noreply.github.com> Co-authored-by: Tihomir Krasimirov Mateev Co-authored-by: Liming Deng Co-authored-by: Wang Zhi Co-authored-by: Luis Miguel Mejía Suárez Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .../core/masterreplica/ReadOnlyCommands.java | 72 ------------------- .../core/protocol/ReadOnlyCommands.java | 2 +- .../ClusterReadOnlyCommandsUnitTests.java | 2 +- .../commands/HashCommandIntegrationTests.java | 4 -- .../ServerCommandIntegrationTests.java | 24 +++++++ 5 files changed, 26 insertions(+), 78 deletions(-) delete mode 100644 src/main/java/io/lettuce/core/masterreplica/ReadOnlyCommands.java diff --git a/src/main/java/io/lettuce/core/masterreplica/ReadOnlyCommands.java b/src/main/java/io/lettuce/core/masterreplica/ReadOnlyCommands.java deleted file mode 100644 index 67616a7ef5..0000000000 --- a/src/main/java/io/lettuce/core/masterreplica/ReadOnlyCommands.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2020-Present, Redis Ltd. and Contributors - * All rights reserved. - * - * Licensed under the MIT License. - * - * This file contains contributions from third-party contributors - * licensed 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 - * - * https://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 io.lettuce.core.masterreplica; - -import java.util.Collections; -import java.util.EnumSet; -import java.util.Set; - -import io.lettuce.core.protocol.CommandType; -import io.lettuce.core.protocol.ProtocolKeyword; - -/** - * Contains all command names that are read-only commands. - * - * @author Mark Paluch - */ -class ReadOnlyCommands { - - private static final Set READ_ONLY_COMMANDS = EnumSet.noneOf(CommandType.class); - - static { - for (CommandName commandNames : CommandName.values()) { - READ_ONLY_COMMANDS.add(CommandType.valueOf(commandNames.name())); - } - } - - /** - * @param protocolKeyword must not be {@code null}. - * @return {@code true} if {@link ProtocolKeyword} is a read-only command. - */ - public static boolean isReadOnlyCommand(ProtocolKeyword protocolKeyword) { - return READ_ONLY_COMMANDS.contains(protocolKeyword); - } - - /** - * @return an unmodifiable {@link Set} of {@link CommandType read-only} commands. - */ - public static Set getReadOnlyCommands() { - return Collections.unmodifiableSet(READ_ONLY_COMMANDS); - } - - enum CommandName { - ASKING, BITCOUNT, BITPOS, CLIENT, COMMAND, DUMP, ECHO, EVAL_RO, EVALSHA_RO, EXISTS, FCALL_RO, // - GEODIST, GEOPOS, GEORADIUS, GEORADIUS_RO, GEORADIUSBYMEMBER, GEORADIUSBYMEMBER_RO, GEOSEARCH, GEOHASH, GET, GETBIT, // - GETRANGE, HEXISTS, HGET, HGETALL, HKEYS, HLEN, HMGET, HRANDFIELD, HSCAN, HSTRLEN, // - HVALS, INFO, KEYS, LINDEX, LLEN, LPOS, LRANGE, SORT_RO, MGET, PFCOUNT, PTTL, // - RANDOMKEY, READWRITE, SCAN, SCARD, SCRIPT, // - SDIFF, SINTER, SISMEMBER, SMISMEMBER, SMEMBERS, SRANDMEMBER, SSCAN, STRLEN, // - SUNION, TIME, TTL, TYPE, // - XINFO, XLEN, XPENDING, XRANGE, XREVRANGE, XREAD, // - ZCARD, ZCOUNT, ZLEXCOUNT, ZRANGE, // - ZRANDMEMBER, ZRANGEBYLEX, ZRANGEBYSCORE, ZRANK, ZREVRANGE, ZREVRANGEBYLEX, ZREVRANGEBYSCORE, ZREVRANK, ZSCAN, ZSCORE, - } - -} diff --git a/src/main/java/io/lettuce/core/protocol/ReadOnlyCommands.java b/src/main/java/io/lettuce/core/protocol/ReadOnlyCommands.java index a6bfc6333f..81671bf761 100644 --- a/src/main/java/io/lettuce/core/protocol/ReadOnlyCommands.java +++ b/src/main/java/io/lettuce/core/protocol/ReadOnlyCommands.java @@ -71,7 +71,7 @@ public static ReadOnlyPredicate asPredicate() { enum CommandName { ASKING, BITCOUNT, BITPOS, CLIENT, COMMAND, DUMP, ECHO, EVAL_RO, EVALSHA_RO, EXISTS, FCALL_RO, // - GEODIST, GEOPOS, GEORADIUS, GEORADIUS_RO, GEORADIUSBYMEMBER, GEORADIUSBYMEMBER_RO, GEOSEARCH, GEOHASH, GET, GETBIT, // + GEODIST, GEOPOS, GEORADIUS_RO, GEORADIUSBYMEMBER_RO, GEOSEARCH, GEOHASH, GET, GETBIT, // GETRANGE, HEXISTS, HGET, HGETALL, HKEYS, HLEN, HMGET, HRANDFIELD, HSCAN, HSTRLEN, // HVALS, INFO, KEYS, LINDEX, LLEN, LPOS, LRANGE, SORT_RO, MGET, PFCOUNT, PTTL, // RANDOMKEY, READWRITE, SCAN, SCARD, SCRIPT, // diff --git a/src/test/java/io/lettuce/core/cluster/ClusterReadOnlyCommandsUnitTests.java b/src/test/java/io/lettuce/core/cluster/ClusterReadOnlyCommandsUnitTests.java index b1f6cad9ff..502198b7ae 100644 --- a/src/test/java/io/lettuce/core/cluster/ClusterReadOnlyCommandsUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/ClusterReadOnlyCommandsUnitTests.java @@ -16,7 +16,7 @@ class ClusterReadOnlyCommandsUnitTests { @Test void testCount() { - assertThat(ClusterReadOnlyCommands.getReadOnlyCommands()).hasSize(86); + assertThat(ClusterReadOnlyCommands.getReadOnlyCommands()).hasSize(84); } @Test diff --git a/src/test/java/io/lettuce/core/commands/HashCommandIntegrationTests.java b/src/test/java/io/lettuce/core/commands/HashCommandIntegrationTests.java index 4984914641..2cd1608e4a 100644 --- a/src/test/java/io/lettuce/core/commands/HashCommandIntegrationTests.java +++ b/src/test/java/io/lettuce/core/commands/HashCommandIntegrationTests.java @@ -561,7 +561,6 @@ void hexpire() { assertThat(redis.hset(MY_KEY, MY_FIELD, MY_VALUE)).isTrue(); assertThat(redis.hexpire(MY_KEY, 1, MY_FIELD, MY_SECOND_FIELD)).containsExactly(1L, -2L); assertThat(redis.hexpire("invalidKey", 1, MY_FIELD)).containsExactly(-2L); - await().until(() -> redis.hget(MY_KEY, MY_FIELD) == null); } @@ -573,7 +572,6 @@ void hexpireExpireArgs() { assertThat(redis.hexpire(MY_KEY, Duration.ofSeconds(1), ExpireArgs.Builder.xx(), MY_FIELD)).containsExactly(1L); assertThat(redis.hexpire(MY_KEY, Duration.ofSeconds(10), ExpireArgs.Builder.gt(), MY_FIELD)).containsExactly(1L); assertThat(redis.hexpire(MY_KEY, Duration.ofSeconds(1), ExpireArgs.Builder.lt(), MY_FIELD)).containsExactly(1L); - await().until(() -> redis.hget(MY_KEY, MY_FIELD) == null); } @@ -585,7 +583,6 @@ void hexpireat() { assertThat(redis.hset(MY_KEY, MY_FIELD, MY_VALUE)).isTrue(); assertThat(redis.hexpireat(MY_KEY, Instant.now().plusSeconds(1), MY_FIELD)).containsExactly(1L); assertThat(redis.hexpireat("invalidKey", Instant.now().plusSeconds(1), MY_FIELD)).containsExactly(-2L); - await().until(() -> redis.hget(MY_KEY, MY_FIELD) == null); } @@ -610,7 +607,6 @@ void hexpiretime() { @EnabledOnCommand("HPERSIST") void hpersist() { assertThat(redis.hpersist(MY_KEY, MY_FIELD)).containsExactly(-2L); - assertThat(redis.hset(MY_KEY, MY_FIELD, MY_VALUE)).isTrue(); assertThat(redis.hpersist(MY_KEY, MY_FIELD)).containsExactly(-1L); diff --git a/src/test/java/io/lettuce/core/commands/ServerCommandIntegrationTests.java b/src/test/java/io/lettuce/core/commands/ServerCommandIntegrationTests.java index b9be862e79..a64414edf4 100644 --- a/src/test/java/io/lettuce/core/commands/ServerCommandIntegrationTests.java +++ b/src/test/java/io/lettuce/core/commands/ServerCommandIntegrationTests.java @@ -33,6 +33,8 @@ import javax.inject.Inject; +import io.lettuce.core.cluster.ClusterReadOnlyCommands; +import io.lettuce.core.protocol.ProtocolKeyword; import io.lettuce.test.condition.RedisConditions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; @@ -592,6 +594,13 @@ void clientSetinfo() { assertThat(redis.clientInfo().contains("lib-name=lettuce")).isTrue(); } + @Test + void testReadOnlyCommands() { + for (ProtocolKeyword readOnlyCommand : ClusterReadOnlyCommands.getReadOnlyCommands()) { + assertThat(isCommandReadOnly(readOnlyCommand.name())).isTrue(); + } + } + private boolean noSaveInProgress() { String info = redis.info(); @@ -599,4 +608,19 @@ private boolean noSaveInProgress() { return !info.contains("aof_rewrite_in_progress:1") && !info.contains("rdb_bgsave_in_progress:1"); } + private boolean isCommandReadOnly(String commandName) { + List commandInfo = redis.commandInfo(commandName); + if (commandInfo == null || commandInfo.isEmpty()) { + throw new IllegalArgumentException("Command not found: " + commandName); + } + + List details = CommandDetailParser.parse(commandInfo); + if (details.isEmpty()) { + throw new IllegalArgumentException("Command details could not be parsed: " + commandName); + } + + CommandDetail detail = details.get(0); + return !detail.getFlags().contains(CommandDetail.Flag.WRITE); + } + }