Croway commented on code in PR #29108:
URL: https://github.com/apache/camel/pull/29108#discussion_r4456929462
##########
components/camel-spring-parent/camel-spring-ai/camel-spring-ai-embeddings/src/main/java/org/apache/camel/component/springai/embeddings/SpringAiEmbeddingsProducer.java:
##########
@@ -109,4 +112,44 @@ public void process(Exchange exchange) throws Exception {
message.setHeader(SpringAiEmbeddingsHeaders.EMBEDDING_METADATA,
response.getMetadata());
}
}
+
+ /**
+ * The text to embed of a body, or of an element of a List body: a Spring
AI Document as its text, and text, bytes,
+ * a stream, a Reader or a file read as text. Any other value, such as a
Map or a POJO, fails as an
+ * InvalidPayloadException instead of being embedded as its toString().
+ *
+ * @param index the index of the element in a List body, or -1 for the
body itself
+ */
+ private static String toText(Exchange exchange, Object value, int index)
+ throws InvalidPayloadException, NoTypeConversionAvailableException
{
+ String what = index < 0 ? "" : "the element at index " + index + " of
the List body is " + typeOf(value) + "; ";
+ if (value instanceof Document document) {
+ if (document.getText() == null) {
+ throw new InvalidPayloadException(
+ exchange, String.class, exchange.getMessage(),
+ what + "spring-ai-embeddings embeds the text of a
Document, but the Document has no text");
+ }
+ return document.getText();
+ }
+ if (!isReadableAsText(value)) {
+ throw new InvalidPayloadException(
+ exchange, String.class, exchange.getMessage(),
+ what + "spring-ai-embeddings embeds text, bytes, a stream,
a Reader, a file or a Spring AI Document;"
+ + " convert
the body to text before the endpoint, such"
+ + " as with
convertBodyTo(String) or a marshal to JSON");
+ }
+ return
exchange.getContext().getTypeConverter().mandatoryConvertTo(String.class,
exchange, value);
+ }
+
+ /** A value Camel converts to its text, rather than to the toString() of
an object. */
+ private static boolean isReadableAsText(Object value) {
Review Comment:
This list also fails bodies that have a real converter to `String` and were
embedded correctly before. With the fake `EmbeddingModel`:
| Body | main | PR |
|---|---|---|
| `char[]` `"hello"` | embeds `hello` | `InvalidPayloadException ... has
type: char[]` |
| Netty `ByteBuf` (netty-http with `disableStreamCache=true`) | embeds the
text | `InvalidPayloadException ... InstrumentedUnpooledHeapByteBuf` |
| a custom type with `addTypeConverter(String.class, Ticket.class, ...)` |
embeds the converter's text | `InvalidPayloadException` |
| a `List` of a `ByteBuf` and a `char[]` | embeds both | `the element at
index 0 of the List body is ...ByteBuf` |
`LangChain4jEmbeddingsProducer.isReadableAsText` has the same list, so this
matches camel-langchain4j-embeddings and could be a follow-up for both. Asking
the type converter is no way out: after one fallback conversion,
`lookup(String.class, Map1.class)` returns the promoted
`ToStringTypeConverter`. `BodyFacts.kindOf` does not help either: it classifies
`ByteBuf` as DATA and `char[]` as COLLECTION, and an `Integer` as PAYLOAD.
For this PR, the upgrade guide entry could mention these bodies and
`convertBodyTo(String.class)`.
##########
components/camel-spring-parent/camel-spring-ai/camel-spring-ai-embeddings/src/main/java/org/apache/camel/component/springai/embeddings/SpringAiEmbeddingsProducer.java:
##########
@@ -109,4 +112,44 @@ public void process(Exchange exchange) throws Exception {
message.setHeader(SpringAiEmbeddingsHeaders.EMBEDDING_METADATA,
response.getMetadata());
}
}
+
+ /**
+ * The text to embed of a body, or of an element of a List body: a Spring
AI Document as its text, and text, bytes,
+ * a stream, a Reader or a file read as text. Any other value, such as a
Map or a POJO, fails as an
+ * InvalidPayloadException instead of being embedded as its toString().
+ *
+ * @param index the index of the element in a List body, or -1 for the
body itself
+ */
+ private static String toText(Exchange exchange, Object value, int index)
+ throws InvalidPayloadException, NoTypeConversionAvailableException
{
+ String what = index < 0 ? "" : "the element at index " + index + " of
the List body is " + typeOf(value) + "; ";
Review Comment:
Nit: `what` (with `typeOf(value)` and the concatenation) is built for every
element of a `List` body, also when nothing fails. camel-langchain4j-embeddings
builds it only in the throw branch.
##########
components/camel-spring-parent/camel-spring-ai/camel-spring-ai-embeddings/src/main/java/org/apache/camel/component/springai/embeddings/SpringAiEmbeddingsEndpoint.java:
##########
@@ -37,10 +37,11 @@
syntax = "spring-ai-embeddings:embeddingId", producerOnly = true,
category = {
Category.AI
}, headersClass = SpringAiEmbeddingsHeaders.class)
-@BodyTypes(producerIn = @BodyType(value = Carrier.ANY,
- description = "The text to embed, or a List
of texts for a batch. Another body, and"
- + " each element of a List, is
converted to text (a Map or a POJO as its"
- + " toString()); no body, or a
null element in a List, fails"),
+@BodyTypes(producerIn = @BodyType(value = { Carrier.TEXT, Carrier.BYTES,
Carrier.STREAM, Carrier.FILE, Carrier.LIST },
Review Comment:
`Carrier.LIST` is documented as "A List, another Collection or an array",
but only a `java.util.List` is batched. A `Set<String>` or `String[]` body
fails with `InvalidPayloadException`. On main they were embedded as one text,
`[a, b]` and `[Ljava.lang.String;@39b7c350`, so failing is better, but the
description could say "a java.util.List of them" so that tooling does not offer
a Set or an array.
A `Document` is accepted, but `Carrier.POJO` is not declared.
camel-langchain4j-embeddings does the same, so that part is up to you.
--
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]
