Merge pull request #378 from marklogic/feature/error-messages

rjrudin · web-flow · commit d1d4c2cf9a60 · 2024-12-11T16:13:12.000-05:00
Fix for specifying both namespaces
diff --git a/marklogic-langchain4j/src/main/java/com/marklogic/langchain4j/embedding/XmlChunkConfig.java b/marklogic-langchain4j/src/main/java/com/marklogic/langchain4j/embedding/XmlChunkConfig.java
@@ -23,8 +23,8 @@ public class XmlChunkConfig {
     private final NamespaceContext namespaceContext;
 
     // Defaults to the config used by the connector's splitter feature.
-    public XmlChunkConfig() {
-        this(null, null, null, null);
+    public XmlChunkConfig(String embeddingNamespace) {
+        this(null, null, embeddingNamespace, null);
     }
 
     public XmlChunkConfig(String textExpression, String embeddingName, String embeddingNamespace, NamespaceContext namespaceContext) {
diff --git a/marklogic-langchain4j/src/main/java/com/marklogic/langchain4j/splitter/ChunkConfig.java b/marklogic-langchain4j/src/main/java/com/marklogic/langchain4j/splitter/ChunkConfig.java
@@ -17,15 +17,20 @@ public class ChunkConfig {
     private final String documentType;
     private final String rootName;
     private final String xmlNamespace;
+    private final String embeddingXmlNamespace;
     private final String uriPrefix;
     private final String uriSuffix;
 
-    private ChunkConfig(DocumentMetadataHandle metadata, int maxChunks, String documentType, String rootName, String xmlNamespace, String uriPrefix, String uriSuffix) {
+    // Ignoring Sonar warning about too many constructor args, as that's mitigated via the builder.
+    @SuppressWarnings("java:S107")
+    private ChunkConfig(DocumentMetadataHandle metadata, int maxChunks, String documentType, String rootName,
+                        String xmlNamespace, String embeddingXmlNamespace, String uriPrefix, String uriSuffix) {
         this.metadata = metadata;
         this.maxChunks = maxChunks;
         this.documentType = documentType;
         this.rootName = rootName;
         this.xmlNamespace = xmlNamespace;
+        this.embeddingXmlNamespace = embeddingXmlNamespace;
         this.uriPrefix = uriPrefix;
         this.uriSuffix = uriSuffix;
     }
@@ -36,11 +41,17 @@ public static class Builder {
         private String documentType;
         private String rootName;
         private String xmlNamespace = Util.DEFAULT_XML_NAMESPACE;
+        private String embeddingXmlNamespace;
         private String uriPrefix;
         private String uriSuffix;
 
         public ChunkConfig build() {
-            return new ChunkConfig(metadata, maxChunks, documentType, rootName, xmlNamespace, uriPrefix, uriSuffix);
+            String tempNamespace = embeddingXmlNamespace;
+            if (tempNamespace == null) {
+                // If no embedding XML namespace is specified, default to the chunk namespace is defined.
+                tempNamespace = xmlNamespace != null ? xmlNamespace : Util.DEFAULT_XML_NAMESPACE;
+            }
+            return new ChunkConfig(metadata, maxChunks, documentType, rootName, xmlNamespace, tempNamespace, uriPrefix, uriSuffix);
         }
 
         public Builder withMetadata(DocumentMetadataHandle metadata) {
@@ -70,6 +81,13 @@ public Builder withXmlNamespace(String xmlNamespace) {
             return this;
         }
 
+        public Builder withEmbeddingXmlNamespace(String embeddingXmlNamespace) {
+            if (embeddingXmlNamespace != null) {
+                this.embeddingXmlNamespace = embeddingXmlNamespace;
+            }
+            return this;
+        }
+
         public Builder withUriPrefix(String uriPrefix) {
             this.uriPrefix = uriPrefix;
             return this;
@@ -108,4 +126,8 @@ public String getUriSuffix() {
     public String getXmlNamespace() {
         return xmlNamespace;
     }
+
+    public String getEmbeddingXmlNamespace() {
+        return embeddingXmlNamespace;
+    }
 }
diff --git a/marklogic-langchain4j/src/main/java/com/marklogic/langchain4j/splitter/XmlChunkDocumentProducer.java b/marklogic-langchain4j/src/main/java/com/marklogic/langchain4j/splitter/XmlChunkDocumentProducer.java
@@ -35,7 +35,7 @@ class XmlChunkDocumentProducer extends AbstractChunkDocumentProducer {
 
         // Namespaces aren't needed for producing chunks.
         this.domHelper = new DOMHelper(null);
-        this.xmlChunkConfig = new XmlChunkConfig();
+        this.xmlChunkConfig = new XmlChunkConfig(chunkConfig.getEmbeddingXmlNamespace());
     }
 
     @Override
diff --git a/marklogic-spark-connector/src/main/resources/marklogic-spark-messages.properties b/marklogic-spark-connector/src/main/resources/marklogic-spark-messages.properties
@@ -18,6 +18,7 @@ spark.marklogic.write.uriTemplate=
 spark.marklogic.write.xmlRootName=
 spark.marklogic.write.splitter.maxChunkSize=
 spark.marklogic.write.splitter.maxOverlapSize=
+spark.marklogic.write.splitter.sidecar.maxChunks=
 spark.marklogic.write.embedder.chunks.jsonPointer=
 spark.marklogic.write.embedder.chunks.xpath=
 spark.marklogic.write.embedder.batchSize=
diff --git a/marklogic-spark-connector/src/test/java/com/marklogic/spark/AbstractIntegrationTest.java b/marklogic-spark-connector/src/test/java/com/marklogic/spark/AbstractIntegrationTest.java
@@ -181,7 +181,8 @@ protected XmlNode readXmlDocument(String uri) {
         // Registers two frequently used namespaces in tests.
         return readXmlDocument(uri,
             Namespace.getNamespace("model", "http://marklogic.com/appservices/model"),
-            Namespace.getNamespace("ex", "org:example")
+            Namespace.getNamespace("ex", "org:example"),
+            Namespace.getNamespace("acme", "org:acme")
         );
     }
 
diff --git a/marklogic-spark-connector/src/test/java/com/marklogic/spark/writer/embedding/AddEmbeddingsToXmlTest.java b/marklogic-spark-connector/src/test/java/com/marklogic/spark/writer/embedding/AddEmbeddingsToXmlTest.java
@@ -66,7 +66,6 @@ void embeddingsInSourceDocument() {
         verifyEachChunkIsReturnedByAVectorQuery("xml_chunks");
     }
 
-    @ExtendWith(RequiresMarkLogic12.class)
     @Test
     void sidecarWithNamespace() {
         readDocument("/marklogic-docs/namespaced-java-client-intro.xml")
@@ -89,9 +88,8 @@ void sidecarWithNamespace() {
         doc.assertElementCount("/ex:sidecar/ex:chunks/ex:chunk", 4);
         for (XmlNode chunk : doc.getXmlNodes("/ex:sidecar/ex:chunks/ex:chunk")) {
             chunk.assertElementExists("/ex:chunk/ex:text");
-            chunk.assertElementExists("For now, the embedding still defaults to the empty namespace. We may change " +
-                "this soon to be a MarkLogic-specific namespace to better distinguish it from the users " +
-                "content.", "/ex:chunk/model:embedding");
+            chunk.assertElementExists("When a namespace is specified for the sidecar XML document, that should " +
+                "override the default namespace for the embedding element.", "/ex:chunk/ex:embedding");
         }
     }
 
@@ -107,7 +105,6 @@ void sidecarWithCustomNamespace() {
         readDocument("/marklogic-docs/java-client-intro.xml")
             .write().format(CONNECTOR_IDENTIFIER)
             .option(Options.CLIENT_URI, makeClientUri())
-            .option(Options.XPATH_NAMESPACE_PREFIX + "ex", "org:example")
             .option(Options.WRITE_SPLITTER_XPATH, "/node()/text/text()")
             .option(Options.WRITE_PERMISSIONS, DEFAULT_PERMISSIONS)
             .option(Options.WRITE_URI_TEMPLATE, "/split-test.xml")
@@ -117,13 +114,60 @@ void sidecarWithCustomNamespace() {
             .option(Options.WRITE_SPLITTER_SIDECAR_XML_NAMESPACE, "org:example")
             .option(Options.WRITE_SPLITTER_SIDECAR_COLLECTIONS, "namespaced-xml-vector-chunks")
             .option(Options.WRITE_EMBEDDER_MODEL_FUNCTION_CLASS_NAME, TEST_EMBEDDING_FUNCTION_CLASS)
+            .option(Options.WRITE_EMBEDDER_EMBEDDING_NAMESPACE, "http://marklogic.com/appservices/model")
             .mode(SaveMode.Append)
             .save();
 
         verifyChunksInNamespacedSidecar();
         verifyEachChunkIsReturnedByAVectorQuery("namespaced_xml_chunks");
     }
 
+    @Test
+    void sidecarWithCustomNamespaceAndCustomEmbeddingNamespace() {
+        readDocument("/marklogic-docs/java-client-intro.xml")
+            .write().format(CONNECTOR_IDENTIFIER)
+            .option(Options.CLIENT_URI, makeClientUri())
+            .option(Options.WRITE_SPLITTER_XPATH, "/node()/text/text()")
+            .option(Options.WRITE_PERMISSIONS, DEFAULT_PERMISSIONS)
+            .option(Options.WRITE_URI_TEMPLATE, "/split-test.xml")
+            .option(Options.WRITE_SPLITTER_SIDECAR_MAX_CHUNKS, 100)
+            .option(Options.WRITE_SPLITTER_SIDECAR_XML_NAMESPACE, "org:example")
+            .option(Options.WRITE_EMBEDDER_MODEL_FUNCTION_CLASS_NAME, TEST_EMBEDDING_FUNCTION_CLASS)
+            .option(Options.WRITE_EMBEDDER_EMBEDDING_NAMESPACE, "org:acme")
+            .mode(SaveMode.Append)
+            .save();
+
+        XmlNode doc = readXmlDocument("/split-test.xml-chunks-1.xml");
+        doc.assertElementValue("/ex:root/ex:source-uri", "/split-test.xml");
+        doc.assertElementExists("/ex:root/ex:chunks/ex:chunk[1]/ex:text");
+        doc.assertElementExists(
+            "When splitting and adding embeddings, the user can specify a namespace both for the sidecar document " +
+                "and a separate namespace for the embedding.",
+            "/ex:root/ex:chunks/ex:chunk[1]/acme:embedding"
+        );
+    }
+
+    @Test
+    void sidecarWithNoNamespace() {
+        readDocument("/marklogic-docs/java-client-intro.xml")
+            .write().format(CONNECTOR_IDENTIFIER)
+            .option(Options.CLIENT_URI, makeClientUri())
+            .option(Options.WRITE_SPLITTER_XPATH, "/node()/text/text()")
+            .option(Options.WRITE_PERMISSIONS, DEFAULT_PERMISSIONS)
+            .option(Options.WRITE_URI_TEMPLATE, "/split-test.xml")
+            .option(Options.WRITE_SPLITTER_SIDECAR_MAX_CHUNKS, 100)
+            .option(Options.WRITE_SPLITTER_SIDECAR_XML_NAMESPACE, "")
+            .option(Options.WRITE_EMBEDDER_MODEL_FUNCTION_CLASS_NAME, TEST_EMBEDDING_FUNCTION_CLASS)
+            .mode(SaveMode.Append)
+            .save();
+
+        XmlNode doc = readXmlDocument("/split-test.xml-chunks-1.xml");
+        doc.assertElementValue("/root/source-uri", "/split-test.xml");
+        doc.assertElementExists("/root/chunks/chunk[1]/text");
+        doc.assertElementExists("Since a namespace is specified for the document - no namespace - it should be " +
+            "applied to the embedding element too.", "/root/chunks/chunk[1]/embedding");
+    }
+
     @Test
     void customChunks() {
         readDocument("/marklogic-docs/custom-chunks.xml")
diff --git a/marklogic-spark-connector/src/test/java/com/marklogic/spark/writer/splitter/SplitJsonDocumentTest.java b/marklogic-spark-connector/src/test/java/com/marklogic/spark/writer/splitter/SplitJsonDocumentTest.java
@@ -174,6 +174,17 @@ void maxChunksOfThree() {
         tester.assertUpdatePermissionExists("spark-user-role");
     }
 
+    @Test
+    void invalidMaxChunksValue() {
+        DataFrameWriter writer = prepareToWriteChunkDocuments()
+            .option(Options.WRITE_SPLITTER_SIDECAR_MAX_CHUNKS, -1)
+            .mode(SaveMode.Append);
+
+        SparkException ex = assertThrows(SparkException.class, () -> writer.save());
+        assertTrue(ex.getMessage().contains("The value of 'spark.marklogic.write.splitter.sidecar.maxChunks' must be 0 or greater."),
+            "Unexpected error: " + ex.getMessage());
+    }
+
     @Test
     void maxChunksWithCustomPermissions() {
         prepareToWriteChunkDocuments()
diff --git a/marklogic-spark-langchain4j/src/main/java/com/marklogic/spark/langchain4j/DocumentTextSplitterFactory.java b/marklogic-spark-langchain4j/src/main/java/com/marklogic/spark/langchain4j/DocumentTextSplitterFactory.java
@@ -88,7 +88,8 @@ private static ChunkAssembler makeChunkAssembler(Context context) {
             .withRootName(context.getStringOption(Options.WRITE_SPLITTER_SIDECAR_ROOT_NAME))
             .withUriPrefix(context.getStringOption(Options.WRITE_SPLITTER_SIDECAR_URI_PREFIX))
             .withUriSuffix(context.getStringOption(Options.WRITE_SPLITTER_SIDECAR_URI_SUFFIX))
-            .withXmlNamespace(context.getStringOption(Options.WRITE_SPLITTER_SIDECAR_XML_NAMESPACE))
+            .withXmlNamespace(context.getProperties().get(Options.WRITE_SPLITTER_SIDECAR_XML_NAMESPACE))
+            .withEmbeddingXmlNamespace(context.getProperties().get(Options.WRITE_EMBEDDER_EMBEDDING_NAMESPACE))
             .build()
         );
     }
diff --git a/marklogic-spark-langchain4j/src/main/java/com/marklogic/spark/langchain4j/EmbeddingAdderFactory.java b/marklogic-spark-langchain4j/src/main/java/com/marklogic/spark/langchain4j/EmbeddingAdderFactory.java
@@ -82,7 +82,7 @@ private static ChunkSelector makeXmlChunkSelector(Context context) {
         XmlChunkConfig xmlChunkConfig = new XmlChunkConfig(
             context.getStringOption(Options.WRITE_EMBEDDER_TEXT_XPATH),
             context.getStringOption(Options.WRITE_EMBEDDER_EMBEDDING_NAME),
-            context.getStringOption(Options.WRITE_EMBEDDER_EMBEDDING_NAMESPACE),
+            context.getProperties().get(Options.WRITE_EMBEDDER_EMBEDDING_NAMESPACE),
             NamespaceContextFactory.makeNamespaceContext(context.getProperties())
         );
         return new DOMChunkSelector(

Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,8 @@ public class XmlChunkConfig {`
`23`	`23`	`private final NamespaceContext namespaceContext;`
`24`	`24`
`25`	`25`	`// Defaults to the config used by the connector's splitter feature.`
`26`		`- public XmlChunkConfig() {`
`27`		`- this(null, null, null, null);`
	`26`	`+ public XmlChunkConfig(String embeddingNamespace) {`
	`27`	`+ this(null, null, embeddingNamespace, null);`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`public XmlChunkConfig(String textExpression, String embeddingName, String embeddingNamespace, NamespaceContext namespaceContext) {`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ class XmlChunkDocumentProducer extends AbstractChunkDocumentProducer {`
`35`	`35`
`36`	`36`	`// Namespaces aren't needed for producing chunks.`
`37`	`37`	`this.domHelper = new DOMHelper(null);`
`38`		`- this.xmlChunkConfig = new XmlChunkConfig();`
	`38`	`+ this.xmlChunkConfig = new XmlChunkConfig(chunkConfig.getEmbeddingXmlNamespace());`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -181,7 +181,8 @@ protected XmlNode readXmlDocument(String uri) {`
`181`	`181`	`// Registers two frequently used namespaces in tests.`
`182`	`182`	`return readXmlDocument(uri,`
`183`	`183`	`Namespace.getNamespace("model", "http://marklogic.com/appservices/model"),`
`184`		`- Namespace.getNamespace("ex", "org:example")`
	`184`	`+ Namespace.getNamespace("ex", "org:example"),`
	`185`	`+ Namespace.getNamespace("acme", "org:acme")`
`185`	`186`	`);`
`186`	`187`	`}`
`187`	`188`