davsclaus commented on code in PR #25900:
URL: https://github.com/apache/camel/pull/25900#discussion_r3889255191
##########
components/camel-ai/camel-weaviate/src/main/java/org/apache/camel/component/weaviate/transform/WeaviateEmbeddingsDataTypeTransformer.java:
##########
@@ -58,30 +58,40 @@ public void transform(Message message, DataType fromType,
DataType toType) {
case QUERY ->
queryEmbeddingOperation(message, embedding, vectorFieldName,
textFieldName, text, collectionName, keyValue,
keyName);
- default -> throw new IllegalStateException("The only operations
supported are create and updatebyid");
+ default ->
+ throw new IllegalStateException("The only operations supported
are create, updatebyid and query");
}
}
private static void createEmbeddingOperation(
Message message, Embedding embedding, String vectorFieldName,
String textFieldName, TextSegment text,
String collectionName, Object keyValue, String keyName) {
message.setBody(embedding.vectorAsList(), List.class);
-
- if (ObjectHelper.isNotEmpty(keyValue) &&
ObjectHelper.isNotEmpty(keyName)) {
- HashMap<String, Object> maps = new HashMap<String, Object>();
- maps.put(keyName, keyValue);
- message.setHeader(WeaviateVectorDbHeaders.PROPERTIES, maps);
- }
+ setProperties(message, textFieldName, text, keyValue, keyName);
}
private static void updateEmbeddingOperation(
Message message, Embedding embedding, String vectorFieldName,
String textFieldName, TextSegment text,
String collectionName, Object keyValue, String keyName) {
message.setBody(embedding.vectorAsList(), List.class);
+ setProperties(message, textFieldName, text, keyValue, keyName);
+ }
+ /**
+ * Writes the object properties for a CREATE / UPDATE_BY_ID operation. The
embedded text is stored under
+ * textFieldName so that the source passage can be retrieved later;
without it only the vector (and optional id) was
+ * persisted and the original text was lost. Mirrors the Milvus
transformer.
+ */
+ private static void setProperties(
+ Message message, String textFieldName, TextSegment text, Object
keyValue, String keyName) {
+ HashMap<String, Object> maps = new HashMap<>();
+ if (text != null && text.text() != null) {
+ maps.put(textFieldName, text.text());
+ }
if (ObjectHelper.isNotEmpty(keyValue) &&
ObjectHelper.isNotEmpty(keyName)) {
- HashMap<String, Object> maps = new HashMap<String, Object>();
maps.put(keyName, keyValue);
+ }
+ if (!maps.isEmpty()) {
message.setHeader(WeaviateVectorDbHeaders.PROPERTIES, maps);
}
}
Review Comment:
This unconditionally builds a **fresh** `HashMap` and replaces the
`PROPERTIES` header, discarding any properties the caller already set on the
message before invoking this transformer.
Git blame shows the original code only touched `PROPERTIES` when
`keyValue`/`keyName` were present, specifically so a caller-supplied
`PROPERTIES` header would survive untouched. The existing IT
`LangChain4jEmbeddingsComponentWeaviateTargetIT` relies on exactly that:
`create()` sets `PROPERTIES` to `{sky: blue, age: 34}` and `updateById()` sets
it to `{dog: dachshund}` before the route runs the transformer. With this
change both calls to `setProperties` overwrite those maps, so only `text` (and
the id, if present) survive — which is why `querybyid` now fails:
`{"text"="hi"}` no longer contains `sky`/`age`/`dog`.
Could you merge into any existing `PROPERTIES` header instead of replacing
it — e.g. read `message.getHeader(WeaviateVectorDbHeaders.PROPERTIES,
Map.class)` first and add `textFieldName`/`keyName` into a copy of that (only
creating a new map when none exists)? It'd also help to extend
`WeaviateEmbeddingsDataTypeTransformerTest` to cover a pre-populated
`PROPERTIES` header, since that's the exact case the new test currently misses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]