redis vectorset embedding store

[Jacopo47]

Issue

Closes #393

Change

General checklist

• There are no breaking changes
• I have added unit and integration tests for my change
• I have manually run all the unit tests in all modules, and they are all green
• I have manually run all integration tests in the module I have added/changed, and they are all green

• I have added/updated the documentation
• I have added an example in the examples repo (only for "big" features)
• I have added/updated Spring Boot starter(s) (if applicable)

Checklist for adding new maven module

• I have added my new module in the root pom.xml and langchain4j-community-bom/pom.xml

Checklist for adding new embedding store integration

• I have added a {NameOfIntegration}EmbeddingStoreIT that extends from either EmbeddingStoreIT or EmbeddingStoreWithFilteringIT
• I have added a {NameOfIntegration}EmbeddingStoreRemovalIT that extends from EmbeddingStoreWithRemovalIT

Checklist for changing existing embedding store integration

• I have manually verified that the {NameOfIntegration}EmbeddingStore works correctly with the data persisted using the latest released version of LangChain4j

[Martin7-1]

@Jacopo47 Thanks for the great work! I'm wondering if we should use UnifiedJedis as the client like RedisEmbeddingStore?

[Martin7-1]

@Jacopo47 Thank you!

Could you please add Builder pattern and some javadoc in the constructor and class to clarify the meaing of parameters in the constructor.

BTW, to make spotless happy, please run make format. Thank you!

[Martin7-1]

copy-paste issue in filename, should be RedisVectorSetsEmbeddingStoreRemovalIT.java

[Jacopo47]

Fixed

[Martin7-1]

Please extend it with EmbeddingStoreIT or EmbeddingStoreWithFilteringIT

[Martin7-1]

nit: could be simplified with:

var joined = i
                    .stream()
                    .map(e -> {
                        if (e == null) {
                            return null;
                        } else if (e instanceof String s) {
                            return toString(s);
                        } else if (e instanceof  UUID s){
                            return toString(s);
                        } else {
                            return e.toString();
                        }
                    })
                    .filter(Objects::nonNull)
                    .collect(Collectors.joining(","));

[Jacopo47]

Thanks! Makes sense to me!
Fixed

[Martin7-1]

duplicate filter.isPresent() check here.

[Jacopo47]

Fixed

[Martin7-1]

I think throwing an exception when there are no matching elements seems a bit too harsh. Wouldn't it be better to just return an empty list?

[Jacopo47]

It should never throw an exception because if keep only Optional that actually has a value
BUT I agree with you that it's a bit too cruel so I changed it in:

  List<EmbeddingMatch<TextSegment>> matches = similarElements
          .entrySet()
          .stream()
          .map(this::mapToEmbeddingMatch)
          .flatMap(Optional::stream)

[Jacopo47]

Hi, @Martin7-1
thank you for your review!
I'll work on it asap

[Jacopo47]

Hi @Martin7-1 ,
I worked on some of your suggestions.
I still have to work on other fixes but I would wait for Redis' developments to be complete:

• [QUESTION] Should VSIM filter expression consider missing json fields implicitly as null? redis/redis#14499 (comment)
• [QUESTION] Should VSIM filter expression support greater-than and less-than while comparing actual strings? redis/redis#14611

Because I would understand if these situation can be addressed by Redis it self or some "walk around" must be made by the embedding store.
(hopefully they will be addressed by redis)

[Martin7-1]

Great! No worry! Just ping me once you complete all tasks.