From d8ec35f48b195f4a6767a6654bc1ba4cefc47b1e Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 11 Jan 2018 14:09:05 +0100 Subject: [PATCH] DATAREDIS-744 - Polishing Move binary lookup methods to ByteUtils, update license headers and restore altered tests by creating additional ones. Original Pull Request: #298 --- .../data/redis/core/RedisKeyExpiredEvent.java | 2 +- .../core/convert/MappingRedisConverter.java | 44 ++---------- .../data/redis/util/ByteUtils.java | 61 +++++++++++++++++ .../core/RedisKeyExpiredEventUnitTests.java | 2 +- .../redis/core/RedisKeyValueAdapterTests.java | 67 ++++++++++++++++++- .../BinaryKeyspaceIdentifierUnitTests.java | 2 +- .../convert/KeyspaceIdentifierUnitTests.java | 2 +- 7 files changed, 136 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyExpiredEvent.java b/src/main/java/org/springframework/data/redis/core/RedisKeyExpiredEvent.java index 299f47727..de664eaf5 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyExpiredEvent.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyExpiredEvent.java @@ -35,7 +35,7 @@ public class RedisKeyExpiredEvent extends RedisKeyspaceEvent { /** * Use {@literal UTF-8} as default charset. */ - public static final Charset CHARSET = StandardCharsets.UTF_8; + static final Charset CHARSET = StandardCharsets.UTF_8; private final BinaryKeyspaceIdentifier objectId; private final @Nullable Object value; diff --git a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java index 0b65db051..9f474f14a 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java +++ b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java @@ -1264,11 +1264,11 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { Assert.isTrue(isValid(key), String.format("Invalid key %s", new String(key))); - boolean phantomKey = startsWith(key, PHANTOM_SUFFIX, key.length - PHANTOM_SUFFIX.length); + boolean phantomKey = ByteUtils.startsWith(key, PHANTOM_SUFFIX, key.length - PHANTOM_SUFFIX.length); - int keyspaceEndIndex = find(key, DELIMITTER); - byte[] keyspace = getKeyspace(key, keyspaceEndIndex); - byte[] id = getId(key, phantomKey, keyspaceEndIndex); + int keyspaceEndIndex = ByteUtils.indexOf(key, DELIMITTER); + byte[] keyspace = extractKeyspace(key, keyspaceEndIndex); + byte[] id = extractId(key, phantomKey, keyspaceEndIndex); return new BinaryKeyspaceIdentifier(keyspace, id, phantomKey); } @@ -1286,12 +1286,12 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { return false; } - int keyspaceEndIndex = find(key, DELIMITTER); + int keyspaceEndIndex = ByteUtils.indexOf(key, DELIMITTER); return keyspaceEndIndex > 0 && key.length > keyspaceEndIndex; } - private static byte[] getId(byte[] key, boolean phantomKey, int keyspaceEndIndex) { + private static byte[] extractId(byte[] key, boolean phantomKey, int keyspaceEndIndex) { int idSize; @@ -1308,42 +1308,12 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { return id; } - private static byte[] getKeyspace(byte[] key, int keyspaceEndIndex) { + private static byte[] extractKeyspace(byte[] key, int keyspaceEndIndex) { byte[] keyspace = new byte[keyspaceEndIndex]; System.arraycopy(key, 0, keyspace, 0, keyspaceEndIndex); return keyspace; } - - private static boolean startsWith(byte[] haystack, byte[] prefix, int offset) { - - int to = offset; - int prefixOffset = 0; - int prefixLength = prefix.length; - - if ((offset < 0) || (offset > haystack.length - prefixLength)) { - return false; - } - - while (--prefixLength >= 0) { - if (haystack[to++] != prefix[prefixOffset++]) { - return false; - } - } - - return true; - } - - private static int find(byte[] haystack, byte needle) { - - for (int i = 0; i < haystack.length; i++) { - if (haystack[i] == needle) { - return i; - } - } - - return -1; - } } } diff --git a/src/main/java/org/springframework/data/redis/util/ByteUtils.java b/src/main/java/org/springframework/data/redis/util/ByteUtils.java index 26a5c3191..7b1783b54 100644 --- a/src/main/java/org/springframework/data/redis/util/ByteUtils.java +++ b/src/main/java/org/springframework/data/redis/util/ByteUtils.java @@ -142,4 +142,65 @@ public final class ByteUtils { duplicate.get(bytes); return bytes; } + + /** + * Tests if the {@code haystack} starts with the given {@code prefix}. + * + * @param haystack the source to scan. + * @param prefix the prefix to find. + * @return {@literal true} if {@code haystack} at position {@code offset} starts with {@code prefix}. + * @since 1.8.10 + * @see #startsWith(byte[], byte[], int) + */ + public static boolean startsWith(byte[] haystack, byte[] prefix) { + return startsWith(haystack, prefix, 0); + } + + /** + * Tests if the {@code haystack} beginning at the specified {@code offset} starts with the given {@code prefix}. + * + * @param haystack the source to scan. + * @param prefix the prefix to find. + * @param offset the offset to start at. + * @return {@literal true} if {@code haystack} at position {@code offset} starts with {@code prefix}. + * @since 1.8.10 + */ + public static boolean startsWith(byte[] haystack, byte[] prefix, int offset) { + + int to = offset; + int prefixOffset = 0; + int prefixLength = prefix.length; + + if ((offset < 0) || (offset > haystack.length - prefixLength)) { + return false; + } + + while (--prefixLength >= 0) { + if (haystack[to++] != prefix[prefixOffset++]) { + return false; + } + } + + return true; + } + + /** + * Searches the specified array of bytes for the specified value. Returns the index of the first matching value in the + * {@code haystack}s natural order or {@code -1} of {@code needle} could not be found. + * + * @param haystack the source to scan. + * @param needle the value to scan for. + * @return index of first appearance, or -1 if not found. + * @since 1.8.10 + */ + public static int indexOf(byte[] haystack, byte needle) { + + for (int i = 0; i < haystack.length; i++) { + if (haystack[i] == needle) { + return i; + } + } + + return -1; + } } diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyExpiredEventUnitTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyExpiredEventUnitTests.java index 3f4cc27bd..e68ceb1d7 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyExpiredEventUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyExpiredEventUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 the original author or authors. + * Copyright 2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java index e24699bf1..4a07446b0 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java @@ -210,12 +210,26 @@ public class RedisKeyValueAdapterTests { assertThat(template.opsForSet().members("persons:address.country:Andor"), hasItems("1")); } - @Test // DATAREDIS-425, DATAREDIS-744 + @Test // DATAREDIS-425 public void getShouldReadSimpleObjectCorrectly() { Map map = new LinkedHashMap<>(); map.put("_class", Person.class.getName()); map.put("age", "24"); + template.opsForHash().putAll("persons:load-1", map); + + Object loaded = adapter.get("load-1", "persons"); + + assertThat(loaded, instanceOf(Person.class)); + assertThat(((Person) loaded).age, is(24)); + } + + @Test // DATAREDIS-744 + public void getShouldReadSimpleObjectWithColonInIdCorrectly() { + + Map map = new LinkedHashMap(); + map.put("_class", Person.class.getName()); + map.put("age", "24"); template.opsForHash().putAll("persons:load-1:a", map); Object loaded = adapter.get("load-1:a", "persons"); @@ -283,7 +297,7 @@ public class RedisKeyValueAdapterTests { assertThat(template.opsForSet().members("persons:firstname:rand"), not(hasItem("1"))); } - @Test // DATAREDIS-425, DATAREDIS-744 + @Test // DATAREDIS-425 public void keyExpiredEventShouldRemoveHelperStructures() throws Exception { assumeTrue(RedisTestProfileValueSource.matches("runLongTests", "true")); @@ -293,6 +307,34 @@ public class RedisKeyValueAdapterTests { map.put("firstname", "rand"); map.put("address.country", "Andor"); + template.opsForHash().putAll("persons:1", map); + + template.opsForSet().add("persons", "1"); + template.opsForSet().add("persons:firstname:rand", "1"); + template.opsForSet().add("persons:1:idx", "persons:firstname:rand"); + + template.expire("persons:1", 100, TimeUnit.MILLISECONDS); + + waitUntilKeyIsGone(template, "persons:1"); + waitUntilKeyIsGone(template, "persons:1:phantom"); + waitUntilKeyIsGone(template, "persons:firstname:rand"); + + assertThat(template.hasKey("persons:1"), is(false)); + assertThat(template.hasKey("persons:firstname:rand"), is(false)); + assertThat(template.hasKey("persons:1:idx"), is(false)); + assertThat(template.opsForSet().members("persons"), not(hasItem("1"))); + } + + @Test // DATAREDIS-744 + public void keyExpiredEventShouldRemoveHelperStructuresForObjectsWithColonInId() throws Exception { + + assumeTrue(RedisTestProfileValueSource.matches("runLongTests", "true")); + + Map map = new LinkedHashMap(); + map.put("_class", Person.class.getName()); + map.put("firstname", "rand"); + map.put("address.country", "Andor"); + template.opsForHash().putAll("persons:1:b", map); template.opsForSet().add("persons", "1"); @@ -374,12 +416,31 @@ public class RedisKeyValueAdapterTests { assertThat(template.opsForSet().isMember("persons:mat:idx", "persons:firstname:mat"), is(true)); } - @Test // DATAREDIS-471, DATAREDIS-744 + @Test // DATAREDIS-471 public void updateShouldAlterIndexDataCorrectly() { Person rand = new Person(); rand.firstname = "rand"; + adapter.put("1", rand, "persons"); + + assertThat(template.hasKey("persons:firstname:rand"), is(true)); + + PartialUpdate update = new PartialUpdate("1", Person.class) // + .set("firstname", "mat"); + + adapter.update(update); + + assertThat(template.hasKey("persons:firstname:rand"), is(false)); + assertThat(template.hasKey("persons:firstname:mat"), is(true)); + } + + @Test // DATAREDIS-744 + public void updateShouldAlterIndexDataForObjectsWithColonInIdCorrectly() { + + Person rand = new Person(); + rand.firstname = "rand"; + adapter.put("1:a", rand, "persons"); assertThat(template.hasKey("persons:firstname:rand"), is(true)); diff --git a/src/test/java/org/springframework/data/redis/core/convert/BinaryKeyspaceIdentifierUnitTests.java b/src/test/java/org/springframework/data/redis/core/convert/BinaryKeyspaceIdentifierUnitTests.java index 821d93794..25d867e7f 100644 --- a/src/test/java/org/springframework/data/redis/core/convert/BinaryKeyspaceIdentifierUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/convert/BinaryKeyspaceIdentifierUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 the original author or authors. + * Copyright 2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/test/java/org/springframework/data/redis/core/convert/KeyspaceIdentifierUnitTests.java b/src/test/java/org/springframework/data/redis/core/convert/KeyspaceIdentifierUnitTests.java index daac75a57..6c7f66009 100644 --- a/src/test/java/org/springframework/data/redis/core/convert/KeyspaceIdentifierUnitTests.java +++ b/src/test/java/org/springframework/data/redis/core/convert/KeyspaceIdentifierUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 the original author or authors. + * Copyright 2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.