Skip to content

Commit

Permalink
Test with 8.0-M04-pre (#4069)
Browse files Browse the repository at this point in the history
* Test with 8.0-M03-pre

* Test with 8.0-M03-pre

* Test with 8.0-M03

* Test with 8.0-M04-pre

* SearchDefaultDialectTest.testDialectsWithFTExplain

Output of ft.explain  has changed in Redis 8.0.
Existing Jedis client ftExplain() API returns the raw output  and does not perform any parsing/mapping, hence updating the test to verify only for received output when used with correct dialect & gets syntax error otherwise.

* Update testAggregationBuilderAddScores test to match change in SEARCH module replacing default SCORER from TF-IDF to BM25.

* Apply suggestions from code review

Co-authored-by: M Sazzadul Hoque <[email protected]>

* Address review comments

* Update SearchDefaultDialectTest.java

* Output of ft.explain  has changed in Redis 8.0.
Existing Jedis client ftExplain() API returns the raw output  and does not perform any parsing/mapping, hence updating the test to verify only for received output when used with correct dialect & gets syntax error otherwise.

* minor changes: format, private, unused variable, etc.

---------

Co-authored-by: M Sazzadul Hoque <[email protected]>
  • Loading branch information
ggivo and sazzad16 authored Jan 30, 2025
1 parent 3e537f9 commit 4b10f2e
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 111 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test-on-docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
fail-fast: false
matrix:
redis_version:
- "8.0-M02"
- "8.0-M04-pre"
- "7.4.1"
- "7.2.6"
# - "6.2.16"
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
PATH := ./redis-git/src:${PATH}

# Supported test env versions
SUPPORTED_TEST_ENV_VERSIONS := 8.0-M02 7.4.1 7.2.6 6.2.16
DEFAULT_TEST_ENV_VERSION := 8.0-M02
SUPPORTED_TEST_ENV_VERSIONS := 8.0-M04-pre, 8.0-M02 7.4.1 7.2.6 6.2.16
DEFAULT_TEST_ENV_VERSION := 8.0-M04-pre
REDIS_ENV_WORK_DIR := $(or ${REDIS_ENV_WORK_DIR},/tmp/redis-env-work)

define REDIS1_CONF
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import redis.clients.jedis.search.aggr.FtAggregateIteration;
import redis.clients.jedis.search.schemafields.NumericField;
import redis.clients.jedis.search.schemafields.TextField;
import redis.clients.jedis.util.RedisConditions;

@RunWith(Parameterized.class)
public class AggregationTest extends RedisModuleCommandsTestBase {
Expand Down Expand Up @@ -202,7 +203,7 @@ public void testAggregationBuilderVerbatim() {
}

@Test
@SinceRedisVersion(value="7.4.0", message="ADDSCORES")
@SinceRedisVersion(value = "7.4.0", message = "ADDSCORES")
public void testAggregationBuilderAddScores() {
Schema sc = new Schema();
sc.addSortableTextField("name", 1.0);
Expand All @@ -215,8 +216,15 @@ public void testAggregationBuilderAddScores() {
.apply("@__score * 100", "normalized_score").dialect(3);

AggregationResult res = client.ftAggregate(index, r);
assertEquals(2, res.getRow(0).getLong("__score"));
assertEquals(200, res.getRow(0).getLong("normalized_score"));
if (RedisConditions.of(client).moduleVersionIsGreatherThan("SEARCH", 79900)) {
// Default scorer is BM25
assertEquals(0.6931, res.getRow(0).getDouble("__score"), 0.0001);
assertEquals(69.31, res.getRow(0).getDouble("normalized_score"), 0.01);
} else {
// Default scorer is TF-IDF
assertEquals(2, res.getRow(0).getLong("__score"));
assertEquals(200, res.getRow(0).getLong("normalized_score"));
}
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
package redis.clients.jedis.modules.search;

import static org.junit.Assert.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyOrNullString;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
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 org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import redis.clients.jedis.RedisProtocol;
import redis.clients.jedis.UnifiedJedis;
import redis.clients.jedis.exceptions.JedisDataException;
import redis.clients.jedis.search.*;
import redis.clients.jedis.search.schemafields.NumericField;
Expand Down Expand Up @@ -107,62 +111,34 @@ public void testDialectsWithFTExplain() throws Exception {

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"));
assertSyntaxError(query, client); // dialect=1 throws syntax error
query = new Query(q); // dialect=default=2 should return execution plan
assertThat(client.ftExplain(INDEX, query), containsString("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"));
assertSyntaxError(query, client); // dialect=1 throws syntax error
query = new Query(q).addParam("hello", "hello"); // dialect=default=2 should return execution plan
assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString()));


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(1); // dialect=1 should return execution plan
assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString()));
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"));
}
assertSyntaxError(query, client); // dialect=2 throws 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(1); // dialect=1 should return execution plan
assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString()));
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"));
}
assertSyntaxError(query, client); // dialect=2 throws 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(1); // dialect=1 should return execution plan
assertThat(client.ftExplain(INDEX, query), not(emptyOrNullString()));
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"));
}
assertSyntaxError(query, client); // dialect=2 throws syntax error
}

@Test
Expand Down Expand Up @@ -194,9 +170,15 @@ public void testAggregationBuilderParamsDialect() {
@Test
public void dialectBoundSpellCheck() {
client.ftCreate(INDEX, TextField.of("t"));
JedisDataException error = Assert.assertThrows(JedisDataException.class,
JedisDataException error = 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"));
assertThat(error.getMessage(), containsString("DIALECT requires a non negative integer"));
}

private void assertSyntaxError(Query query, UnifiedJedis client) {
JedisDataException error = assertThrows(JedisDataException.class,
() -> client.ftExplain(INDEX, query));
assertThat(error.getMessage(), containsString("Syntax error"));
}
}
81 changes: 26 additions & 55 deletions src/test/java/redis/clients/jedis/modules/search/SearchTest.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package redis.clients.jedis.modules.search;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyOrNullString;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.*;

import java.util.*;
import java.util.stream.Collectors;

import io.redis.test.annotations.SinceRedisVersion;
import io.redis.test.utils.RedisVersion;
import org.hamcrest.Matchers;
import org.junit.Assume;
Expand All @@ -15,6 +18,7 @@
import org.junit.runners.Parameterized;

import redis.clients.jedis.RedisProtocol;
import redis.clients.jedis.UnifiedJedis;
import redis.clients.jedis.exceptions.JedisDataException;
import redis.clients.jedis.json.Path;
import redis.clients.jedis.search.*;
Expand Down Expand Up @@ -1078,84 +1082,45 @@ public void testDialectsWithFTExplain() throws Exception {

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"));
}
assertSyntaxError(query, client); // dialect=1 throws syntax error
query = new Query(q).dialect(2);
assertTrue("Should contain 'WILDCARD'", client.ftExplain(index, query).contains("WILDCARD"));
assertThat(client.ftExplain(index, query), containsString("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"));
}
assertSyntaxError(query, client); // dialect=1 throws syntax error
query = new Query(q).dialect(2).addParam("hello", "hello");
assertTrue("Should contain 'UNION {\n hello\n +hello(expanded)\n}\n'",
client.ftExplain(index, query).contains("UNION {\n hello\n +hello(expanded)\n}\n"));
assertThat(client.ftExplain(index, query), not(emptyOrNullString()));

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}"));
assertThat(client.ftExplain(index, query), not(emptyOrNullString()));
query = new Query(q).dialect(2);
try {
client.ftExplain(index, query);
fail();
} catch (JedisDataException e) {
assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error"));
}
assertSyntaxError(query, client); // dialect=2 throws 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"));
assertThat(client.ftExplain(index, query), not(emptyOrNullString()));
query = new Query(q).dialect(2);
try {
client.ftExplain(index, query);
fail();
} catch (JedisDataException e) {
assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error"));
}
assertSyntaxError(query, client); // dialect=2 throws 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"));
assertThat(client.ftExplain(index, query), not(emptyOrNullString()));
query = new Query(q).dialect(2);
try {
client.ftExplain(index, query);
fail();
} catch (JedisDataException e) {
assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error"));
}
assertSyntaxError(query, client); // dialect=2 throws syntax error

q = "*=>[KNN 10 @v $BLOB]";
query = new Query(q).addParam("BLOB", "aaaa").dialect(1);
try {
client.ftExplain(index, query);
fail();
} catch (JedisDataException e) {
assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error"));
}
assertSyntaxError(query, client); // dialect=1 throws syntax error
query = new Query(q).addParam("BLOB", "aaaa").dialect(2);
assertTrue("Should contain '{K=10 nearest vector'", client.ftExplain(index, query).contains("{K=10 nearest vector"));
assertThat(client.ftExplain(index, query), not(emptyOrNullString()));

q = "*=>[knn $K @vec_field $BLOB as score]";
q = "*=>[knn $K @v $BLOB as score]";
query = new Query(q).addParam("BLOB", "aaaa").addParam("K", "10").dialect(1);
try {
client.ftExplain(index, query);
fail();
} catch (JedisDataException e) {
assertTrue("Should contain 'Syntax error'", e.getMessage().contains("Syntax error"));
}
assertSyntaxError(query, client); // dialect=1 throws syntax error
query = new Query(q).addParam("BLOB", "aaaa").addParam("K", "10").dialect(2);
assertTrue("Should contain '{K=10 nearest vector'", client.ftExplain(index, query).contains("{K=10 nearest vector"));
assertThat(client.ftExplain(index, query), not(emptyOrNullString()));
}

@org.junit.Ignore
Expand Down Expand Up @@ -1298,4 +1263,10 @@ public void searchIteration() throws Exception {
}
assertEquals(7, total);
}

void assertSyntaxError(Query query, UnifiedJedis client) {
JedisDataException error = assertThrows(JedisDataException.class,
() -> client.ftExplain(index, query));
assertThat(error.getMessage(), containsString("Syntax error"));
}
}
77 changes: 77 additions & 0 deletions src/test/java/redis/clients/jedis/util/RedisConditions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package redis.clients.jedis.util;

import io.redis.test.utils.RedisVersion;
import redis.clients.jedis.CommandArguments;
import redis.clients.jedis.CommandObject;
import redis.clients.jedis.Module;
import redis.clients.jedis.UnifiedJedis;
import redis.clients.jedis.resps.CommandInfo;

import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import static redis.clients.jedis.BuilderFactory.MODULE_LIST;
import static redis.clients.jedis.Protocol.Command.COMMAND;
import static redis.clients.jedis.Protocol.Command.MODULE;
import static redis.clients.jedis.Protocol.Keyword.LIST;

public class RedisConditions {

private final RedisVersion version;
private final Map<String, Integer> modules;
private final Map<String, CommandInfo> commands;

private RedisConditions(RedisVersion version, Map< String, CommandInfo> commands, Map<String, Integer> modules) {
this.version = version;
this.commands = commands;
this.modules = modules;
}

public static RedisConditions of(UnifiedJedis jedis) {
RedisVersion version = RedisVersionUtil.getRedisVersion(jedis);

CommandObject<Map<String, CommandInfo>> commandInfoCmd
= new CommandObject<>(new CommandArguments(COMMAND), CommandInfo.COMMAND_INFO_RESPONSE);
Map<String, CommandInfo> commands = jedis.executeCommand(commandInfoCmd);

CommandObject<List<Module>> moduleListCmd
= new CommandObject<>(new CommandArguments(MODULE).add(LIST), MODULE_LIST);

Map<String, Integer> modules = jedis.executeCommand(moduleListCmd)
.stream()
.collect(Collectors.toMap((m) -> m.getName().toUpperCase(), Module::getVersion));

return new RedisConditions(version, commands, modules);
}

public RedisVersion getVersion() {
return version;
}

/**
* @param command
* @return {@code true} if the command is present.
*/
public boolean hasCommand(String command) {
return commands.containsKey(command.toUpperCase());
}

/**
* @param module
* @return {@code true} if the module is present.
*/
public boolean hasModule(String module) {
return modules.containsKey(module.toUpperCase());
}

/**
* @param module
* @param version
* @return {@code true} if the module is present.
*/
public boolean moduleVersionIsGreatherThan(String module, int version) {
Integer moduleVersion = modules.get(module.toUpperCase());
return moduleVersion != null && moduleVersion > version;
}
}

0 comments on commit 4b10f2e

Please sign in to comment.