feat: normalize key separators to prevent double separators

Port of Python RedisVL fix for issue #368. Addresses key separator
handling when prefix ends with the separator character.

Changes:
- BaseStorage.createKey(): Strip trailing separators from prefix before
  concatenating to avoid double separators (e.g., "user::123" becomes
  "user:123")
- SearchIndex.fetch(): Apply same normalization when checking if input
  is a full key to match new key format
- Add KeySeparatorTest with 8 unit tests covering edge cases
- Update JsonStorageIntegrationTest to use correct single-separator format

This ensures consistent key format regardless of whether the prefix
is defined with or without a trailing separator, matching Python
behavior where prefix.rstrip(separator) is used.

Author: bsbodden

core/src/main/java/com/redis/vl/index/SearchIndex.java

@@ -1148,10 +1148,21 @@ public List<String> load(
   public Map<String, Object> fetch(String idOrKey) {
     // If input already contains the prefix, use it as-is, otherwise construct the key
     String key;
-    if (getPrefix() != null
-        && !getPrefix().isEmpty()
-        && idOrKey.startsWith(getPrefix() + getKeySeparator())) {
-      key = idOrKey; // Already a full key
+    if (getPrefix() != null && !getPrefix().isEmpty()) {
+      // Normalize prefix to avoid double separator issues (issue #368)
+      String normalizedPrefix = getPrefix();
+      String separator = getKeySeparator();
+      if (separator != null && !separator.isEmpty() && normalizedPrefix.endsWith(separator)) {
+        normalizedPrefix =
+            normalizedPrefix.substring(0, normalizedPrefix.length() - separator.length());
+      }
+      String prefixWithSeparator = normalizedPrefix + separator;
+
+      if (idOrKey.startsWith(prefixWithSeparator)) {
+        key = idOrKey; // Already a full key
+      } else {
+        key = key(idOrKey); // Just an ID, construct the key
+      }
     } else {
       key = key(idOrKey); // Just an ID, construct the key
     }

core/src/main/java/com/redis/vl/storage/BaseStorage.java

@@ -42,6 +42,9 @@ public BaseStorage(IndexSchema indexSchema) {
   /**
    * Create a Redis key using a combination of a prefix, separator, and the identifier.
    *
+   * <p>Python port: Normalizes the prefix by removing trailing separators to avoid double
+   * separators (issue #368). Matches behavior of Python's BaseStorage._key() method.
+   *
    * @param id The unique identifier for the Redis entry
    * @param prefix A prefix to append before the key value
    * @param keySeparator A separator to insert between prefix and key value
@@ -50,9 +53,24 @@ public BaseStorage(IndexSchema indexSchema) {
   protected static String createKey(String id, String prefix, String keySeparator) {
     if (prefix == null || prefix.isEmpty()) {
       return id;
-    } else {
-      return prefix + keySeparator + id;
     }
+
+    // Normalize prefix by removing trailing separators to avoid doubles (issue #368)
+    // Python equivalent: prefix.rstrip(key_separator) if key_separator else prefix
+    String normalizedPrefix = prefix;
+    if (keySeparator != null && !keySeparator.isEmpty()) {
+      while (normalizedPrefix.endsWith(keySeparator)) {
+        normalizedPrefix =
+            normalizedPrefix.substring(0, normalizedPrefix.length() - keySeparator.length());
+      }
+    }
+
+    // If normalized prefix is empty, return just the id
+    if (normalizedPrefix.isEmpty()) {
+      return keySeparator + id;
+    }
+
+    return normalizedPrefix + keySeparator + id;
   }
 
   /**

core/src/test/java/com/redis/vl/index/JsonStorageIntegrationTest.java

@@ -223,7 +223,8 @@ private static void loadTestData() {
   void testDocumentsStored() {
     // Check if we can fetch a document
     // The key format is prefix + separator + id, where separator is ":"
-    Map<String, Object> doc = jsonIndex.fetch("json:" + TEST_PREFIX + "::Laptop Pro");
+    // Note: Prefix already ends with ":", so no double separator (issue #368)
+    Map<String, Object> doc = jsonIndex.fetch("json:" + TEST_PREFIX + ":Laptop Pro");
     assertThat(doc).as("Document should be retrievable").isNotNull();
     assertThat(doc.get("title")).isEqualTo("Laptop Pro");
 
@@ -249,7 +250,8 @@ void testJsonVectorQueryWithJsonPath() {
     assertThat(indexInfo).isNotNull();
 
     // Fetch a document to see its structure
-    String docId = "json:" + TEST_PREFIX + "::Laptop Pro";
+    // Note: Prefix already ends with ":", so no double separator (issue #368)
+    String docId = "json:" + TEST_PREFIX + ":Laptop Pro";
     Map<String, Object> doc = jsonIndex.fetch(docId);
     assertThat(doc).isNotNull();
 
@@ -598,10 +600,15 @@ void testJsonFetchDocument() {
     assertThat(keys).hasSize(1);
     String key = keys.get(0);
 
+    // Verify the key was written to Redis
+    boolean keyExists = unifiedJedis.exists(key);
+    assertThat(keyExists).as("Key should exist in Redis: " + key).isTrue();
+
     // Fetch the document
     Map<String, Object> doc = jsonIndex.fetch(key);
 
     // Should preserve nested structure
+    assertThat(doc).as("Document should be fetched with key: " + key).isNotNull();
     assertThat(doc).containsKey("metadata");
     assertThat(doc.get("metadata")).isInstanceOf(Map.class);
 

core/src/test/java/com/redis/vl/storage/KeySeparatorTest.java

@@ -0,0 +1,130 @@
+package com.redis.vl.storage;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Unit tests for key separator handling in BaseStorage.
+ *
+ * <p>Tests the fix for issue #368: Handle key separators correctly to avoid double separators when
+ * prefix ends with the separator character.
+ *
+ * <p>Python port: Ensures BaseStorage.createKey() normalizes prefixes by stripping trailing
+ * separators, matching the behavior of Python's BaseStorage._key() method.
+ */
+@DisplayName("Key Separator Handling Tests")
+class KeySeparatorTest {
+
+  @Test
+  @DisplayName("Should avoid double separator when prefix ends with separator")
+  void testPrefixEndingWithSeparator() {
+    // Issue #368: Prefix ending with separator should not create double separator
+    String key = BaseStorage.createKey("123", "user:", ":");
+    assertThat(key)
+        .isEqualTo("user:123")
+        .describedAs("Should strip trailing separator from prefix");
+
+    key = BaseStorage.createKey("456", "app:", ":");
+    assertThat(key).isEqualTo("app:456").describedAs("Should strip trailing separator from prefix");
+  }
+
+  @Test
+  @DisplayName("Should work correctly when prefix does not end with separator")
+  void testPrefixWithoutTrailingSeparator() {
+    // Normal case: prefix without trailing separator
+    String key = BaseStorage.createKey("123", "user", ":");
+    assertThat(key).isEqualTo("user:123").describedAs("Should add separator between prefix and id");
+
+    key = BaseStorage.createKey("456", "app", ":");
+    assertThat(key).isEqualTo("app:456").describedAs("Should add separator between prefix and id");
+  }
+
+  @Test
+  @DisplayName("Should handle empty or null prefix")
+  void testEmptyPrefix() {
+    // Empty prefix should return just the id
+    String key = BaseStorage.createKey("123", "", ":");
+    assertThat(key).isEqualTo("123").describedAs("Empty prefix should return just the id");
+
+    // Null prefix should return just the id
+    key = BaseStorage.createKey("123", null, ":");
+    assertThat(key).isEqualTo("123").describedAs("Null prefix should return just the id");
+  }
+
+  @Test
+  @DisplayName("Should handle different separator characters")
+  void testDifferentSeparators() {
+    // Test with underscore separator
+    String key = BaseStorage.createKey("123", "prefix_", "_");
+    assertThat(key)
+        .isEqualTo("prefix_123")
+        .describedAs("Should strip trailing underscore from prefix");
+
+    key = BaseStorage.createKey("123", "prefix", "_");
+    assertThat(key)
+        .isEqualTo("prefix_123")
+        .describedAs("Should add underscore between prefix and id");
+
+    // Test with dash separator
+    key = BaseStorage.createKey("123", "prefix-", "-");
+    assertThat(key).isEqualTo("prefix-123").describedAs("Should strip trailing dash from prefix");
+
+    key = BaseStorage.createKey("123", "prefix", "-");
+    assertThat(key).isEqualTo("prefix-123").describedAs("Should add dash between prefix and id");
+  }
+
+  @Test
+  @DisplayName("Should handle multi-level prefixes with trailing separator")
+  void testMultiLevelPrefix() {
+    // Multi-level prefix ending with separator
+    String key = BaseStorage.createKey("123", "app:env:service:", ":");
+    assertThat(key)
+        .isEqualTo("app:env:service:123")
+        .describedAs("Should strip trailing separator from multi-level prefix");
+
+    // Multi-level prefix without trailing separator
+    key = BaseStorage.createKey("123", "app:env:service", ":");
+    assertThat(key)
+        .isEqualTo("app:env:service:123")
+        .describedAs("Should add separator to multi-level prefix");
+  }
+
+  @Test
+  @DisplayName("Should handle null or empty separator")
+  void testNullOrEmptySeparator() {
+    // Empty separator should concatenate directly
+    String key = BaseStorage.createKey("123", "prefix", "");
+    assertThat(key)
+        .isEqualTo("prefix123")
+        .describedAs("Empty separator should concatenate directly");
+
+    // Null separator - current behavior might vary, test for safety
+    // The fix should handle this edge case gracefully
+    key = BaseStorage.createKey("123", "prefix", null);
+    // With null separator, we can't strip trailing separator, so just concatenate
+    assertThat(key).isNotNull().describedAs("Should handle null separator gracefully");
+  }
+
+  @Test
+  @DisplayName("Should handle prefix that is just the separator")
+  void testPrefixIsJustSeparator() {
+    // Edge case: prefix is just the separator character
+    String key = BaseStorage.createKey("123", ":", ":");
+    assertThat(key)
+        .isEqualTo(":123")
+        .describedAs("Prefix of just separator should normalize to separator + id");
+  }
+
+  @Test
+  @DisplayName("Should handle multiple trailing separators")
+  void testMultipleTrailingSeparators() {
+    // Edge case: prefix ends with multiple separator characters
+    // Note: Python's rstrip() removes all trailing occurrences
+    String key = BaseStorage.createKey("123", "prefix:::", ":");
+    assertThat(key)
+        .isEqualTo("prefix:123")
+        .describedAs("Should strip all trailing separators from prefix");
+  }
+}