From cbcf0f3b1b9f8daed0b5dbcda004a5920119bbb0 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 7 Jun 2023 00:09:48 +0600 Subject: [PATCH 1/4] Remove existing print from JedisShardedPubSubBase.java --- src/main/java/redis/clients/jedis/JedisShardedPubSubBase.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/redis/clients/jedis/JedisShardedPubSubBase.java b/src/main/java/redis/clients/jedis/JedisShardedPubSubBase.java index ad3acd18e8..f0a251f61f 100644 --- a/src/main/java/redis/clients/jedis/JedisShardedPubSubBase.java +++ b/src/main/java/redis/clients/jedis/JedisShardedPubSubBase.java @@ -94,7 +94,6 @@ private void process() { final T enmesg = (bmesg == null) ? null : encode(bmesg); onSMessage(enchannel, enmesg); } else { - System.out.println(redis.clients.jedis.util.SafeEncoder.encodeObject(resp)); throw new JedisException("Unknown message type: " + firstObj); } } else { From 9383cd4cce54766cd80a3d30a4f710475c38da35 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 8 Jun 2023 19:54:53 +0600 Subject: [PATCH 2/4] Allow setting default dialect for search module (#3452) --- docs/jedis5-breaking.md | 2 + .../redis/clients/jedis/CommandObjects.java | 33 +-- .../redis/clients/jedis/UnifiedJedis.java | 4 + .../clients/jedis/search/FTSearchParams.java | 12 ++ .../jedis/search/FTSpellCheckParams.java | 12 ++ .../redis/clients/jedis/search/Query.java | 16 +- .../jedis/search/aggr/AggregationBuilder.java | 104 ++++++---- .../search/aggr/FtAggregateIteration.java | 2 +- .../search/SearchDefaultDialectTest.java | 194 ++++++++++++++++++ 9 files changed, 319 insertions(+), 60 deletions(-) create mode 100644 src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java diff --git a/docs/jedis5-breaking.md b/docs/jedis5-breaking.md index 2c67b538a9..2f50318c8c 100644 --- a/docs/jedis5-breaking.md +++ b/docs/jedis5-breaking.md @@ -70,6 +70,8 @@ - `addCommandEncodedArguments` and `addCommandBinaryArguments` methods have been removed from `FieldName` class. +- `getArgs` method is removed from `AggregationBuilder` class. + - `limit` and `getArgs` methods have been removed from `Group` class. - `Reducer` abstract class is refactored: diff --git a/src/main/java/redis/clients/jedis/CommandObjects.java b/src/main/java/redis/clients/jedis/CommandObjects.java index 462e6a083c..a7198c3b3d 100644 --- a/src/main/java/redis/clients/jedis/CommandObjects.java +++ b/src/main/java/redis/clients/jedis/CommandObjects.java @@ -4,6 +4,7 @@ import static redis.clients.jedis.Protocol.Keyword.*; import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.json.JSONArray; import org.json.JSONObject; @@ -40,6 +41,7 @@ protected void setProtocol(RedisProtocol proto) { } private volatile JsonObjectMapper jsonObjectMapper; + private final AtomicInteger searchDialect = new AtomicInteger(0); private JedisBroadcastAndRoundRobinConfig broadcastAndRoundRobinConfig = null; @@ -3198,32 +3200,34 @@ public final CommandObject ftSearch(String indexName, String query public final CommandObject ftSearch(String indexName, String query, FTSearchParams params) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName) - .add(query).addParams(params), new SearchResultBuilder(!params.getNoContent(), params.getWithScores(), true)); + .add(query).addParams(params.dialectOptional(searchDialect.get())), new SearchResultBuilder(!params.getNoContent(), params.getWithScores(), true)); } public final CommandObject ftSearch(String indexName, Query query) { - return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName).addParams(query), + return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName) + .addParams(query.dialectOptional(searchDialect.get())), new SearchResultBuilder(!query.getNoContent(), query.getWithScores(), true)); } public final CommandObject ftSearch(byte[] indexName, Query query) { - return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName).addParams(query), + return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SEARCH), indexName) + .addParams(query.dialectOptional(searchDialect.get())), new SearchResultBuilder(!query.getNoContent(), query.getWithScores(), false)); } public final CommandObject ftExplain(String indexName, Query query) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.EXPLAIN), indexName) - .addParams(query), BuilderFactory.STRING); + .addParams(query.dialectOptional(searchDialect.get())), BuilderFactory.STRING); } public final CommandObject> ftExplainCLI(String indexName, Query query) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.EXPLAINCLI), indexName) - .addParams(query), BuilderFactory.STRING_LIST); + .addParams(query.dialectOptional(searchDialect.get())), BuilderFactory.STRING_LIST); } public final CommandObject ftAggregate(String indexName, AggregationBuilder aggr) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.AGGREGATE), indexName) - .addObjects(aggr.getArgs()), !aggr.isWithCursor() ? SearchBuilderFactory.SEARCH_AGGREGATION_RESULT + .addParams(aggr.dialectOptional(searchDialect.get())), !aggr.isWithCursor() ? SearchBuilderFactory.SEARCH_AGGREGATION_RESULT : SearchBuilderFactory.SEARCH_AGGREGATION_RESULT_WITH_CURSOR); } @@ -3242,7 +3246,7 @@ public final CommandObject>> ft String indexName, FTProfileParams profileParams, AggregationBuilder aggr) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.PROFILE), indexName) .add(SearchKeyword.AGGREGATE).addParams(profileParams).add(SearchKeyword.QUERY) - .addObjects(aggr.getArgs()), new SearchProfileResponseBuilder<>(!aggr.isWithCursor() + .addParams(aggr.dialectOptional(searchDialect.get())), new SearchProfileResponseBuilder<>(!aggr.isWithCursor() ? SearchBuilderFactory.SEARCH_AGGREGATION_RESULT : SearchBuilderFactory.SEARCH_AGGREGATION_RESULT_WITH_CURSOR)); } @@ -3251,16 +3255,16 @@ public final CommandObject>> ftProfi String indexName, FTProfileParams profileParams, Query query) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.PROFILE), indexName) .add(SearchKeyword.SEARCH).addParams(profileParams).add(SearchKeyword.QUERY) - .addParams(query), new SearchProfileResponseBuilder<>(new SearchResultBuilder( - !query.getNoContent(), query.getWithScores(), true))); + .addParams(query.dialectOptional(searchDialect.get())), new SearchProfileResponseBuilder<>( + new SearchResultBuilder(!query.getNoContent(), query.getWithScores(), true))); } public final CommandObject>> ftProfileSearch( String indexName, FTProfileParams profileParams, String query, FTSearchParams searchParams) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.PROFILE), indexName) .add(SearchKeyword.SEARCH).addParams(profileParams).add(SearchKeyword.QUERY).add(query) - .addParams(searchParams), new SearchProfileResponseBuilder<>(new SearchResultBuilder( - !searchParams.getNoContent(), searchParams.getWithScores(), true))); + .addParams(searchParams.dialectOptional(searchDialect.get())), new SearchProfileResponseBuilder<>( + new SearchResultBuilder(!searchParams.getNoContent(), searchParams.getWithScores(), true))); } public final CommandObject ftDropIndex(String indexName) { @@ -3316,7 +3320,7 @@ public final CommandObject>> ftSpellCheck(String public final CommandObject>> ftSpellCheck(String index, String query, FTSpellCheckParams spellCheckParams) { return new CommandObject<>(checkAndRoundRobinSearchCommand(commandArguments(SearchCommand.SPELLCHECK), index).add(query) - .addParams(spellCheckParams), SearchBuilderFactory.SEARCH_SPELLCHECK_RESPONSE); + .addParams(spellCheckParams.dialectOptional(searchDialect.get())), SearchBuilderFactory.SEARCH_SPELLCHECK_RESPONSE); } public final CommandObject> ftInfo(String indexName) { @@ -4192,6 +4196,11 @@ public void setJsonObjectMapper(JsonObjectMapper jsonObjectMapper) { this.jsonObjectMapper = jsonObjectMapper; } + public void setDefaultSearchDialect(int dialect) { + if (dialect == 0) throw new IllegalArgumentException("DIALECT=0 cannot be set."); + this.searchDialect.set(dialect); + } + private class SearchProfileResponseBuilder extends Builder>> { private final Builder replyBuilder; diff --git a/src/main/java/redis/clients/jedis/UnifiedJedis.java b/src/main/java/redis/clients/jedis/UnifiedJedis.java index 211dbeaf4b..acf6ad7dfd 100644 --- a/src/main/java/redis/clients/jedis/UnifiedJedis.java +++ b/src/main/java/redis/clients/jedis/UnifiedJedis.java @@ -4803,4 +4803,8 @@ public Object executeCommand(CommandArguments args) { public void setJsonObjectMapper(JsonObjectMapper jsonObjectMapper) { this.commandObjects.setJsonObjectMapper(jsonObjectMapper); } + + public void setDefaultSearchDialect(int dialect) { + this.commandObjects.setDefaultSearchDialect(dialect); + } } diff --git a/src/main/java/redis/clients/jedis/search/FTSearchParams.java b/src/main/java/redis/clients/jedis/search/FTSearchParams.java index 2f3d6394de..1bb5597085 100644 --- a/src/main/java/redis/clients/jedis/search/FTSearchParams.java +++ b/src/main/java/redis/clients/jedis/search/FTSearchParams.java @@ -422,6 +422,18 @@ public FTSearchParams dialect(int dialect) { return this; } + /** + * This method will not replace the dialect if it has been already set. + * @param dialect dialect + * @return this + */ + public FTSearchParams dialectOptional(int dialect) { + if (dialect != 0 && this.dialect == null) { + this.dialect = dialect; + } + return this; + } + public boolean getNoContent() { return noContent; } diff --git a/src/main/java/redis/clients/jedis/search/FTSpellCheckParams.java b/src/main/java/redis/clients/jedis/search/FTSpellCheckParams.java index c3cde42cfb..439885a2a1 100644 --- a/src/main/java/redis/clients/jedis/search/FTSpellCheckParams.java +++ b/src/main/java/redis/clients/jedis/search/FTSpellCheckParams.java @@ -69,6 +69,18 @@ public FTSpellCheckParams dialect(int dialect) { return this; } + /** + * This method will not replace the dialect if it has been already set. + * @param dialect dialect + * @return this + */ + public FTSpellCheckParams dialectOptional(int dialect) { + if (dialect != 0 && this.dialect == null) { + this.dialect = dialect; + } + return this; + } + @Override public void addParams(CommandArguments args) { diff --git a/src/main/java/redis/clients/jedis/search/Query.java b/src/main/java/redis/clients/jedis/search/Query.java index f4de2f78a2..66cba96acf 100644 --- a/src/main/java/redis/clients/jedis/search/Query.java +++ b/src/main/java/redis/clients/jedis/search/Query.java @@ -156,7 +156,7 @@ public HighlightTags(String open, String close) { private boolean wantsSummarize = false; private String _scorer = null; private Map _params = null; - private int _dialect = 0; + private Integer _dialect; private int _slop = -1; private long _timeout = -1; private boolean _inOrder = false; @@ -299,7 +299,7 @@ public void addParams(CommandArguments args) { } } - if (_dialect != 0) { + if (_dialect != null) { args.add(SearchKeyword.DIALECT.getRaw()); args.add(_dialect); } @@ -544,6 +544,18 @@ public Query dialect(int dialect) { return this; } + /** + * This method will not replace the dialect if it has been already set. + * @param dialect dialect + * @return this + */ + public Query dialectOptional(int dialect) { + if (dialect != 0 && this._dialect == null) { + this._dialect = dialect; + } + return this; + } + /** * Set the slop to execute the query accordingly * diff --git a/src/main/java/redis/clients/jedis/search/aggr/AggregationBuilder.java b/src/main/java/redis/clients/jedis/search/aggr/AggregationBuilder.java index 13d6f9a270..eb8e039d02 100644 --- a/src/main/java/redis/clients/jedis/search/aggr/AggregationBuilder.java +++ b/src/main/java/redis/clients/jedis/search/aggr/AggregationBuilder.java @@ -7,7 +7,9 @@ import java.util.List; import java.util.Map; +import redis.clients.jedis.CommandArguments; import redis.clients.jedis.Protocol; +import redis.clients.jedis.params.IParams; import redis.clients.jedis.search.FieldName; import redis.clients.jedis.search.SearchProtocol.SearchKeyword; import redis.clients.jedis.util.LazyRawable; @@ -15,14 +17,14 @@ /** * @author Guy Korland */ -public class AggregationBuilder { +public class AggregationBuilder implements IParams { - private final List args = new ArrayList<>(); + private final List aggrArgs = new ArrayList<>(); private Integer dialect; private boolean isWithCursor = false; public AggregationBuilder(String query) { - args.add(query); + aggrArgs.add(query); } public AggregationBuilder() { @@ -34,27 +36,27 @@ public AggregationBuilder load(String... fields) { } public AggregationBuilder load(FieldName... fields) { - args.add(SearchKeyword.LOAD); + aggrArgs.add(SearchKeyword.LOAD); LazyRawable rawLoadCount = new LazyRawable(); - args.add(rawLoadCount); + aggrArgs.add(rawLoadCount); int loadCount = 0; for (FieldName fn : fields) { - loadCount += fn.addCommandArguments(args); + loadCount += fn.addCommandArguments(aggrArgs); } rawLoadCount.setRaw(Protocol.toByteArray(loadCount)); return this; } public AggregationBuilder loadAll() { - args.add(SearchKeyword.LOAD); - args.add(Protocol.BYTES_ASTERISK); + aggrArgs.add(SearchKeyword.LOAD); + aggrArgs.add(Protocol.BYTES_ASTERISK); return this; } public AggregationBuilder limit(int offset, int count) { - args.add(SearchKeyword.LIMIT); - args.add(offset); - args.add(count); + aggrArgs.add(SearchKeyword.LIMIT); + aggrArgs.add(offset); + aggrArgs.add(count); return this; } @@ -63,11 +65,11 @@ public AggregationBuilder limit(int count) { } public AggregationBuilder sortBy(SortedField... fields) { - args.add(SearchKeyword.SORTBY); - args.add(Integer.toString(fields.length * 2)); + aggrArgs.add(SearchKeyword.SORTBY); + aggrArgs.add(Integer.toString(fields.length * 2)); for (SortedField field : fields) { - args.add(field.getField()); - args.add(field.getOrder()); + aggrArgs.add(field.getField()); + aggrArgs.add(field.getOrder()); } return this; } @@ -89,8 +91,8 @@ public AggregationBuilder sortByDesc(String field) { * @return this */ public AggregationBuilder sortByMax(int max) { - args.add(SearchKeyword.MAX); - args.add(max); + aggrArgs.add(SearchKeyword.MAX); + aggrArgs.add(max); return this; } @@ -108,16 +110,16 @@ public AggregationBuilder sortBy(int max, SortedField... fields) { } public AggregationBuilder apply(String projection, String alias) { - args.add(SearchKeyword.APPLY); - args.add(projection); - args.add(SearchKeyword.AS); - args.add(alias); + aggrArgs.add(SearchKeyword.APPLY); + aggrArgs.add(projection); + aggrArgs.add(SearchKeyword.AS); + aggrArgs.add(alias); return this; } public AggregationBuilder groupBy(Group group) { - args.add(SearchKeyword.GROUPBY); - group.addArgs(args); + aggrArgs.add(SearchKeyword.GROUPBY); + group.addArgs(aggrArgs); return this; } @@ -134,46 +136,46 @@ public AggregationBuilder groupBy(String field, Reducer... reducers) { } public AggregationBuilder filter(String expression) { - args.add(SearchKeyword.FILTER); - args.add(expression); + aggrArgs.add(SearchKeyword.FILTER); + aggrArgs.add(expression); return this; } public AggregationBuilder cursor(int count) { isWithCursor = true; - args.add(SearchKeyword.WITHCURSOR); - args.add(SearchKeyword.COUNT); - args.add(count); + aggrArgs.add(SearchKeyword.WITHCURSOR); + aggrArgs.add(SearchKeyword.COUNT); + aggrArgs.add(count); return this; } public AggregationBuilder cursor(int count, long maxIdle) { isWithCursor = true; - args.add(SearchKeyword.WITHCURSOR); - args.add(SearchKeyword.COUNT); - args.add(count); - args.add(SearchKeyword.MAXIDLE); - args.add(maxIdle); + aggrArgs.add(SearchKeyword.WITHCURSOR); + aggrArgs.add(SearchKeyword.COUNT); + aggrArgs.add(count); + aggrArgs.add(SearchKeyword.MAXIDLE); + aggrArgs.add(maxIdle); return this; } public AggregationBuilder verbatim() { - args.add(SearchKeyword.VERBATIM); + aggrArgs.add(SearchKeyword.VERBATIM); return this; } public AggregationBuilder timeout(long timeout) { - args.add(SearchKeyword.TIMEOUT); - args.add(timeout); + aggrArgs.add(SearchKeyword.TIMEOUT); + aggrArgs.add(timeout); return this; } public AggregationBuilder params(Map params) { - args.add(SearchKeyword.PARAMS); - args.add(params.size() * 2); + aggrArgs.add(SearchKeyword.PARAMS); + aggrArgs.add(params.size() * 2); params.forEach((k, v) -> { - args.add(k); - args.add(v); + aggrArgs.add(k); + aggrArgs.add(v); }); return this; } @@ -183,15 +185,27 @@ public AggregationBuilder dialect(int dialect) { return this; } - public List getArgs() { - if (dialect != null) { - args.add(SearchKeyword.DIALECT); - args.add(dialect); + /** + * This method will not replace the dialect if it has been already set. + * @param dialect dialect + * @return this + */ + public AggregationBuilder dialectOptional(int dialect) { + if (dialect != 0 && this.dialect == null) { + this.dialect = dialect; } - return Collections.unmodifiableList(args); + return this; } public boolean isWithCursor() { return isWithCursor; } + + @Override + public void addParams(CommandArguments commArgs) { + commArgs.addObjects(aggrArgs); + if (dialect != null) { + commArgs.add(SearchKeyword.DIALECT).add(dialect); + } + } } diff --git a/src/main/java/redis/clients/jedis/search/aggr/FtAggregateIteration.java b/src/main/java/redis/clients/jedis/search/aggr/FtAggregateIteration.java index 20d89a3b7a..f35100af09 100644 --- a/src/main/java/redis/clients/jedis/search/aggr/FtAggregateIteration.java +++ b/src/main/java/redis/clients/jedis/search/aggr/FtAggregateIteration.java @@ -23,7 +23,7 @@ public FtAggregateIteration(ConnectionProvider connectionProvider, String indexN super(connectionProvider, SearchBuilderFactory.SEARCH_AGGREGATION_RESULT_WITH_CURSOR); if (!aggr.isWithCursor()) throw new IllegalArgumentException("cursor must be set"); this.indexName = indexName; - this.args = new CommandArguments(SearchProtocol.SearchCommand.AGGREGATE).add(this.indexName).addObjects(aggr.getArgs()); + this.args = new CommandArguments(SearchProtocol.SearchCommand.AGGREGATE).add(this.indexName).addParams(aggr); } @Override diff --git a/src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java b/src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java new file mode 100644 index 0000000000..b803b1eceb --- /dev/null +++ b/src/test/java/redis/clients/jedis/modules/search/SearchDefaultDialectTest.java @@ -0,0 +1,194 @@ +package redis.clients.jedis.modules.search; + +import static org.junit.Assert.*; +import static redis.clients.jedis.util.AssertUtil.assertOK; + +import java.util.*; + +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +import redis.clients.jedis.exceptions.JedisDataException; +import redis.clients.jedis.search.*; +import redis.clients.jedis.search.schemafields.NumericField; +import redis.clients.jedis.search.schemafields.TextField; +import redis.clients.jedis.modules.RedisModuleCommandsTestBase; +import redis.clients.jedis.search.aggr.AggregationBuilder; +import redis.clients.jedis.search.aggr.AggregationResult; +import redis.clients.jedis.search.aggr.Reducers; +import redis.clients.jedis.search.aggr.Row; + +public class SearchDefaultDialectTest extends RedisModuleCommandsTestBase { + + private static final String INDEX = "dialect-INDEX"; + private static final int DEFAULT_DIALECT = 2; + + @BeforeClass + public static void prepare() { + RedisModuleCommandsTestBase.prepare(); + } + + @Override + public void setUp() { + super.setUp(); + client.setDefaultSearchDialect(DEFAULT_DIALECT); + } +// +// @AfterClass +// public static void tearDown() { +//// RedisModuleCommandsTestBase.tearDown(); +// } + + private void addDocument(Document doc) { + String key = doc.getId(); + Map map = new LinkedHashMap<>(); + doc.getProperties().forEach(entry -> map.put(entry.getKey(), String.valueOf(entry.getValue()))); + client.hset(key, map); + } + + @Test + public void testQueryParams() { + Schema sc = new Schema().addNumericField("numval"); + assertEquals("OK", client.ftCreate(INDEX, IndexOptions.defaultOptions(), sc)); + + client.hset("1", "numval", "1"); + client.hset("2", "numval", "2"); + client.hset("3", "numval", "3"); + + Query query = new Query("@numval:[$min $max]").addParam("min", 1).addParam("max", 2); + assertEquals(2, client.ftSearch(INDEX, query).getTotalResults()); + } + + @Test + public void testQueryParamsWithParams() { + assertOK(client.ftCreate(INDEX, NumericField.of("numval"))); + + client.hset("1", "numval", "1"); + client.hset("2", "numval", "2"); + client.hset("3", "numval", "3"); + + assertEquals(2, client.ftSearch(INDEX, "@numval:[$min $max]", + FTSearchParams.searchParams().addParam("min", 1).addParam("max", 2)).getTotalResults()); + + Map paramValues = new HashMap<>(); + paramValues.put("min", 1); + paramValues.put("max", 2); + assertEquals(2, client.ftSearch(INDEX, "@numval:[$min $max]", + FTSearchParams.searchParams().params(paramValues)).getTotalResults()); + } + + @Test + public void testDialectsWithFTExplain() throws Exception { + Map attr = new HashMap<>(); + attr.put("TYPE", "FLOAT32"); + attr.put("DIM", 2); + attr.put("DISTANCE_METRIC", "L2"); + + Schema sc = new Schema() + .addFlatVectorField("v", attr) + .addTagField("title") + .addTextField("t1", 1.0) + .addTextField("t2", 1.0) + .addNumericField("num"); + assertEquals("OK", client.ftCreate(INDEX, IndexOptions.defaultOptions(), sc)); + + client.hset("1", "t1", "hello"); + + String q = "(*)"; + Query query = new Query(q).dialect(1); + try { + client.ftExplain(INDEX, query); + fail(); + } catch (JedisDataException e) { + assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); + } + query = new Query(q); // dialect=default=2 + assertTrue("Should contain 'WILDCARD'", client.ftExplain(INDEX, query).contains("WILDCARD")); + + q = "$hello"; + query = new Query(q).dialect(1); + try { + client.ftExplain(INDEX, query); + fail(); + } catch (JedisDataException e) { + assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); + } + query = new Query(q).addParam("hello", "hello"); // dialect=default=2 + assertTrue("Should contain 'UNION {\n hello\n +hello(expanded)\n}\n'", + client.ftExplain(INDEX, query).contains("UNION {\n hello\n +hello(expanded)\n}\n")); + + q = "@title:(@num:[0 10])"; + query = new Query(q).dialect(1); + assertTrue("Should contain 'NUMERIC {0.000000 <= @num <= 10.000000}'", + client.ftExplain(INDEX, query).contains("NUMERIC {0.000000 <= @num <= 10.000000}")); + query = new Query(q); // dialect=default=2 + try { + client.ftExplain(INDEX, query); + fail(); + } catch (JedisDataException e) { + assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); + } + + q = "@t1:@t2:@t3:hello"; + query = new Query(q).dialect(1); + assertTrue("Should contain '@NULL:UNION {\n @NULL:hello\n @NULL:+hello(expanded)\n}\n'", + client.ftExplain(INDEX, query).contains("@NULL:UNION {\n @NULL:hello\n @NULL:+hello(expanded)\n}\n")); + query = new Query(q); // dialect=default=2 + try { + client.ftExplain(INDEX, query); + fail(); + } catch (JedisDataException e) { + assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); + } + + q = "@title:{foo}}}}}"; + query = new Query(q).dialect(1); + assertTrue("Should contain 'TAG:@title {\n foo\n}\n'", + client.ftExplain(INDEX, query).contains("TAG:@title {\n foo\n}\n")); + query = new Query(q); // dialect=default=2 + try { + client.ftExplain(INDEX, query); + fail(); + } catch (JedisDataException e) { + assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error")); + } + } + + @Test + public void testAggregationBuilderParamsDialect() { + Schema sc = new Schema(); + sc.addSortableTextField("name", 1.0); + sc.addSortableNumericField("count"); + client.ftCreate(INDEX, IndexOptions.defaultOptions(), sc); + addDocument(new Document("data1").set("name", "abc").set("count", 10)); + addDocument(new Document("data2").set("name", "def").set("count", 5)); + addDocument(new Document("data3").set("name", "def").set("count", 25)); + + Map params = new HashMap<>(); + params.put("name", "abc"); + + AggregationBuilder r = new AggregationBuilder("$name") + .groupBy("@name", Reducers.sum("@count").as("sum")) + .params(params); + + AggregationResult res = client.ftAggregate(INDEX, r); + assertEquals(1, res.getTotalResults()); + + Row r1 = res.getRow(0); + assertNotNull(r1); + assertEquals("abc", r1.getString("name")); + assertEquals(10, r1.getLong("sum")); + } + + @Test + public void dialectBoundSpellCheck() { + client.ftCreate(INDEX, TextField.of("t")); + JedisDataException error = Assert.assertThrows(JedisDataException.class, + () -> client.ftSpellCheck(INDEX, "Tooni toque kerfuffle", + FTSpellCheckParams.spellCheckParams().dialect(0))); + MatcherAssert.assertThat(error.getMessage(), Matchers.containsString("DIALECT requires a non negative integer")); + } +} From 869dc0bb6625b85c8bf15bf1361bde485a304338 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 8 Jun 2023 19:55:28 +0600 Subject: [PATCH 3/4] Fix tsInfoDebug after update (#3461) --- .../java/redis/clients/jedis/BuilderFactory.java | 15 --------------- .../redis/clients/jedis/timeseries/TSInfo.java | 2 +- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/main/java/redis/clients/jedis/BuilderFactory.java b/src/main/java/redis/clients/jedis/BuilderFactory.java index b54ee8222a..bb01dcebcc 100644 --- a/src/main/java/redis/clients/jedis/BuilderFactory.java +++ b/src/main/java/redis/clients/jedis/BuilderFactory.java @@ -1719,21 +1719,6 @@ public String toString() { } }; - public static final Builder> ENCODED_OBJECT_MAP_FROM_PAIRS = new Builder>() { - @Override - public Map build(Object data) { - final List list = (List) data; - final Map map = new HashMap<>(list.size(), 1f); - for (Object object : list) { - if (object == null) continue; - final List flat = (List) object; - if (flat.isEmpty()) continue; - map.put(STRING.build(flat.get(0)), ENCODED_OBJECT.build(flat.get(1))); - } - return map; - } - }; - public static final Builder> LIBRARY_LIST = new Builder>() { @Override public List build(Object data) { diff --git a/src/main/java/redis/clients/jedis/timeseries/TSInfo.java b/src/main/java/redis/clients/jedis/timeseries/TSInfo.java index b77743fdce..2519d26232 100644 --- a/src/main/java/redis/clients/jedis/timeseries/TSInfo.java +++ b/src/main/java/redis/clients/jedis/timeseries/TSInfo.java @@ -163,7 +163,7 @@ public TSInfo build(Object data) { List> chunksValueList = new ArrayList<>(chunksDataList.size()); chunks = new ArrayList<>(chunksDataList.size()); for (Object chunkData : chunksDataList) { - Map chunk = BuilderFactory.ENCODED_OBJECT_MAP_FROM_PAIRS.build(chunkData); + Map chunk = BuilderFactory.ENCODED_OBJECT_MAP.build(chunkData); chunksValueList.add(new HashMap<>(chunk)); chunks.add(chunk); } From 7a68df83daaf491b485dfe681a602f85b22bab14 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Sun, 11 Jun 2023 14:20:12 +0600 Subject: [PATCH 4/4] Use List to read RESP3 Map reply (#3456) When implementing RESP3, I converted the Redis Map reply (map{k1->v1, k2->v2, ...}) to simple flat array (list{k1, v1, k2, v2, ...}). In this PR, I have converted the Map reply to array of KeyValues (list{ kv{k1, v1}, kv{k2, v2}, ...}). As a by-product of this PR, some commands were found where the return can/should be a Map instead of a List. --- * Use List to read RESP3 Map reply * module replies * fix lcs * fix xinfo * fix moduleList * fix timeoutConnection * fix aclLog already fixed * edit * fix moduleList * fix encodeCompleteResponse * Fix tsInfoDebug after timeseries update * remove commented code * docs and test edit --- docs/jedis5-breaking.md | 6 + .../redis/clients/jedis/BuilderFactory.java | 346 +++++++++--------- .../redis/clients/jedis/CommandObjects.java | 9 +- src/main/java/redis/clients/jedis/Jedis.java | 76 ++-- src/main/java/redis/clients/jedis/Module.java | 2 + .../java/redis/clients/jedis/Protocol.java | 37 +- .../AccessControlLogBinaryCommands.java | 18 +- .../commands/AccessControlLogCommands.java | 18 +- .../jedis/commands/ConfigCommands.java | 14 +- .../jedis/resps/AccessControlLogEntry.java | 1 + .../jedis/resps/AccessControlUser.java | 113 ++++-- .../clients/jedis/resps/CommandDocument.java | 37 +- .../clients/jedis/resps/FunctionStats.java | 35 +- .../clients/jedis/resps/LibraryInfo.java | 41 ++- .../redis/clients/jedis/resps/Slowlog.java | 11 +- .../clients/jedis/timeseries/TSInfo.java | 34 +- .../timeseries/TimeSeriesBuilderFactory.java | 36 +- .../redis/clients/jedis/util/SafeEncoder.java | 5 + .../java/redis/clients/jedis/JedisTest.java | 6 +- .../jedis/AccessControlListCommandsTest.java | 8 +- .../jedis/AllKindOfValuesCommandsTest.java | 94 ++++- .../commands/jedis/ControlCommandsTest.java | 36 +- .../commands/jedis/JedisCommandsTestBase.java | 6 +- .../jedis/commands/jedis/ModuleTest.java | 19 +- .../commands/jedis/ScriptingCommandsTest.java | 41 ++- .../commands/jedis/SlowlogCommandsTest.java | 49 +-- .../commands/jedis/StreamsCommandsTest.java | 2 +- .../AllKindOfValuesCommandsTestBase.java | 97 ++++- 28 files changed, 756 insertions(+), 441 deletions(-) diff --git a/docs/jedis5-breaking.md b/docs/jedis5-breaking.md index 2f50318c8c..f9d5df7ad4 100644 --- a/docs/jedis5-breaking.md +++ b/docs/jedis5-breaking.md @@ -26,6 +26,12 @@ - `zunion(ZParams params, byte[]... keys)` method now returns `List` (instead of `Set`). - Both `zunionWithScores(ZParams params, String... keys)` and `zunionWithScores(ZParams params, byte[]... keys)` methods now return `List` (instead of `Set`). +- Both `configGet(String pattern)` and `configGet(String... patterns)` methods now return `Map` instead of `List`. +- Both `configGet(byte[] pattern)` and `configGet(byte[]... patterns)` methods now return `Map` instead of `List`. + +- New `aclDelUser(String... names)` method replaces `aclDelUser(String name)` and `aclDelUser(String name, String... names)` methods. +- New `aclDelUser(byte[]... names)` method replaces `aclDelUser(byte[] name)` and `aclDelUser(byte[] name, byte[]... names)` methods. + - `tsMGet(TSMGetParams multiGetParams, String... filters)` method now returns `Map` instead of `List>`. - Following methods now return `Map` instead of `List`: diff --git a/src/main/java/redis/clients/jedis/BuilderFactory.java b/src/main/java/redis/clients/jedis/BuilderFactory.java index bb01dcebcc..46ce255c35 100644 --- a/src/main/java/redis/clients/jedis/BuilderFactory.java +++ b/src/main/java/redis/clients/jedis/BuilderFactory.java @@ -63,19 +63,6 @@ public String toString() { } }; - public static final Builder> ENCODED_OBJECT_MAP = new Builder>() { - @Override - public Map build(Object data) { - final List list = (List) data; - final Map map = new HashMap<>(list.size() / 2, 1f); - final Iterator iterator = list.iterator(); - while (iterator.hasNext()) { - map.put(STRING.build(iterator.next()), ENCODED_OBJECT.build(iterator.next())); - } - return map; - } - }; - public static final Builder LONG = new Builder() { @Override public Long build(Object data) { @@ -221,26 +208,6 @@ public String toString() { } }; - public static final Builder> BINARY_MAP = new Builder>() { - @Override - @SuppressWarnings("unchecked") - public Map build(Object data) { - final List flatHash = (List) data; - final Map hash = new JedisByteHashMap(); - final Iterator iterator = flatHash.iterator(); - while (iterator.hasNext()) { - hash.put(iterator.next(), iterator.next()); - } - - return hash; - } - - @Override - public String toString() { - return "Map"; - } - }; - public static final Builder>> BINARY_PAIR_LIST = new Builder>>() { @Override @@ -323,18 +290,86 @@ public String toString() { } }; + public static final Builder> ENCODED_OBJECT_MAP = new Builder>() { + @Override + public Map build(Object data) { + if (data == null) return null; + final List list = (List) data; + if (list.isEmpty()) return Collections.emptyMap(); + + if (list.get(0) instanceof KeyValue) { + final Map map = new HashMap<>(list.size(), 1f); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + KeyValue kv = (KeyValue) iterator.next(); + map.put(STRING.build(kv.getKey()), ENCODED_OBJECT.build(kv.getValue())); + } + return map; + } else { + final Map map = new HashMap<>(list.size() / 2, 1f); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + map.put(STRING.build(iterator.next()), ENCODED_OBJECT.build(iterator.next())); + } + return map; + } + } + }; + + public static final Builder> BINARY_MAP = new Builder>() { + @Override + @SuppressWarnings("unchecked") + public Map build(Object data) { + final List list = (List) data; + if (list.isEmpty()) return Collections.emptyMap(); + + if (list.get(0) instanceof KeyValue) { + final Map map = new JedisByteHashMap(); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + KeyValue kv = (KeyValue) iterator.next(); + map.put(BINARY.build(kv.getKey()), BINARY.build(kv.getValue())); + } + return map; + } else { + final Map map = new JedisByteHashMap(); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + map.put(BINARY.build(iterator.next()), BINARY.build(iterator.next())); + } + return map; + } + } + + @Override + public String toString() { + return "Map"; + } + }; + public static final Builder> STRING_MAP = new Builder>() { @Override @SuppressWarnings("unchecked") public Map build(Object data) { - final List flatHash = (List) data; - final Map hash = new HashMap<>(flatHash.size() / 2, 1f); - final Iterator iterator = flatHash.iterator(); - while (iterator.hasNext()) { - hash.put(SafeEncoder.encode(iterator.next()), SafeEncoder.encode(iterator.next())); + final List list = (List) data; + if (list.isEmpty()) return Collections.emptyMap(); + + if (list.get(0) instanceof KeyValue) { + final Map map = new HashMap<>(list.size(), 1f); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + KeyValue kv = (KeyValue) iterator.next(); + map.put(STRING.build(kv.getKey()), STRING.build(kv.getValue())); + } + return map; + } else { + final Map map = new HashMap<>(list.size() / 2, 1f); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + map.put(STRING.build(iterator.next()), STRING.build(iterator.next())); + } + return map; } - - return hash; } @Override @@ -852,20 +887,26 @@ public String toString() { public static final Builder> COMMAND_DOCS_RESPONSE = new Builder>() { @Override public Map build(Object data) { - if (data == null) { - return null; - } - + if (data == null) return null; List list = (List) data; - Map map = new HashMap<>(list.size() / 2, 1f); - - for (int i = 0; i < list.size();) { - String name = STRING.build(list.get(i++)); - CommandDocument doc = CommandDocument.COMMAND_DOCUMENT_BUILDER.build(list.get(i++)); - map.put(name, doc); + if (list.isEmpty()) return Collections.emptyMap(); + + if (list.get(0) instanceof KeyValue) { + final Map map = new HashMap<>(list.size(), 1f); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + KeyValue kv = (KeyValue) iterator.next(); + map.put(STRING.build(kv.getKey()), new CommandDocument(ENCODED_OBJECT_MAP.build(kv.getValue()))); + } + return map; + } else { + final Map map = new HashMap<>(list.size() / 2, 1f); + final Iterator iterator = list.iterator(); + while (iterator.hasNext()) { + map.put(STRING.build(iterator.next()), new CommandDocument(ENCODED_OBJECT_MAP.build(iterator.next()))); + } + return map; } - - return map; } }; @@ -909,6 +950,11 @@ public List build(Object data) { } for (List moduleResp : objectList) { + if (moduleResp.get(0) instanceof KeyValue) { + responses.add(new Module(STRING.build(((KeyValue) moduleResp.get(0)).getValue()), + LONG.build(((KeyValue) moduleResp.get(1)).getValue()).intValue())); + continue; + } Module m = new Module(SafeEncoder.encode((byte[]) moduleResp.get(1)), ((Long) moduleResp.get(3)).intValue()); responses.add(m); @@ -927,74 +973,17 @@ public String toString() { * Create a AccessControlUser object from the ACL GETUSER reply. */ public static final Builder ACCESS_CONTROL_USER = new Builder() { - @SuppressWarnings("unchecked") @Override public AccessControlUser build(Object data) { - if (data == null) { - return null; - } - - List objectList = (List) data; - if (objectList.isEmpty()) { - return null; - } - - AccessControlUser accessControlUser = new AccessControlUser(); - - // flags - List flags = (List) objectList.get(1); - for (Object f : flags) { - accessControlUser.addFlag(SafeEncoder.encode((byte[]) f)); - } - - // passwords - List passwords = (List) objectList.get(3); - for (Object p : passwords) { - accessControlUser.addPassword(SafeEncoder.encode((byte[]) p)); - } - - // commands - accessControlUser.setCommands(SafeEncoder.encode((byte[]) objectList.get(5))); - - // Redis 7 --> - boolean withSelectors = objectList.size() >= 12; - if (!withSelectors) { - - // keys - List keys = (List) objectList.get(7); - for (Object k : keys) { - accessControlUser.addKey(SafeEncoder.encode((byte[]) k)); - } - - // Redis 6.2 --> - // channels - if (objectList.size() >= 10) { - List channels = (List) objectList.get(9); - for (Object channel : channels) { - accessControlUser.addChannel(SafeEncoder.encode((byte[]) channel)); - } - } - - } else { - // TODO: Proper implementation of ACL V2. - - // keys - accessControlUser.addKeys(SafeEncoder.encode((byte[]) objectList.get(7))); - - // channels - accessControlUser.addChannels(SafeEncoder.encode((byte[]) objectList.get(9))); - } - - // selectors - // TODO: Proper implementation of ACL V2. - return accessControlUser; + Map map = ENCODED_OBJECT_MAP.build(data); + if (map == null) return null; + return new AccessControlUser(map); } @Override public String toString() { return "AccessControlUser"; } - }; /** @@ -1210,41 +1199,24 @@ public String toString() { @Override public List>> build(Object data) { if (data == null) return null; - List streamObjects = (List) data; - - List>> result = new ArrayList<>(streamObjects.size()); - for (Object streamObj : streamObjects) { - List stream = (List) streamObj; - String streamKey = STRING.build(stream.get(0)); - List streamEntries = STREAM_ENTRY_LIST.build(stream.get(1)); - result.add(KeyValue.of(streamKey, streamEntries)); - } - - return result; - } - - @Override - public String toString() { - return "List>>"; - } - }; - - public static final Builder>>> STREAM_READ_RESPONSE_RESP3 - = new Builder>>>() { - @Override - public List>> build(Object data) { - if (data == null) return null; - List streamObjects = (List) data; - - List>> result = new ArrayList<>(streamObjects.size() / 2); - Iterator iter = streamObjects.iterator(); - while (iter.hasNext()) { - String streamKey = STRING.build(iter.next()); - List streamEntries = STREAM_ENTRY_LIST.build(iter.next()); - result.add(KeyValue.of(streamKey, streamEntries)); + List list = (List) data; + if (list.isEmpty()) return Collections.emptyList(); + + if (list.get(0) instanceof KeyValue) { + return ((List) list).stream() + .map(kv -> new KeyValue<>(STRING.build(kv.getKey()), + STREAM_ENTRY_LIST.build(kv.getValue()))) + .collect(Collectors.toList()); + } else { + List>> result = new ArrayList<>(list.size()); + for (Object streamObj : list) { + List stream = (List) streamObj; + String streamKey = STRING.build(stream.get(0)); + List streamEntries = STREAM_ENTRY_LIST.build(stream.get(1)); + result.add(KeyValue.of(streamKey, streamEntries)); + } + return result; } - - return result; } @Override @@ -1628,18 +1600,32 @@ private static Map createMapFromDecodingFunctions(Iterator createMapFromDecodingFunctions(Iterator iterator, Map mappingFunctions, Collection backupBuilders) { + if (!iterator.hasNext()) { + return Collections.emptyMap(); + } + Map resultMap = new HashMap<>(); while (iterator.hasNext()) { + final Object tempObject = iterator.next(); + final String mapKey; + final Object rawValue; + + if (tempObject instanceof KeyValue) { + KeyValue kv = (KeyValue) tempObject; + mapKey = STRING.build(kv.getKey()); + rawValue = kv.getValue(); + } else { + mapKey = STRING.build(tempObject); + rawValue = iterator.next(); + } - String mapKey = STRING.build(iterator.next()); if (mappingFunctions.containsKey(mapKey)) { - resultMap.put(mapKey, mappingFunctions.get(mapKey).build(iterator.next())); + resultMap.put(mapKey, mappingFunctions.get(mapKey).build(rawValue)); } else { // For future - if we don't find an element in our builder map - Object unknownData = iterator.next(); Collection builders = backupBuilders != null ? backupBuilders : mappingFunctions.values(); for (Builder b : builders) { try { - resultMap.put(mapKey, b.build(unknownData)); + resultMap.put(mapKey, b.build(rawValue)); break; } catch (ClassCastException e) { // We continue with next builder @@ -1668,34 +1654,51 @@ public LCSMatchResult build(Object data) { List matchedPositions = new ArrayList<>(); List objectList = (List) data; - if ("matches".equalsIgnoreCase(STRING.build(objectList.get(0)))) { - List matches = (List)objectList.get(1); - for (Object obj : matches) { - if (obj instanceof List) { - List positions = (List) obj; - Position a = new Position( - LONG.build(((List) positions.get(0)).get(0)), - LONG.build(((List) positions.get(0)).get(1)) - ); - Position b = new Position( - LONG.build(((List) positions.get(1)).get(0)), - LONG.build(((List) positions.get(1)).get(1)) - ); - long matchLen = 0; - if (positions.size() >= 3) { - matchLen = LONG.build(positions.get(2)); - } - matchedPositions.add(new MatchedPosition(a, b, matchLen)); + if (objectList.get(0) instanceof KeyValue) { + Iterator iterator = objectList.iterator(); + while (iterator.hasNext()) { + KeyValue kv = (KeyValue) iterator.next(); + if ("matches".equalsIgnoreCase(STRING.build(kv.getKey()))) { + addMatchedPosition(matchedPositions, kv.getValue()); + } else if ("len".equalsIgnoreCase(STRING.build(kv.getKey()))) { + len = LONG.build(kv.getValue()); + } + } + } else { + for (int i = 0; i < objectList.size(); i += 2) { + if ("matches".equalsIgnoreCase(STRING.build(objectList.get(i)))) { + addMatchedPosition(matchedPositions, objectList.get(i + 1)); + } else if ("len".equalsIgnoreCase(STRING.build(objectList.get(i)))) { + len = LONG.build(objectList.get(i + 1)); } } } - if ("len".equalsIgnoreCase(STRING.build(objectList.get(2)))) { - len = LONG.build(objectList.get(3)); - } return new LCSMatchResult(matchedPositions, len); } } + + private void addMatchedPosition(List matchedPositions, Object o) { + List matches = (List) o; + for (Object obj : matches) { + if (obj instanceof List) { + List positions = (List) obj; + Position a = new Position( + LONG.build(((List) positions.get(0)).get(0)), + LONG.build(((List) positions.get(0)).get(1)) + ); + Position b = new Position( + LONG.build(((List) positions.get(1)).get(0)), + LONG.build(((List) positions.get(1)).get(1)) + ); + long matchLen = 0; + if (positions.size() >= 3) { + matchLen = LONG.build(positions.get(2)); + } + matchedPositions.add(new MatchedPosition(a, b, matchLen)); + } + } + } }; public static final Builder> STRING_MAP_FROM_PAIRS = new Builder>() { @@ -1864,5 +1867,4 @@ protected static SetFromList of(List list) { private BuilderFactory() { throw new InstantiationError("Must not instantiate this class"); } - } diff --git a/src/main/java/redis/clients/jedis/CommandObjects.java b/src/main/java/redis/clients/jedis/CommandObjects.java index a7198c3b3d..d3d77db3b4 100644 --- a/src/main/java/redis/clients/jedis/CommandObjects.java +++ b/src/main/java/redis/clients/jedis/CommandObjects.java @@ -2637,7 +2637,7 @@ public final CommandObject>>> xread( Set> entrySet = streams.entrySet(); entrySet.forEach(entry -> args.key(entry.getKey())); entrySet.forEach(entry -> args.add(entry.getValue())); - return new CommandObject<>(args, getStreamReadResponseBuilder()); + return new CommandObject<>(args, BuilderFactory.STREAM_READ_RESPONSE); } public final CommandObject>>> xreadGroup( @@ -2649,7 +2649,7 @@ public final CommandObject>>> xreadGrou Set> entrySet = streams.entrySet(); entrySet.forEach(entry -> args.key(entry.getKey())); entrySet.forEach(entry -> args.add(entry.getValue())); - return new CommandObject<>(args, getStreamReadResponseBuilder()); + return new CommandObject<>(args, BuilderFactory.STREAM_READ_RESPONSE); } public final CommandObject> xread(XReadParams xReadParams, Map.Entry... streams) { @@ -2676,11 +2676,6 @@ public final CommandObject> xreadGroup(byte[] groupName, byte[] con } return new CommandObject<>(args, BuilderFactory.BINARY_LIST); } - - private Builder>>> getStreamReadResponseBuilder() { - if (proto == RedisProtocol.RESP3) return BuilderFactory.STREAM_READ_RESPONSE_RESP3; - return BuilderFactory.STREAM_READ_RESPONSE; - } // Stream commands // Scripting commands diff --git a/src/main/java/redis/clients/jedis/Jedis.java b/src/main/java/redis/clients/jedis/Jedis.java index fc49d9761c..e8c15864e3 100644 --- a/src/main/java/redis/clients/jedis/Jedis.java +++ b/src/main/java/redis/clients/jedis/Jedis.java @@ -3572,17 +3572,17 @@ public List roleBinary() { * @return Bulk reply. */ @Override - public List configGet(final byte[] pattern) { + public Map configGet(final byte[] pattern) { checkIsInMultiOrPipeline(); connection.sendCommand(Command.CONFIG, Keyword.GET.getRaw(), pattern); - return connection.getBinaryMultiBulkReply(); + return BuilderFactory.BINARY_MAP.build(connection.getOne()); } @Override - public List configGet(byte[]... patterns) { + public Map configGet(byte[]... patterns) { checkIsInMultiOrPipeline(); connection.sendCommand(Command.CONFIG, joinParameters(Keyword.GET.getRaw(), patterns)); - return connection.getBinaryMultiBulkReply(); + return BuilderFactory.BINARY_MAP.build(connection.getOne()); } /** @@ -3671,6 +3671,15 @@ public String configSet(final byte[]... parameterValues) { return connection.getStatusCodeReply(); } + @Override + public String configSetBinary(Map parameterValues) { + checkIsInMultiOrPipeline(); + CommandArguments args = new CommandArguments(Command.CONFIG).add(Keyword.SET); + parameterValues.forEach((k, v) -> args.add(k).add(v)); + connection.sendCommand(args); + return connection.getStatusCodeReply(); + } + @Override public long strlen(final byte[] key) { checkIsInMultiOrPipeline(); @@ -4109,23 +4118,16 @@ public String aclSetUser(byte[] name) { } @Override - public String aclSetUser(byte[] name, byte[]... keys) { + public String aclSetUser(byte[] name, byte[]... rules) { checkIsInMultiOrPipeline(); - connection.sendCommand(ACL, joinParameters(SETUSER.getRaw(), name, keys)); + connection.sendCommand(ACL, joinParameters(SETUSER.getRaw(), name, rules)); return connection.getStatusCodeReply(); } @Override - public long aclDelUser(byte[] name) { + public long aclDelUser(byte[]... names) { checkIsInMultiOrPipeline(); - connection.sendCommand(ACL, DELUSER.getRaw(), name); - return connection.getIntegerReply(); - } - - @Override - public long aclDelUser(byte[] name, byte[]... names) { - checkIsInMultiOrPipeline(); - connection.sendCommand(ACL, joinParameters(DELUSER.getRaw(), name, names)); + connection.sendCommand(ACL, joinParameters(DELUSER.getRaw(), names)); return connection.getIntegerReply(); } @@ -7864,17 +7866,17 @@ public List role() { * @return Bulk reply. */ @Override - public List configGet(final String pattern) { + public Map configGet(final String pattern) { checkIsInMultiOrPipeline(); connection.sendCommand(Command.CONFIG, Keyword.GET.name(), pattern); - return connection.getMultiBulkReply(); + return BuilderFactory.STRING_MAP.build(connection.getOne()); } @Override - public List configGet(String... patterns) { + public Map configGet(String... patterns) { checkIsInMultiOrPipeline(); connection.sendCommand(Command.CONFIG, joinParameters(Keyword.GET.name(), patterns)); - return connection.getMultiBulkReply(); + return BuilderFactory.STRING_MAP.build(connection.getOne()); } /** @@ -7920,6 +7922,15 @@ public String configSet(final String... parameterValues) { return connection.getStatusCodeReply(); } + @Override + public String configSet(Map parameterValues) { + checkIsInMultiOrPipeline(); + CommandArguments args = new CommandArguments(Command.CONFIG).add(Keyword.SET); + parameterValues.forEach((k, v) -> args.add(k).add(v)); + connection.sendCommand(args); + return connection.getStatusCodeReply(); + } + public long publish(final String channel, final String message) { checkIsInMultiOrPipeline(); connection.sendCommand(PUBLISH, channel, message); @@ -8360,23 +8371,16 @@ public String aclSetUser(final String name) { } @Override - public String aclSetUser(String name, String... params) { + public String aclSetUser(String name, String... rules) { checkIsInMultiOrPipeline(); - connection.sendCommand(ACL, joinParameters(SETUSER.name(), name, params)); + connection.sendCommand(ACL, joinParameters(SETUSER.name(), name, rules)); return connection.getStatusCodeReply(); } @Override - public long aclDelUser(final String name) { - checkIsInMultiOrPipeline(); - connection.sendCommand(ACL, DELUSER.name(), name); - return connection.getIntegerReply(); - } - - @Override - public long aclDelUser(final String name, String... names) { + public long aclDelUser(final String... names) { checkIsInMultiOrPipeline(); - connection.sendCommand(ACL, joinParameters(DELUSER.name(), name, names)); + connection.sendCommand(ACL, joinParameters(DELUSER.name(), names)); return connection.getIntegerReply(); } @@ -8384,7 +8388,7 @@ public long aclDelUser(final String name, String... names) { public AccessControlUser aclGetUser(final String name) { checkIsInMultiOrPipeline(); connection.sendCommand(ACL, GETUSER.name(), name); - return BuilderFactory.ACCESS_CONTROL_USER.build(connection.getObjectMultiBulkReply()); + return BuilderFactory.ACCESS_CONTROL_USER.build(connection.getOne()); } @Override @@ -8412,28 +8416,28 @@ public String aclWhoAmI() { public List aclCat() { checkIsInMultiOrPipeline(); connection.sendCommand(ACL, CAT); - return BuilderFactory.STRING_LIST.build(connection.getObjectMultiBulkReply()); + return BuilderFactory.STRING_LIST.build(connection.getOne()); } @Override public List aclCat(String category) { checkIsInMultiOrPipeline(); connection.sendCommand(ACL, CAT.name(), category); - return BuilderFactory.STRING_LIST.build(connection.getObjectMultiBulkReply()); + return BuilderFactory.STRING_LIST.build(connection.getOne()); } @Override public List aclLog() { checkIsInMultiOrPipeline(); connection.sendCommand(ACL, LOG); - return BuilderFactory.ACCESS_CONTROL_LOG_ENTRY_LIST.build(connection.getObjectMultiBulkReply()); + return BuilderFactory.ACCESS_CONTROL_LOG_ENTRY_LIST.build(connection.getOne()); } @Override public List aclLog(int limit) { checkIsInMultiOrPipeline(); connection.sendCommand(ACL, LOG.getRaw(), toByteArray(limit)); - return BuilderFactory.ACCESS_CONTROL_LOG_ENTRY_LIST.build(connection.getObjectMultiBulkReply()); + return BuilderFactory.ACCESS_CONTROL_LOG_ENTRY_LIST.build(connection.getOne()); } @Override @@ -9185,7 +9189,7 @@ public String moduleUnload(final String name) { public List moduleList() { checkIsInMultiOrPipeline(); connection.sendCommand(Command.MODULE, LIST); - return BuilderFactory.MODULE_LIST.build(connection.getObjectMultiBulkReply()); + return BuilderFactory.MODULE_LIST.build(connection.getOne()); } @Override diff --git a/src/main/java/redis/clients/jedis/Module.java b/src/main/java/redis/clients/jedis/Module.java index 3dbf9b46cf..0c3d356c21 100644 --- a/src/main/java/redis/clients/jedis/Module.java +++ b/src/main/java/redis/clients/jedis/Module.java @@ -1,5 +1,7 @@ package redis.clients.jedis; +// TODO: 'resps' package +// TODO: remove public class Module { private final String name; diff --git a/src/main/java/redis/clients/jedis/Protocol.java b/src/main/java/redis/clients/jedis/Protocol.java index 154549d2a3..3f273877b5 100644 --- a/src/main/java/redis/clients/jedis/Protocol.java +++ b/src/main/java/redis/clients/jedis/Protocol.java @@ -10,6 +10,7 @@ import redis.clients.jedis.exceptions.*; import redis.clients.jedis.args.Rawable; import redis.clients.jedis.commands.ProtocolCommand; +import redis.clients.jedis.util.KeyValue; import redis.clients.jedis.util.RedisInputStream; import redis.clients.jedis.util.RedisOutputStream; import redis.clients.jedis.util.SafeEncoder; @@ -133,7 +134,6 @@ private static String[] parseTargetHostAndSlot(String clusterRedirectResponse) { private static Object process(final RedisInputStream is) { final byte b = is.readByte(); //System.out.println((char) b); - int num; switch (b) { case PLUS_BYTE: return is.readLineBytes(); @@ -141,9 +141,7 @@ private static Object process(final RedisInputStream is) { case EQUAL_BYTE: return processBulkReply(is); case ASTERISK_BYTE: - num = is.readIntCrLf(); - if (num == -1) return null; - return processMultiBulkReply(num, is); + return processMultiBulkReply(is); case UNDERSCORE_BYTE: return is.readNullCrLf(); case HASH_BYTE: @@ -155,17 +153,11 @@ private static Object process(final RedisInputStream is) { case LEFT_BRACE_BYTE: return is.readBigIntegerCrLf(); case PERCENT_BYTE: // TODO: currently just to start working with HELLO - num = is.readIntCrLf(); - if (num == -1) return null; - return processMultiBulkReply(2 * num, is); + return processMapKeyValueReply(is); case TILDE_BYTE: // TODO: - num = is.readIntCrLf(); - if (num == -1) return null; - return processMultiBulkReply(num, is); + return processMultiBulkReply(is); case GREATER_THAN_BYTE: - num = is.readIntCrLf(); - if (num == -1) return null; - return processMultiBulkReply(num, is); + return processMultiBulkReply(is); case MINUS_BYTE: processError(is); return null; @@ -198,9 +190,10 @@ private static byte[] processBulkReply(final RedisInputStream is) { return read; } - // private static List processMultiBulkReply(final RedisInputStream is) { - private static List processMultiBulkReply(final int num, final RedisInputStream is) { - // final int num = is.readIntCrLf(); + private static List processMultiBulkReply(final RedisInputStream is) { + // private static List processMultiBulkReply(final int num, final RedisInputStream is) { + final int num = is.readIntCrLf(); + if (num == -1) return null; final List ret = new ArrayList<>(num); for (int i = 0; i < num; i++) { try { @@ -212,6 +205,18 @@ private static List processMultiBulkReply(final int num, final RedisInpu return ret; } + // private static List processMultiBulkReply(final RedisInputStream is) { + // private static List processMultiBulkReply(final int num, final RedisInputStream is) { + private static List processMapKeyValueReply(final RedisInputStream is) { + final int num = is.readIntCrLf(); + if (num == -1) return null; + final List ret = new ArrayList<>(num); + for (int i = 0; i < num; i++) { + ret.add(new KeyValue(process(is), process(is))); + } + return ret; + } + public static Object read(final RedisInputStream is) { return process(is); } diff --git a/src/main/java/redis/clients/jedis/commands/AccessControlLogBinaryCommands.java b/src/main/java/redis/clients/jedis/commands/AccessControlLogBinaryCommands.java index 925286d38f..98aaec8c61 100644 --- a/src/main/java/redis/clients/jedis/commands/AccessControlLogBinaryCommands.java +++ b/src/main/java/redis/clients/jedis/commands/AccessControlLogBinaryCommands.java @@ -74,30 +74,20 @@ public interface AccessControlLogBinaryCommands { * Create an ACL for the specified user, while specifying the rules. * * @param name user who receives an acl - * @param keys the acl rules for the specified user + * @param rules the acl rules for the specified user * @see ACL SETUSER * @return A string containing OK on success */ - String aclSetUser(byte[] name, byte[]... keys); + String aclSetUser(byte[] name, byte[]... rules); /** * Delete the specified user, from the ACL. * - * @param name The username to delete + * @param names The username to delete * @see ACL DELUSER * @return The number of users delete */ - long aclDelUser(byte[] name); - - /** - * Delete the specified users, from the ACL. - * - * @param name The username to delete - * @param names Other usernames to delete - * @see ACL DELUSER - * @return The number of users delete - */ - long aclDelUser(byte[] name, byte[]... names); + long aclDelUser(byte[]... names); /** * Show the available ACL categories. diff --git a/src/main/java/redis/clients/jedis/commands/AccessControlLogCommands.java b/src/main/java/redis/clients/jedis/commands/AccessControlLogCommands.java index b6f6b85cde..a65207c038 100644 --- a/src/main/java/redis/clients/jedis/commands/AccessControlLogCommands.java +++ b/src/main/java/redis/clients/jedis/commands/AccessControlLogCommands.java @@ -76,30 +76,20 @@ public interface AccessControlLogCommands { * Create an ACL for the specified user, while specifying the rules. * * @param name user who receives an acl - * @param keys the acl rules for the specified user + * @param rules the acl rules for the specified user * @see ACL SETUSER * @return A string containing OK on success */ - String aclSetUser(String name, String... keys); + String aclSetUser(String name, String... rules); /** * Delete the specified user, from the ACL. * - * @param name The username to delete + * @param names The usernames to delete * @see ACL DELUSER * @return The number of users delete */ - long aclDelUser(String name); - - /** - * Delete the specified users, from the ACL. - * - * @param name The username to delete - * @param names Other usernames to delete - * @see ACL DELUSER - * @return The number of users delete - */ - long aclDelUser(String name, String... names); + long aclDelUser(String... names); /** * Show the available ACL categories. diff --git a/src/main/java/redis/clients/jedis/commands/ConfigCommands.java b/src/main/java/redis/clients/jedis/commands/ConfigCommands.java index 268205b1cb..3c099043da 100644 --- a/src/main/java/redis/clients/jedis/commands/ConfigCommands.java +++ b/src/main/java/redis/clients/jedis/commands/ConfigCommands.java @@ -1,6 +1,6 @@ package redis.clients.jedis.commands; -import java.util.List; +import java.util.Map; /** * The interface about managing configuration parameters of Redis server. @@ -13,7 +13,7 @@ public interface ConfigCommands { * @param pattern name of Redis server's configuration * @return config value of Redis server */ - List configGet(String pattern); + Map configGet(String pattern); /** * Used to read the configuration parameters of Redis server. @@ -21,7 +21,7 @@ public interface ConfigCommands { * @param patterns names of Redis server's configuration * @return values of Redis server's configuration */ - List configGet(String... patterns); + Map configGet(String... patterns); /** * Used to read the configuration parameters of Redis server. @@ -29,7 +29,7 @@ public interface ConfigCommands { * @param pattern name of Redis server's configuration * @return value of Redis server's configuration */ - List configGet(byte[] pattern); + Map configGet(byte[] pattern); /** * Used to read the configuration parameters of Redis server. @@ -37,7 +37,7 @@ public interface ConfigCommands { * @param patterns names of Redis server's configuration * @return values of Redis server's configuration */ - List configGet(byte[]... patterns); + Map configGet(byte[]... patterns); /** * Used in order to reconfigure the Redis server at run time without @@ -52,6 +52,8 @@ public interface ConfigCommands { String configSet(String... parameterValues); + String configSet(Map parameterValues); + /** * Used in order to reconfigure the Redis server at run time without * the need to restart. @@ -65,6 +67,8 @@ public interface ConfigCommands { String configSet(byte[]... parameterValues); + String configSetBinary(Map parameterValues); + /** * Resets the statistics reported by Redis using the INFO command. *

diff --git a/src/main/java/redis/clients/jedis/resps/AccessControlLogEntry.java b/src/main/java/redis/clients/jedis/resps/AccessControlLogEntry.java index d23b10acf9..09ef3a43bb 100644 --- a/src/main/java/redis/clients/jedis/resps/AccessControlLogEntry.java +++ b/src/main/java/redis/clients/jedis/resps/AccessControlLogEntry.java @@ -9,6 +9,7 @@ * can be access via getters. For future purpose there is also {@link #getlogEntry} method that * returns a generic {@code Map} - in case where more info is returned from a server */ +// TODO: remove public class AccessControlLogEntry implements Serializable { private static final long serialVersionUID = 1L; diff --git a/src/main/java/redis/clients/jedis/resps/AccessControlUser.java b/src/main/java/redis/clients/jedis/resps/AccessControlUser.java index 6013a8f760..2c9d0f9e78 100644 --- a/src/main/java/redis/clients/jedis/resps/AccessControlUser.java +++ b/src/main/java/redis/clients/jedis/resps/AccessControlUser.java @@ -1,75 +1,116 @@ package redis.clients.jedis.resps; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.StringJoiner; +// TODO: remove public class AccessControlUser { - private final List flags = new ArrayList<>(); - private final List keys = new ArrayList<>(); - private final List passwords = new ArrayList<>(); - private final List channels = new ArrayList<>(); - private String commands; + private final Map userInfo; + private final List flags; + private final List passwords; + private final String commands; + private final List keysList; + private final String keys; + private final List channelsList; + private final String channels; + private final List selectors; + + public AccessControlUser(Map map) { + this.userInfo = map; + + this.flags = (List) map.get("flags"); + + this.passwords = (List) map.get("passwords"); + + this.commands = (String) map.get("commands"); + + Object localKeys = map.get("keys"); + if (localKeys == null) { + this.keys = null; + this.keysList = null; + } else if (localKeys instanceof List) { + this.keysList = (List) localKeys; + this.keys = joinStrings(this.keysList); + } else { + this.keys = (String) localKeys; + this.keysList = Arrays.asList(this.keys.split(" ")); + } + + Object localChannels = map.get("channels"); + if (localChannels == null) { + this.channels = null; + this.channelsList = null; + } else if (localChannels instanceof List) { + this.channelsList = (List) localChannels; + this.channels = joinStrings(this.channelsList); + } else { + this.channels = (String) localChannels; + this.channelsList = Arrays.asList(this.channels.split(" ")); + } - public AccessControlUser() { + this.selectors = (List) map.get("selectors"); } - public void addFlag(String flag) { - flags.add(flag); + private static String joinStrings(List list) { + StringJoiner joiner = new StringJoiner(" "); + list.forEach(s -> joiner.add(s)); + return joiner.toString(); } public List getFlags() { return flags; } - public void addKey(String key) { - keys.add(key); - } - - public List getKeys() { - return keys; + /** + * @deprecated Use {@link AccessControlUser#getPasswords()}. + */ + @Deprecated + public List getPassword() { + return passwords; } - public void addKeys(String keys) { - if (!keys.isEmpty()) { - this.keys.addAll(Arrays.asList(keys.split(" "))); - } + public List getPasswords() { + return passwords; } - public void addPassword(String password) { - passwords.add(password); + public String getCommands() { + return commands; } - public List getPassword() { - return passwords; + /** + * @return Generic map containing all key-value pairs returned by the server + */ + public Map getUserInfo() { + return userInfo; } - public void addChannel(String channel) { - channels.add(channel); + public String getKeys() { + return keys; } - public List getChannels() { - return channels; + public List getKeysList() { + return keysList; } - public void addChannels(String channels) { - if (!channels.isEmpty()) { - this.channels.addAll(Arrays.asList(channels.split(" "))); - } + public List getChannelsList() { + return channelsList; } - public String getCommands() { - return commands; + public String getChannels() { + return channels; } - public void setCommands(String commands) { - this.commands = commands; + public List getSelectors() { + return selectors; } @Override public String toString() { return "AccessControlUser{" + "flags=" + flags + ", passwords=" + passwords - + ", commands='" + commands + "', keys='" + keys + "', channels='" + channels + "'}"; + + ", commands='" + commands + "', keys='" + keys + "', channels='" + channels + + "', selectors=" + selectors + "}"; } } diff --git a/src/main/java/redis/clients/jedis/resps/CommandDocument.java b/src/main/java/redis/clients/jedis/resps/CommandDocument.java index 219397811e..3e320d8dec 100644 --- a/src/main/java/redis/clients/jedis/resps/CommandDocument.java +++ b/src/main/java/redis/clients/jedis/resps/CommandDocument.java @@ -5,21 +5,55 @@ import static redis.clients.jedis.BuilderFactory.STRING; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import redis.clients.jedis.util.KeyValue; public class CommandDocument { + + private static final String SUMMARY_STR = "summary"; + private static final String SINCE_STR = "since"; + private static final String GROUP_STR = "group"; + private static final String COMPLEXITY_STR = "complexity"; + private static final String HISTORY_STR = "history"; + private final String summary; private final String since; private final String group; private final String complexity; private final List history; + @Deprecated public CommandDocument(String summary, String since, String group, String complexity, List history) { this.summary = summary; this.since = since; this.group = group; this.complexity = complexity; - this.history = history; + this.history = (List) history; + } + + public CommandDocument(Map map) { + this.summary = (String) map.get(SUMMARY_STR); + this.since = (String) map.get(SINCE_STR); + this.group = (String) map.get(GROUP_STR); + this.complexity = (String) map.get(COMPLEXITY_STR); + + List historyObject = (List) map.get(HISTORY_STR); + if (historyObject == null) { + this.history = null; + } else if (historyObject.isEmpty()) { + this.history = Collections.emptyList(); + } else if (historyObject.get(0) instanceof KeyValue) { + this.history = historyObject.stream().map(o -> (KeyValue) o) + .map(kv -> (String) kv.getKey() + ": " + (String) kv.getValue()) + .collect(Collectors.toList()); + } else { + this.history = historyObject.stream().map(o -> (List) o) + .map(l -> (String) l.get(0) + ": " + (String) l.get(1)) + .collect(Collectors.toList()); + } } public String getSummary() { @@ -42,6 +76,7 @@ public List getHistory() { return history; } + @Deprecated public static final Builder COMMAND_DOCUMENT_BUILDER = new Builder() { @Override public CommandDocument build(Object data) { diff --git a/src/main/java/redis/clients/jedis/resps/FunctionStats.java b/src/main/java/redis/clients/jedis/resps/FunctionStats.java index 917af0ce68..b8d55c181f 100644 --- a/src/main/java/redis/clients/jedis/resps/FunctionStats.java +++ b/src/main/java/redis/clients/jedis/resps/FunctionStats.java @@ -5,6 +5,7 @@ import java.util.Map; import redis.clients.jedis.Builder; import redis.clients.jedis.BuilderFactory; +import redis.clients.jedis.util.KeyValue; public class FunctionStats { @@ -28,12 +29,38 @@ public Map> getEngines() { @Override public FunctionStats build(Object data) { - List superMapList = (List) data; + if (data == null) return null; + List list = (List) data; + if (list.isEmpty()) return null; - Map runningScriptMap = superMapList.get(1) == null ? null - : BuilderFactory.ENCODED_OBJECT_MAP.build(superMapList.get(1)); + if (list.get(0) instanceof KeyValue) { - List enginesList = (List) superMapList.get(3); + Map runningScriptMap = null; + Map> enginesMap = null; + + for (KeyValue kv : (List) list) { + switch (BuilderFactory.STRING.build(kv.getKey())) { + case "running_script": + runningScriptMap = BuilderFactory.ENCODED_OBJECT_MAP.build(kv.getValue()); + break; + case "engines": + List ilist = (List) kv.getValue(); + enginesMap = new LinkedHashMap<>(ilist.size()); + for (KeyValue ikv : (List) kv.getValue()) { + enginesMap.put(BuilderFactory.STRING.build(ikv.getKey()), + BuilderFactory.ENCODED_OBJECT_MAP.build(ikv.getValue())); + } + break; + } + } + + return new FunctionStats(runningScriptMap, enginesMap); + } + + Map runningScriptMap = list.get(1) == null ? null + : BuilderFactory.ENCODED_OBJECT_MAP.build(list.get(1)); + + List enginesList = (List) list.get(3); Map> enginesMap = new LinkedHashMap<>(enginesList.size() / 2); for (int i = 0; i < enginesList.size(); i += 2) { diff --git a/src/main/java/redis/clients/jedis/resps/LibraryInfo.java b/src/main/java/redis/clients/jedis/resps/LibraryInfo.java index 7934ce7080..6471ac9ba7 100644 --- a/src/main/java/redis/clients/jedis/resps/LibraryInfo.java +++ b/src/main/java/redis/clients/jedis/resps/LibraryInfo.java @@ -8,7 +8,11 @@ import java.util.stream.Collectors; import redis.clients.jedis.Builder; +import redis.clients.jedis.BuilderFactory; +import redis.clients.jedis.util.KeyValue; +//[library_name=mylib, engine=LUA, functions=[[name=myfunc, description=null, flags=[]]], library_code=#!LUA name=mylib +// redis.register_function('myfunc', function(keys, args) return args[1] end)] public class LibraryInfo { private final String libraryName; @@ -46,15 +50,40 @@ public String getLibraryCode() { public static final Builder LIBRARY_BUILDER = new Builder() { @Override public LibraryInfo build(Object data) { - List objectList = (List) data; - String libname = STRING.build(objectList.get(1)); - String engine = STRING.build(objectList.get(3)); - List rawFunctions = (List) objectList.get(5); + if (data == null) return null; + List list = (List) data; + if (list.isEmpty()) return null; + + if (list.get(0) instanceof KeyValue) { + String libname = null, enginename = null, librarycode = null; + List> functions = null; + for (KeyValue kv : (List) list) { + switch (BuilderFactory.STRING.build(kv.getKey())) { + case "library_name": + libname = BuilderFactory.STRING.build(kv.getValue()); + break; + case "engine": + enginename = BuilderFactory.STRING.build(kv.getValue()); + break; + case "functions": + functions = ((List) kv.getValue()).stream().map(o -> ENCODED_OBJECT_MAP.build(o)).collect(Collectors.toList()); + break; + case "library_code": + librarycode = BuilderFactory.STRING.build(kv.getValue()); + break; + } + } + return new LibraryInfo(libname, enginename, functions, librarycode); + } + + String libname = STRING.build(list.get(1)); + String engine = STRING.build(list.get(3)); + List rawFunctions = (List) list.get(5); List> functions = rawFunctions.stream().map(o -> ENCODED_OBJECT_MAP.build(o)).collect(Collectors.toList()); - if (objectList.size() <= 6) { + if (list.size() <= 6) { return new LibraryInfo(libname, engine, functions); } - String code = STRING.build(objectList.get(7)); + String code = STRING.build(list.get(7)); return new LibraryInfo(libname, engine, functions, code); } }; diff --git a/src/main/java/redis/clients/jedis/resps/Slowlog.java b/src/main/java/redis/clients/jedis/resps/Slowlog.java index 13898af98d..58ccf53e21 100644 --- a/src/main/java/redis/clients/jedis/resps/Slowlog.java +++ b/src/main/java/redis/clients/jedis/resps/Slowlog.java @@ -2,7 +2,7 @@ import java.util.ArrayList; import java.util.List; - +import redis.clients.jedis.BuilderFactory; import redis.clients.jedis.HostAndPort; import redis.clients.jedis.util.SafeEncoder; @@ -19,17 +19,11 @@ public class Slowlog { @SuppressWarnings("unchecked") private Slowlog(List properties) { - super(); this.id = (Long) properties.get(0); this.timeStamp = (Long) properties.get(1); this.executionTime = (Long) properties.get(2); - List bargs = (List) properties.get(3); - this.args = new ArrayList<>(bargs.size()); - - for (byte[] barg : bargs) { - this.args.add(SafeEncoder.encode(barg)); - } + this.args = BuilderFactory.STRING_LIST.build(properties.get(3)); if (properties.size() == 4) return; this.clientIpPort = HostAndPort.from(SafeEncoder.encode((byte[]) properties.get(4))); @@ -43,7 +37,6 @@ public static List from(List nestedMultiBulkReply) { List properties = (List) obj; logs.add(new Slowlog(properties)); } - return logs; } diff --git a/src/main/java/redis/clients/jedis/timeseries/TSInfo.java b/src/main/java/redis/clients/jedis/timeseries/TSInfo.java index 2519d26232..bc37edf8fe 100644 --- a/src/main/java/redis/clients/jedis/timeseries/TSInfo.java +++ b/src/main/java/redis/clients/jedis/timeseries/TSInfo.java @@ -2,13 +2,14 @@ import java.util.ArrayList; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import redis.clients.jedis.Builder; import redis.clients.jedis.BuilderFactory; import redis.clients.jedis.util.DoublePrecision; +import redis.clients.jedis.util.KeyValue; import redis.clients.jedis.util.SafeEncoder; public class TSInfo { @@ -127,19 +128,18 @@ public TSInfo build(Object data) { } }; - // TODO: unify INFO builders public static Builder TIMESERIES_INFO_RESP3 = new Builder() { @Override public TSInfo build(Object data) { - List list = (List) data; + List list = (List) data; Map properties = new HashMap<>(); Map labels = null; Map rules = null; List> chunks = null; - for (int i = 0; i < list.size(); i += 2) { - String prop = SafeEncoder.encode((byte[]) list.get(i)); - Object value = list.get(i + 1); + for (KeyValue propertyValue : list) { + String prop = BuilderFactory.STRING.build(propertyValue.getKey()); + Object value = propertyValue.getValue(); if (value instanceof List) { switch (prop) { case LABELS_PROPERTY: @@ -147,24 +147,26 @@ public TSInfo build(Object data) { value = labels; break; case RULES_PROPERTY: - List rulesDataList = (List) value; - Map> rulesValueMap = new HashMap<>(rulesDataList.size() / 2, 1f); + List rulesDataList = (List) value; + Map> rulesValueMap = new HashMap<>(rulesDataList.size(), 1f); rules = new HashMap<>(rulesDataList.size()); - for (Iterator iterator = rulesDataList.iterator(); iterator.hasNext();) { - String ruleName = BuilderFactory.STRING.build(iterator.next()); - List ruleValueList = BuilderFactory.ENCODED_OBJECT_LIST.build(iterator.next()); + for (KeyValue rkv : rulesDataList) { + String ruleName = BuilderFactory.STRING.build(rkv.getKey()); + List ruleValueList = BuilderFactory.ENCODED_OBJECT_LIST.build(rkv.getValue()); rulesValueMap.put(ruleName, ruleValueList); rules.put(ruleName, new Rule(ruleName, ruleValueList)); } value = rulesValueMap; break; case CHUNKS_PROPERTY: - List chunksDataList = (List) value; + List> chunksDataList = (List>) value; List> chunksValueList = new ArrayList<>(chunksDataList.size()); chunks = new ArrayList<>(chunksDataList.size()); - for (Object chunkData : chunksDataList) { - Map chunk = BuilderFactory.ENCODED_OBJECT_MAP.build(chunkData); - chunksValueList.add(new HashMap<>(chunk)); + for (List chunkDataAsList : chunksDataList) { + Map chunk = chunkDataAsList.stream() + .collect(Collectors.toMap(kv -> BuilderFactory.STRING.build(kv.getKey()), + kv -> BuilderFactory.ENCODED_OBJECT.build(kv.getValue()))); + chunksValueList.add(chunk); chunks.add(chunk); } value = chunksValueList; @@ -174,7 +176,7 @@ public TSInfo build(Object data) { break; } } else if (value instanceof byte[]) { - value = SafeEncoder.encode((byte[]) value); + value = BuilderFactory.STRING.build(value); if (DUPLICATE_POLICY_PROPERTY.equals(prop)) { try { value = DuplicatePolicy.valueOf(((String) value).toUpperCase()); diff --git a/src/main/java/redis/clients/jedis/timeseries/TimeSeriesBuilderFactory.java b/src/main/java/redis/clients/jedis/timeseries/TimeSeriesBuilderFactory.java index 0b90d4328e..96e40d5769 100644 --- a/src/main/java/redis/clients/jedis/timeseries/TimeSeriesBuilderFactory.java +++ b/src/main/java/redis/clients/jedis/timeseries/TimeSeriesBuilderFactory.java @@ -1,6 +1,5 @@ package redis.clients.jedis.timeseries; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -9,7 +8,7 @@ import redis.clients.jedis.Builder; import redis.clients.jedis.BuilderFactory; -import redis.clients.jedis.util.SafeEncoder; +import redis.clients.jedis.util.KeyValue; public final class TimeSeriesBuilderFactory { @@ -49,31 +48,32 @@ public Map build(Object data) { = new Builder>() { @Override public Map build(Object data) { - List dataList = (List) data; + List dataList = (List) data; Map map = new LinkedHashMap<>(dataList.size() / 2, 1f); - for (Iterator iterator = dataList.iterator(); iterator.hasNext();) { - String key = BuilderFactory.STRING.build(iterator.next()); - List valueList = (List) iterator.next(); + for (KeyValue kv : dataList) { + String key = BuilderFactory.STRING.build(kv.getKey()); + List valueList = (List) kv.getValue(); TSMRangeElements elements; switch (valueList.size()) { case 3: List aggrMapObj = (List) valueList.get(1); - assert "aggregators".equalsIgnoreCase(BuilderFactory.STRING.build(aggrMapObj.get(0))); + KeyValue aggKV = (KeyValue) aggrMapObj.get(0); + assert "aggregators".equalsIgnoreCase(BuilderFactory.STRING.build(aggKV.getKey())); elements = new TSMRangeElements(key, BuilderFactory.STRING_MAP.build(valueList.get(0)), - ((List) aggrMapObj.get(1)).stream().map(BuilderFactory.STRING::build) + ((List) aggKV.getValue()).stream().map(BuilderFactory.STRING::build) .map(AggregationType::safeValueOf).collect(Collectors.toList()), TIMESERIES_ELEMENT_LIST.build(valueList.get(2))); break; case 4: - List rdcMapObj = (List) valueList.get(1); - assert "reducers".equalsIgnoreCase(BuilderFactory.STRING.build(rdcMapObj.get(0))); - List srcMapObj = (List) valueList.get(2); - assert "sources".equalsIgnoreCase(BuilderFactory.STRING.build(srcMapObj.get(0))); + List rdcMapObj = (List) valueList.get(1); + assert "reducers".equalsIgnoreCase(BuilderFactory.STRING.build(rdcMapObj.get(0).getKey())); + List srcMapObj = (List) valueList.get(2); + assert "sources".equalsIgnoreCase(BuilderFactory.STRING.build(srcMapObj.get(0).getKey())); elements = new TSMRangeElements(key, BuilderFactory.STRING_MAP.build(valueList.get(0)), - BuilderFactory.STRING_LIST.build(rdcMapObj.get(1)), - BuilderFactory.STRING_LIST.build(srcMapObj.get(1)), + BuilderFactory.STRING_LIST.build(rdcMapObj.get(0).getValue()), + BuilderFactory.STRING_LIST.build(srcMapObj.get(0).getValue()), TIMESERIES_ELEMENT_LIST.build(valueList.get(3))); break; default: @@ -101,11 +101,11 @@ public Map build(Object data) { = new Builder>() { @Override public Map build(Object data) { - List dataList = (List) data; + List dataList = (List) data; Map map = new LinkedHashMap<>(dataList.size()); - for (Iterator iterator = dataList.iterator(); iterator.hasNext();) { - String key = BuilderFactory.STRING.build(iterator.next()); - List valueList = (List) iterator.next(); + for (KeyValue kv : dataList) { + String key = BuilderFactory.STRING.build(kv.getKey()); + List valueList = (List) kv.getValue(); TSMGetElement value = new TSMGetElement(key, BuilderFactory.STRING_MAP.build(valueList.get(0)), TIMESERIES_ELEMENT.build(valueList.get(1))); diff --git a/src/main/java/redis/clients/jedis/util/SafeEncoder.java b/src/main/java/redis/clients/jedis/util/SafeEncoder.java index 314d721851..fb5d8fb24e 100644 --- a/src/main/java/redis/clients/jedis/util/SafeEncoder.java +++ b/src/main/java/redis/clients/jedis/util/SafeEncoder.java @@ -46,6 +46,11 @@ public static Object encodeObject(Object dataToEncode) { return SafeEncoder.encode((byte[]) dataToEncode); } + if (dataToEncode instanceof KeyValue) { + KeyValue keyValue = (KeyValue) dataToEncode; + return new KeyValue<>(encodeObject(keyValue.getKey()), encodeObject(keyValue.getValue())); + } + if (dataToEncode instanceof List) { List arrayToDecode = (List) dataToEncode; List returnValueArray = new ArrayList(arrayToDecode.size()); diff --git a/src/test/java/redis/clients/jedis/JedisTest.java b/src/test/java/redis/clients/jedis/JedisTest.java index f108c68718..df56ea4fb2 100644 --- a/src/test/java/redis/clients/jedis/JedisTest.java +++ b/src/test/java/redis/clients/jedis/JedisTest.java @@ -100,10 +100,12 @@ public void connectOnResp3ProtocolShortcut() { @Test public void timeoutConnection() throws Exception { + final String TIMEOUT_STR = "timeout"; + Jedis jedis = new Jedis("localhost", 6379, 15000); jedis.auth("foobared"); // read current config - final String timeout = jedis.configGet("timeout").get(1); + final String timeout = jedis.configGet(TIMEOUT_STR).get(TIMEOUT_STR); try { jedis.configSet("timeout", "1"); Thread.sleep(5000); @@ -118,7 +120,7 @@ public void timeoutConnection() throws Exception { // reset config jedis = new Jedis("localhost", 6379); jedis.auth("foobared"); - jedis.configSet("timeout", timeout); + jedis.configSet(TIMEOUT_STR, timeout); jedis.close(); } } diff --git a/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java index 777f8badac..9d4f3c6568 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java @@ -4,6 +4,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.startsWith; + import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -13,7 +14,6 @@ import java.util.Arrays; import java.util.List; -import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; import org.junit.After; import org.junit.BeforeClass; @@ -88,7 +88,7 @@ public void addAndRemoveUser() { public void aclUsers() { List users = jedis.aclUsers(); assertEquals(2, users.size()); - assertThat(users, CoreMatchers.hasItem("default")); + assertThat(users, Matchers.hasItem("default")); assertEquals(2, jedis.aclUsersBinary().size()); // Test binary } @@ -101,7 +101,7 @@ public void aclGetUser() { assertFalse(userInfo.getFlags().isEmpty()); assertEquals(1, userInfo.getPassword().size()); assertEquals("+@all", userInfo.getCommands()); - assertEquals("~*", userInfo.getKeys().get(0)); + assertEquals("~*", userInfo.getKeys()); // create new user jedis.aclSetUser(USER_NAME); @@ -504,7 +504,7 @@ public void aclBinaryCommandsTest() { assertThat(userInfo.getCommands(), containsString("+@all")); assertThat(userInfo.getCommands(), containsString("-@string")); assertThat(userInfo.getCommands(), containsString("+debug|digest")); - assertEquals("&testchannel:*", userInfo.getChannels().get(0)); + assertEquals("&testchannel:*", userInfo.getChannels()); jedis.aclDelUser(USER_NAME.getBytes()); diff --git a/src/test/java/redis/clients/jedis/commands/jedis/AllKindOfValuesCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/AllKindOfValuesCommandsTest.java index 48e8d38942..fc3f4c9c3e 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/AllKindOfValuesCommandsTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/AllKindOfValuesCommandsTest.java @@ -1,5 +1,6 @@ package redis.clients.jedis.commands.jedis; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.*; import static redis.clients.jedis.Protocol.Command.BLPOP; @@ -14,6 +15,8 @@ import static redis.clients.jedis.params.ScanParams.SCAN_POINTER_START_BINARY; import java.util.*; +import org.hamcrest.Matchers; +import org.junit.Assume; import org.junit.Test; import redis.clients.jedis.HostAndPort; @@ -25,10 +28,13 @@ import redis.clients.jedis.args.FlushMode; import redis.clients.jedis.params.RestoreParams; import redis.clients.jedis.HostAndPorts; +import redis.clients.jedis.RedisProtocol; import redis.clients.jedis.util.SafeEncoder; import redis.clients.jedis.exceptions.JedisDataException; import redis.clients.jedis.params.SetParams; import redis.clients.jedis.util.AssertUtil; +import redis.clients.jedis.util.KeyValue; +import redis.clients.jedis.util.RedisProtocolUtil; public class AllKindOfValuesCommandsTest extends JedisCommandsTestBase { final byte[] bfoo = { 0x01, 0x02, 0x03, 0x04 }; @@ -971,17 +977,60 @@ public void sendBlockingCommandTest() { } @Test - public void encodeCompleteResponse() { + public void encodeCompleteResponsePing() { + assertEquals("PONG", SafeEncoder.encodeObject(jedis.sendCommand(PING))); + } + + @Test + public void encodeCompleteResponseHgetall() { + Assume.assumeFalse(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); + + HashMap entries = new HashMap<>(); + entries.put("foo", "bar"); + entries.put("foo2", "bar2"); + jedis.hset("hash:test:encode", entries); + + List encodeObj = (List) SafeEncoder.encodeObject(jedis.sendCommand(HGETALL, "hash:test:encode")); + + assertEquals(4, encodeObj.size()); + entries.forEach((k, v) -> { + assertThat((Iterable) encodeObj, Matchers.hasItem(k)); + assertEquals(v, findValueFromMapAsList(encodeObj, k)); + }); + } + + @Test + public void encodeCompleteResponseHgetallResp3() { + Assume.assumeTrue(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); + + HashMap entries = new HashMap<>(); + entries.put("foo", "bar"); + entries.put("foo2", "bar2"); + jedis.hset("hash:test:encode", entries); + + List encodeObj = (List) SafeEncoder.encodeObject(jedis.sendCommand(HGETALL, "hash:test:encode")); + + assertEquals(2, encodeObj.size()); + encodeObj.forEach(kv -> { + assertThat(entries, Matchers.hasEntry(kv.getKey(), kv.getValue())); + }); + } + + @Test + public void encodeCompleteResponseXinfoStream() { + Assume.assumeFalse(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); + HashMap entry = new HashMap<>(); entry.put("foo", "bar"); StreamEntryID entryID = jedis.xadd("mystream", StreamEntryID.NEW_ENTRY, entry); jedis.xgroupCreate("mystream", "mygroup", null, false); Object obj = jedis.sendCommand(XINFO, "STREAM", "mystream"); + List encodeObj = (List) SafeEncoder.encodeObject(obj); - assertTrue(encodeObj.size() >= 14); - assertEquals(0, encodeObj.size() % 2); // must be even + assertThat(encodeObj.size(), Matchers.greaterThanOrEqualTo(14)); + assertEquals("must have even number of elements", 0, encodeObj.size() % 2); // must be even assertEquals(1L, findValueFromMapAsList(encodeObj, "length")); assertEquals(entryID.toString(), findValueFromMapAsList(encodeObj, "last-generated-id")); @@ -992,16 +1041,32 @@ public void encodeCompleteResponse() { assertEquals(entryAsList, ((List) findValueFromMapAsList(encodeObj, "first-entry")).get(1)); assertEquals(entryAsList, ((List) findValueFromMapAsList(encodeObj, "last-entry")).get(1)); + } - assertEquals("PONG", SafeEncoder.encodeObject(jedis.sendCommand(PING))); + @Test + public void encodeCompleteResponseXinfoStreamResp3() { + Assume.assumeTrue(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); + + HashMap entry = new HashMap<>(); + entry.put("foo", "bar"); + StreamEntryID entryID = jedis.xadd("mystream", StreamEntryID.NEW_ENTRY, entry); + jedis.xgroupCreate("mystream", "mygroup", null, false); - entry.put("foo2", "bar2"); - jedis.hset("hash:test:encode", entry); - encodeObj = (List) SafeEncoder.encodeObject(jedis.sendCommand(HGETALL, "hash:test:encode")); + Object obj = jedis.sendCommand(XINFO, "STREAM", "mystream"); - assertEquals(4, encodeObj.size()); - assertTrue(encodeObj.contains("foo")); - assertTrue(encodeObj.contains("foo2")); + List encodeObj = (List) SafeEncoder.encodeObject(obj); + + assertThat(encodeObj.size(), Matchers.greaterThanOrEqualTo(7)); + + assertEquals(1L, findValueFromMapAsKeyValueList(encodeObj, "length")); + assertEquals(entryID.toString(), findValueFromMapAsKeyValueList(encodeObj, "last-generated-id")); + + List entryAsList = new ArrayList<>(2); + entryAsList.add("foo"); + entryAsList.add("bar"); + + assertEquals(entryAsList, ((List) findValueFromMapAsKeyValueList(encodeObj, "first-entry")).get(1)); + assertEquals(entryAsList, ((List) findValueFromMapAsKeyValueList(encodeObj, "last-entry")).get(1)); } private Object findValueFromMapAsList(List list, Object key) { @@ -1013,6 +1078,15 @@ private Object findValueFromMapAsList(List list, Object key) { return null; } + private Object findValueFromMapAsKeyValueList(List list, Object key) { + for (KeyValue kv : list) { + if (key.equals(kv.getKey())) { + return kv.getValue(); + } + } + return null; + } + @Test public void copy() { assertFalse(jedis.copy("unknown", "foo", false)); diff --git a/src/test/java/redis/clients/jedis/commands/jedis/ControlCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/ControlCommandsTest.java index d97614cc32..df3fef6886 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/ControlCommandsTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/ControlCommandsTest.java @@ -207,21 +207,23 @@ public void onCommand(String command) { @Test public void configGet() { - List info = jedis.configGet("m*"); + Map info = jedis.configGet("m*"); assertNotNull(info); assertFalse(info.isEmpty()); - assertTrue(info.size() % 2 == 0); - List infoBinary = jedis.configGet("m*".getBytes()); +// assertTrue(info.size() % 2 == 0); + Map infoBinary = jedis.configGet("m*".getBytes()); assertNotNull(infoBinary); assertFalse(infoBinary.isEmpty()); - assertTrue(infoBinary.size() % 2 == 0); +// assertTrue(infoBinary.size() % 2 == 0); } @Test public void configSet() { - List info = jedis.configGet("maxmemory"); - assertEquals("maxmemory", info.get(0)); - String memory = info.get(1); + Map info = jedis.configGet("maxmemory"); +// assertEquals("maxmemory", info.get(0)); +// String memory = info.get(1); + String memory = info.get("maxmemory"); + assertNotNull(memory); assertEquals("OK", jedis.configSet("maxmemory", "200")); assertEquals("OK", jedis.configSet("maxmemory", memory)); } @@ -229,9 +231,11 @@ public void configSet() { @Test public void configSetBinary() { byte[] maxmemory = SafeEncoder.encode("maxmemory"); - List info = jedis.configGet(maxmemory); - assertArrayEquals(maxmemory, info.get(0)); - byte[] memory = info.get(1); + Map info = jedis.configGet(maxmemory); +// assertArrayEquals(maxmemory, info.get(0)); +// byte[] memory = info.get(1); + byte[] memory = info.get(maxmemory); + assertNotNull(memory); assertEquals("OK", jedis.configSet(maxmemory, Protocol.toByteArray(200))); assertEquals("OK", jedis.configSet(maxmemory, memory)); } @@ -239,15 +243,15 @@ public void configSetBinary() { @Test public void configGetSetMulti() { String[] params = new String[]{"hash-max-listpack-entries", "set-max-intset-entries", "zset-max-listpack-entries"}; - List info = jedis.configGet(params); - assertEquals(6, info.size()); - assertEquals("OK", jedis.configSet(info.toArray(new String[6]))); + Map info = jedis.configGet(params); + assertEquals(3, info.size()); + assertEquals("OK", jedis.configSet(info)); byte[][] bparams = new byte[][]{SafeEncoder.encode("hash-max-listpack-entries"), SafeEncoder.encode("set-max-intset-entries"), SafeEncoder.encode("zset-max-listpack-entries")}; - List binfo = jedis.configGet(bparams); - assertEquals(6, binfo.size()); - assertEquals("OK", jedis.configSet(binfo.toArray(new byte[6][]))); + Map binfo = jedis.configGet(bparams); + assertEquals(3, binfo.size()); + assertEquals("OK", jedis.configSetBinary(binfo)); } @Test diff --git a/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java b/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java index 1a7f3a876c..c69302d3c5 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java @@ -23,8 +23,7 @@ public JedisCommandsTestBase() { public void setUp() throws Exception { // jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().timeoutMillis(500).password("foobared").build()); jedis = new Jedis(hnp, DefaultJedisClientConfig.builder() - .protocol(RedisProtocolUtil.getRedisProtocol()) - .timeoutMillis(500).password("foobared").build()); + .protocol(RedisProtocolUtil.getRedisProtocol()).timeoutMillis(500).password("foobared").build()); jedis.flushAll(); } @@ -36,7 +35,6 @@ public void tearDown() throws Exception { protected Jedis createJedis() { // return new Jedis(hnp, DefaultJedisClientConfig.builder().password("foobared").build()); return new Jedis(hnp, DefaultJedisClientConfig.builder() - .protocol(RedisProtocolUtil.getRedisProtocol()) - .password("foobared").build()); + .protocol(RedisProtocolUtil.getRedisProtocol()).password("foobared").build()); } } diff --git a/src/test/java/redis/clients/jedis/commands/jedis/ModuleTest.java b/src/test/java/redis/clients/jedis/commands/jedis/ModuleTest.java index 8159f45d4a..1d4a9d981b 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/ModuleTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/ModuleTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.util.Collections; import java.util.List; import org.junit.Test; @@ -30,16 +31,20 @@ public byte[] getRaw() { @Test public void testModules() { - assertEquals("OK", jedis.moduleLoad("/tmp/testmodule.so")); + try { + assertEquals("OK", jedis.moduleLoad("/tmp/testmodule.so")); - List modules = jedis.moduleList(); + List modules = jedis.moduleList(); - assertEquals("testmodule", modules.get(0).getName()); + assertEquals("testmodule", modules.get(0).getName()); - Object output = jedis.sendCommand(ModuleCommand.SIMPLE); - assertTrue((Long) output > 0); + Object output = jedis.sendCommand(ModuleCommand.SIMPLE); + assertTrue((Long) output > 0); - assertEquals("OK", jedis.moduleUnload("testmodule")); - assertEquals(0, jedis.moduleList().size()); + } finally { + + assertEquals("OK", jedis.moduleUnload("testmodule")); + assertEquals(Collections.emptyList(), jedis.moduleList()); + } } } diff --git a/src/test/java/redis/clients/jedis/commands/jedis/ScriptingCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/ScriptingCommandsTest.java index f21464ff77..1e258dbad3 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/ScriptingCommandsTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/ScriptingCommandsTest.java @@ -14,6 +14,7 @@ import org.junit.Test; import redis.clients.jedis.Jedis; +import redis.clients.jedis.RedisProtocol; import redis.clients.jedis.args.FlushMode; import redis.clients.jedis.args.FunctionRestorePolicy; import redis.clients.jedis.exceptions.JedisConnectionException; @@ -22,6 +23,8 @@ import redis.clients.jedis.resps.FunctionStats; import redis.clients.jedis.resps.LibraryInfo; import redis.clients.jedis.util.ClientKillerUtil; +import redis.clients.jedis.util.KeyValue; +import redis.clients.jedis.util.RedisProtocolUtil; import redis.clients.jedis.util.SafeEncoder; public class ScriptingCommandsTest extends JedisCommandsTestBase { @@ -387,19 +390,37 @@ public void functionList() { assertEquals(functionCode, response.getLibraryCode()); // Binary - List bresponse = (List) jedis.functionListBinary().get(0); - assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); + if (RedisProtocolUtil.getRedisProtocol() != RedisProtocol.RESP3) { - bresponse = (List) jedis.functionListWithCodeBinary().get(0); - assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); - assertNotNull(bresponse.get(7)); + List bresponse = (List) jedis.functionListBinary().get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); - bresponse = (List) jedis.functionList(library.getBytes()).get(0); - assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); + bresponse = (List) jedis.functionListWithCodeBinary().get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); + assertNotNull(bresponse.get(7)); - bresponse = (List) jedis.functionListWithCode(library.getBytes()).get(0); - assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); - assertNotNull(bresponse.get(7)); + bresponse = (List) jedis.functionList(library.getBytes()).get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); + + bresponse = (List) jedis.functionListWithCode(library.getBytes()).get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(1)); + assertNotNull(bresponse.get(7)); + } else { + + List bresponse = (List) jedis.functionListBinary().get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(0).getValue()); + + bresponse = (List) jedis.functionListWithCodeBinary().get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(0).getValue()); + assertNotNull(bresponse.get(3)); + + bresponse = (List) jedis.functionList(library.getBytes()).get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(0).getValue()); + + bresponse = (List) jedis.functionListWithCode(library.getBytes()).get(0); + assertArrayEquals(library.getBytes(), (byte[]) bresponse.get(0).getValue()); + assertNotNull(bresponse.get(3)); + } } @Test diff --git a/src/test/java/redis/clients/jedis/commands/jedis/SlowlogCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/SlowlogCommandsTest.java index d9b3ba70ba..66806b933a 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/SlowlogCommandsTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/SlowlogCommandsTest.java @@ -1,12 +1,13 @@ package redis.clients.jedis.commands.jedis; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.List; +import org.hamcrest.Matchers; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -28,7 +29,7 @@ public class SlowlogCommandsTest extends JedisCommandsTestBase { @Override public void setUp() throws Exception { super.setUp(); - slowlogTimeValue = jedis.configGet(SLOWLOG_TIME_PARAM).get(1); + slowlogTimeValue = jedis.configGet(SLOWLOG_TIME_PARAM).get(SLOWLOG_TIME_PARAM); } @After @@ -41,6 +42,8 @@ public void tearDown() throws Exception { @Test public void slowlog() { jedis.configSet(SLOWLOG_TIME_PARAM, ZERO_STRING); + jedis.slowlogReset(); + jedis.set("foo", "bar"); jedis.set("foo2", "bar2"); @@ -48,9 +51,9 @@ public void slowlog() { assertEquals(1, reducedLog.size()); Slowlog log = reducedLog.get(0); - assertTrue(log.getId() > 0); - assertTrue(log.getTimeStamp() > 0); - assertTrue(log.getExecutionTime() >= 0); + assertThat(log.getId(), Matchers.greaterThan(0L)); + assertThat(log.getTimeStamp(), Matchers.greaterThan(0L)); + assertThat(log.getExecutionTime(), Matchers.greaterThanOrEqualTo(0L)); assertNotNull(log.getArgs()); List breducedLog = jedis.slowlogGetBinary(1); @@ -63,8 +66,9 @@ public void slowlog() { assertNotNull(blog1); // assertEquals(7, jedis.slowlogLen()); - assertTrue(jedis.slowlogLen() > 5 && jedis.slowlogLen() < 12); - assertTrue(jedis.slowlogGet().toString().contains("SLOWLOG")); + assertThat(jedis.slowlogLen(), + Matchers.allOf(Matchers.greaterThanOrEqualTo(6L), Matchers.lessThanOrEqualTo(13L))); + assertThat(jedis.slowlogGet().toString(), Matchers.containsString("SLOWLOG")); } @Test @@ -74,36 +78,37 @@ public void slowlogObjectDetails() { jedis.slowlogReset(); jedis.configSet(SLOWLOG_TIME_PARAM, ZERO_STRING); - List logs = jedis.slowlogGet(); // Get only 'CONFIG SET' - assertEquals(1, logs.size()); + List logs = jedis.slowlogGet(); // Get only 'CONFIG SET', or including 'SLOWLOG RESET' + //assertEquals(1, logs.size()); + assertThat(logs.size(), Matchers.allOf(Matchers.greaterThanOrEqualTo(1), Matchers.lessThanOrEqualTo(2))); Slowlog log = logs.get(0); - assertTrue(log.getId() > 0); - assertTrue(log.getTimeStamp() > 0); - assertTrue(log.getExecutionTime() > 0); + assertThat(log.getId(), Matchers.greaterThan(0L)); + assertThat(log.getTimeStamp(), Matchers.greaterThan(0L)); + assertThat(log.getExecutionTime(), Matchers.greaterThan(0L)); assertEquals(4, log.getArgs().size()); assertEquals(SafeEncoder.encode(Protocol.Command.CONFIG.getRaw()), log.getArgs().get(0)); assertEquals(SafeEncoder.encode(Protocol.Keyword.SET.getRaw()), log.getArgs().get(1)); assertEquals(SLOWLOG_TIME_PARAM, log.getArgs().get(2)); assertEquals(ZERO_STRING, log.getArgs().get(3)); -// assertEquals("127.0.0.1", log.getClientIpPort().getHost()); - assertTrue(LOCAL_IPS.contains(log.getClientIpPort().getHost())); - assertTrue(log.getClientIpPort().getPort() > 0); + assertThat(log.getClientIpPort().getHost(), Matchers.in(LOCAL_IPS)); + assertThat(log.getClientIpPort().getPort(), Matchers.greaterThan(0)); assertEquals(clientName, log.getClientName()); } @Test - public void slowlogBinaryDetails() { + public void slowlogBinaryObjectDetails() { final byte[] clientName = SafeEncoder.encode("slowlog-binary-client"); jedis.clientSetname(clientName); jedis.slowlogReset(); jedis.configSet(SafeEncoder.encode(SLOWLOG_TIME_PARAM), SafeEncoder.encode(ZERO_STRING)); - List logs = jedis.slowlogGetBinary(); // Get only 'CONFIG SET' - assertEquals(1, logs.size()); + List logs = jedis.slowlogGetBinary(); // Get only 'CONFIG SET', or including 'SLOWLOG RESET' + //assertEquals(1, logs.size()); + assertThat(logs.size(), Matchers.allOf(Matchers.greaterThanOrEqualTo(1), Matchers.lessThanOrEqualTo(2))); List log = (List) logs.get(0); - assertTrue((Long) log.get(0) > 0); - assertTrue((Long) log.get(1) > 0); - assertTrue((Long) log.get(2) > 0); + assertThat((Long) log.get(0), Matchers.greaterThan(0L)); + assertThat((Long) log.get(1), Matchers.greaterThan(0L)); + assertThat((Long) log.get(2), Matchers.greaterThan(0L)); List args = (List) log.get(3); assertEquals(4, args.size()); assertArrayEquals(Protocol.Command.CONFIG.getRaw(), (byte[]) args.get(0)); @@ -111,7 +116,7 @@ public void slowlogBinaryDetails() { assertArrayEquals(SafeEncoder.encode(SLOWLOG_TIME_PARAM), (byte[]) args.get(2)); assertArrayEquals(Protocol.toByteArray(0), (byte[]) args.get(3)); // assertTrue(SafeEncoder.encode((byte[]) log.get(4)).startsWith("127.0.0.1:")); - assertTrue(((byte[]) log.get(4)).length > 0); + assertThat(((byte[]) log.get(4)).length, Matchers.greaterThanOrEqualTo(10)); // 'IP:PORT' assertArrayEquals(clientName, (byte[]) log.get(5)); } } diff --git a/src/test/java/redis/clients/jedis/commands/jedis/StreamsCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/StreamsCommandsTest.java index 6713fb3428..6b28f9ed19 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/StreamsCommandsTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/StreamsCommandsTest.java @@ -500,7 +500,7 @@ public void xreadGroupWithParamsWhenPendingMessageIsDiscarded() { @Test public void xack() { - Map map = new HashMap(); + Map map = new HashMap<>(); map.put("f1", "v1"); jedis.xadd("xack-stream", (StreamEntryID) null, map); diff --git a/src/test/java/redis/clients/jedis/commands/unified/AllKindOfValuesCommandsTestBase.java b/src/test/java/redis/clients/jedis/commands/unified/AllKindOfValuesCommandsTestBase.java index 176c7b78d7..25ecad1eea 100644 --- a/src/test/java/redis/clients/jedis/commands/unified/AllKindOfValuesCommandsTestBase.java +++ b/src/test/java/redis/clients/jedis/commands/unified/AllKindOfValuesCommandsTestBase.java @@ -1,5 +1,6 @@ package redis.clients.jedis.commands.unified; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -28,8 +29,11 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import org.hamcrest.Matchers; +import org.junit.Assume; import org.junit.Test; +import redis.clients.jedis.RedisProtocol; import redis.clients.jedis.ScanIteration; import redis.clients.jedis.StreamEntryID; import redis.clients.jedis.args.ExpiryOption; @@ -39,6 +43,9 @@ import redis.clients.jedis.exceptions.JedisDataException; import redis.clients.jedis.params.SetParams; import redis.clients.jedis.util.AssertUtil; +import redis.clients.jedis.util.KeyValue; +import redis.clients.jedis.util.RedisProtocolUtil; +import redis.clients.jedis.util.SafeEncoder; public abstract class AllKindOfValuesCommandsTestBase extends UnifiedJedisCommandsTestBase { @@ -788,17 +795,60 @@ public void sendBlockingCommandTest() { } @Test - public void encodeCompleteResponse() { + public void encodeCompleteResponsePing() { + assertEquals("PONG", SafeEncoder.encodeObject(jedis.sendCommand(PING))); + } + + @Test + public void encodeCompleteResponseHgetall() { + Assume.assumeFalse(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); + + HashMap entries = new HashMap<>(); + entries.put("foo", "bar"); + entries.put("foo2", "bar2"); + jedis.hset("hash:test:encode", entries); + + List encodeObj = (List) SafeEncoder.encodeObject(jedis.sendCommand(HGETALL, "hash:test:encode")); + + assertEquals(4, encodeObj.size()); + entries.forEach((k, v) -> { + assertThat((Iterable) encodeObj, Matchers.hasItem(k)); + assertEquals(v, findValueFromMapAsList(encodeObj, k)); + }); + } + + @Test + public void encodeCompleteResponseHgetallResp3() { + Assume.assumeTrue(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); + + HashMap entries = new HashMap<>(); + entries.put("foo", "bar"); + entries.put("foo2", "bar2"); + jedis.hset("hash:test:encode", entries); + + List encodeObj = (List) SafeEncoder.encodeObject(jedis.sendCommand(HGETALL, "hash:test:encode")); + + assertEquals(2, encodeObj.size()); + encodeObj.forEach(kv -> { + assertThat(entries, Matchers.hasEntry(kv.getKey(), kv.getValue())); + }); + } + + @Test + public void encodeCompleteResponseXinfoStream() { + Assume.assumeFalse(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); + HashMap entry = new HashMap<>(); entry.put("foo", "bar"); StreamEntryID entryID = jedis.xadd("mystream", StreamEntryID.NEW_ENTRY, entry); jedis.xgroupCreate("mystream", "mygroup", null, false); Object obj = jedis.sendCommand(XINFO, "STREAM", "mystream"); - List encodeObj = (List) encodeObject(obj); - assertTrue(encodeObj.size() >= 14); - assertEquals(0, encodeObj.size() % 2); // must be even + List encodeObj = (List) SafeEncoder.encodeObject(obj); + + assertThat(encodeObj.size(), Matchers.greaterThanOrEqualTo(14)); + assertEquals("must have even number of elements", 0, encodeObj.size() % 2); // must be even assertEquals(1L, findValueFromMapAsList(encodeObj, "length")); assertEquals(entryID.toString(), findValueFromMapAsList(encodeObj, "last-generated-id")); @@ -809,16 +859,32 @@ public void encodeCompleteResponse() { assertEquals(entryAsList, ((List) findValueFromMapAsList(encodeObj, "first-entry")).get(1)); assertEquals(entryAsList, ((List) findValueFromMapAsList(encodeObj, "last-entry")).get(1)); + } - assertEquals("PONG", encodeObject(jedis.sendCommand(PING))); + @Test + public void encodeCompleteResponseXinfoStreamResp3() { + Assume.assumeTrue(RedisProtocolUtil.getRedisProtocol() == RedisProtocol.RESP3); - entry.put("foo2", "bar2"); - jedis.hset("hash:test:encode", entry); - encodeObj = (List) encodeObject(jedis.sendCommand(HGETALL, "hash:test:encode")); + HashMap entry = new HashMap<>(); + entry.put("foo", "bar"); + StreamEntryID entryID = jedis.xadd("mystream", StreamEntryID.NEW_ENTRY, entry); + jedis.xgroupCreate("mystream", "mygroup", null, false); - assertEquals(4, encodeObj.size()); - assertTrue(encodeObj.contains("foo")); - assertTrue(encodeObj.contains("foo2")); + Object obj = jedis.sendCommand(XINFO, "STREAM", "mystream"); + + List encodeObj = (List) SafeEncoder.encodeObject(obj); + + assertThat(encodeObj.size(), Matchers.greaterThanOrEqualTo(7)); + + assertEquals(1L, findValueFromMapAsKeyValueList(encodeObj, "length")); + assertEquals(entryID.toString(), findValueFromMapAsKeyValueList(encodeObj, "last-generated-id")); + + List entryAsList = new ArrayList<>(2); + entryAsList.add("foo"); + entryAsList.add("bar"); + + assertEquals(entryAsList, ((List) findValueFromMapAsKeyValueList(encodeObj, "first-entry")).get(1)); + assertEquals(entryAsList, ((List) findValueFromMapAsKeyValueList(encodeObj, "last-entry")).get(1)); } private Object findValueFromMapAsList(List list, Object key) { @@ -830,6 +896,15 @@ private Object findValueFromMapAsList(List list, Object key) { return null; } + private Object findValueFromMapAsKeyValueList(List list, Object key) { + for (KeyValue kv : list) { + if (key.equals(kv.getKey())) { + return kv.getValue(); + } + } + return null; + } + @Test public void copy() { assertFalse(jedis.copy("unknown", "foo", false));