Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove not readonly commands #2832 #2871

Merged
merged 29 commits into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
10ebbf4
Remove not readonly commands #2832
thachlp Jun 4, 2024
e3080fd
Fix comment
thachlp Jun 8, 2024
36dc55a
Update test
thachlp Jun 10, 2024
1dcef97
Update cicd.yaml
atakavci Apr 24, 2024
430945e
GitHub issue template polishing and stale issues action (#2833)
tishun Apr 25, 2024
c1d213c
Implement HEXPIRE, HEXPIREAT, HEXPIRETIME and HPERSIST (#2836)
tishun Apr 25, 2024
9781769
MAke sure we wait for the check so it does not fail on the slower pip…
tishun Apr 30, 2024
53050ea
Add support for `SPUBLISH` (#2838)
atakavci May 2, 2024
e8ae914
Applying code formatter each time we run a Maven build (#2841)
tishun May 2, 2024
c0c5782
Update cicd.yaml
atakavci May 7, 2024
4eac609
Add support CLIENT KILL [MAXAGE] (#2782)
dengliming May 14, 2024
2899770
Add support for `SUNSUBSCRIBE` #2759 (#2851)
atakavci May 20, 2024
7a5f988
Mark dnsResolver(DnsResolver) as deprecated. (#2855)
yfwz100 May 22, 2024
11e5ee6
Implement HPEXPIRE, HPEXPIREAT, HPEXPIRETIME, HTTL and HPTTL (#2857)
tishun May 22, 2024
5612e29
XREAD support for reading last message from stream (#2863)
tishun May 28, 2024
7d5087e
Add a `evalReadOnly` overload that accepts the script as a `String` (…
BalmungSan May 31, 2024
9832f90
Add release drafter workflow (#2820)
tishun Jun 6, 2024
3c44012
Bump org.apache.maven.plugins:maven-javadoc-plugin from 3.6.3 to 3.7.…
dependabot[bot] Jun 7, 2024
1c272ec
Bump org.codehaus.mojo:flatten-maven-plugin from 1.5.0 to 1.6.0 (#2874)
dependabot[bot] Jun 7, 2024
eb07932
Bump org.apache.maven.plugins:maven-jar-plugin from 3.3.0 to 3.4.1 (#…
dependabot[bot] Jun 7, 2024
a5e535c
Bump org.openjdk.jmh:jmh-generator-annprocess from 1.21 to 1.37 (#2876)
dependabot[bot] Jun 7, 2024
d3b6214
Bump org.apache.commons:commons-pool2 from 2.11.1 to 2.12.0 (#2877)
dependabot[bot] Jun 7, 2024
544ec67
Setting the next release to be 6.4.x as part of #2880 (#2881)
tishun Jun 7, 2024
caadbfe
Modify the release acrtion to call the proper maven target for releas…
tishun Jun 7, 2024
74ec22f
Format file
thachlp Jun 11, 2024
8fdbd97
Using interface method from java 8 instead of java 9
thachlp Jun 12, 2024
a1ab7ef
Fix test
thachlp Jun 12, 2024
fe2f247
Revert readwrite command to the list
thachlp Jun 12, 2024
2f54211
Merge branch 'main' into remove-not-read-only-command
thachlp Jun 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 0 additions & 72 deletions src/main/java/io/lettuce/core/masterreplica/ReadOnlyCommands.java

This file was deleted.

4 changes: 2 additions & 2 deletions src/main/java/io/lettuce/core/protocol/ReadOnlyCommands.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ 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, //
RANDOMKEY, SCAN, SCARD, SCRIPT, //
SDIFF, SINTER, SISMEMBER, SMISMEMBER, SMEMBERS, SRANDMEMBER, SSCAN, STRLEN, //
SUNION, TIME, TTL, TYPE, //
XINFO, XLEN, XPENDING, XRANGE, XREVRANGE, XREAD, //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class ClusterReadOnlyCommandsUnitTests {

@Test
void testCount() {
assertThat(ClusterReadOnlyCommands.getReadOnlyCommands()).hasSize(86);
assertThat(ClusterReadOnlyCommands.getReadOnlyCommands()).hasSize(83);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package io.lettuce.core.commands;

import io.lettuce.core.TestSupport;
import io.lettuce.core.api.sync.RedisCommands;
import io.lettuce.core.cluster.ClusterReadOnlyCommands;
import io.lettuce.core.cluster.ClusterTestUtil;
import io.lettuce.core.cluster.api.StatefulRedisClusterConnection;
import io.lettuce.core.cluster.api.sync.RedisClusterCommands;
import io.lettuce.core.protocol.CommandType;
import io.lettuce.core.protocol.ProtocolKeyword;
import io.lettuce.core.protocol.ReadOnlyCommands;
import io.lettuce.test.KeyValueStreamingAdapter;
import io.lettuce.test.LettuceExtension;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.ExtendWith;

import javax.inject.Inject;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Thach Le
*/
@ExtendWith(LettuceExtension.class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class ReadOnlyCommandIntegrationTests extends TestSupport {

private final RedisCommands<String, String> redis;

@Inject
protected ReadOnlyCommandIntegrationTests(RedisCommands<String, String> redis) {
this.redis = redis;
}

@BeforeEach
void setUp() {
redis.flushall();
}

@Test
void testReadOnlyCommands() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! This is exactly what I wanted to see and it would help us keep this list up-to-date.

Two notes:

  • you can move this test to the ServerCommandIntegrationTest - no need to create a whole new class for it
  • you can use the CommandDetailParser to parse the result, instead of parsing it yourself (would also mean that if something changes in the result API you would not have to modify the test and we will keep the change locally to the parser)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tishun, thanks for the notes.
I updated the test, please help check 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, If anything need to pass the CI check, please suggest.
On local, I got this message:

[ERROR] Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.16.0:validate (default) on project lettuce-core: File '/...lettuce/src/test/java/io/lettuce/core/commands/ServerCommandIntegrationTests.java' has not been previously formatted.  Please format file and commit before running validation! -> [Help 1]

for (ProtocolKeyword readOnlyCommand : ClusterReadOnlyCommands.getReadOnlyCommands()) {
assertThat(isCommandReadOnly(readOnlyCommand.name())).isTrue();
}
}

private boolean isCommandReadOnly(String commandName) {
List<Object> commandInfo = redis.commandInfo(commandName);
if (commandInfo == null || commandInfo.isEmpty()) {
throw new IllegalArgumentException("Command not found: " + commandName);
}

List<Object> flags = (List<Object>) commandInfo.get(2); // Index 2 is the flags list
return flags.contains("readonly") && !flags.contains("write");
}
}
Loading