Skip to content

Commit 0673351

Browse files
committed
Better error message for invalid permission capabilities
Ran into this while testing sidecar permissions in Flux.
1 parent d1d4c2c commit 0673351

File tree

6 files changed

+51
-10
lines changed

6 files changed

+51
-10
lines changed

marklogic-langchain4j/src/main/java/com/marklogic/langchain4j/Util.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.fasterxml.jackson.databind.JsonNode;
88
import com.fasterxml.jackson.databind.ObjectMapper;
99
import com.marklogic.client.impl.HandleAccessor;
10+
import com.marklogic.client.io.DocumentMetadataHandle;
1011
import com.marklogic.client.io.JacksonHandle;
1112
import com.marklogic.client.io.marker.AbstractWriteHandle;
1213
import org.slf4j.Logger;
@@ -35,4 +36,23 @@ static JsonNode getJsonFromHandle(AbstractWriteHandle writeHandle) {
3536
}
3637
}
3738
}
39+
40+
static void addPermissionsFromDelimitedString(DocumentMetadataHandle.DocumentPermissions permissions,
41+
String rolesAndCapabilities) {
42+
// This isn't likely the best home for this class, but it's needed by this module and by the connector to
43+
// massage an error message that is meaningful for a Java Client user but not meaningful for a connector
44+
// or Flux user.
45+
try {
46+
permissions.addFromDelimitedString(rolesAndCapabilities);
47+
} catch (IllegalArgumentException ex) {
48+
String message = ex.getMessage();
49+
final String confusingMessageForUser = "No enum constant com.marklogic.client.io.DocumentMetadataHandle.Capability.";
50+
if (message != null && message.contains(confusingMessageForUser)) {
51+
message = message.replace(confusingMessageForUser, "Not a valid capability: ");
52+
throw new IllegalArgumentException(message, ex);
53+
}
54+
throw ex;
55+
}
56+
}
57+
3858
}

marklogic-spark-connector/src/main/java/com/marklogic/spark/writer/DocBuilderFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package com.marklogic.spark.writer;
55

66
import com.marklogic.client.io.DocumentMetadataHandle;
7+
import com.marklogic.langchain4j.Util;
78

89
/**
910
* This is intended to migrate to java-client-api and likely just be a Builder class on DocumentWriteOperation.
@@ -29,7 +30,7 @@ DocBuilderFactory withCollections(String collections) {
2930
}
3031

3132
DocBuilderFactory withPermissions(String permissionsString) {
32-
metadata.getPermissions().addFromDelimitedString(permissionsString);
33+
Util.addPermissionsFromDelimitedString(metadata.getPermissions(), permissionsString);
3334
return this;
3435
}
3536

marklogic-spark-connector/src/main/java/com/marklogic/spark/writer/rdf/GraphWriter.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import com.marklogic.client.DatabaseClient;
77
import com.marklogic.client.eval.ServerEvaluationCall;
88
import com.marklogic.client.io.DocumentMetadataHandle;
9+
import com.marklogic.langchain4j.Util;
910
import org.slf4j.Logger;
1011
import org.slf4j.LoggerFactory;
1112

@@ -22,10 +23,10 @@ public class GraphWriter {
2223
private final DatabaseClient databaseClient;
2324
private final String permissions;
2425

25-
public GraphWriter(DatabaseClient databaseClient, String permissionsString) {
26+
public GraphWriter(DatabaseClient databaseClient, String rolesAndCapabilities) {
2627
this.databaseClient = databaseClient;
27-
this.permissions = permissionsString != null && permissionsString.trim().length() > 0 ?
28-
parsePermissions(permissionsString) :
28+
this.permissions = rolesAndCapabilities != null && rolesAndCapabilities.trim().length() > 0 ?
29+
parsePermissions(rolesAndCapabilities) :
2930
"xdmp:default-permissions()";
3031
}
3132

@@ -50,12 +51,12 @@ public void createGraphs(Set<String> graphs) {
5051
* We know the permissions string is valid at this point, as if it weren't, the writing process would have failed
5152
* before the connector gets to here.
5253
*
53-
* @param permissions
54+
* @param rolesAndCapabilities
5455
* @return
5556
*/
56-
private String parsePermissions(final String permissions) {
57+
private String parsePermissions(final String rolesAndCapabilities) {
5758
DocumentMetadataHandle metadata = new DocumentMetadataHandle();
58-
metadata.getPermissions().addFromDelimitedString(permissions);
59+
Util.addPermissionsFromDelimitedString(metadata.getPermissions(), rolesAndCapabilities);
5960
StringBuilder permissionsString = new StringBuilder("(");
6061
boolean firstOne = true;
6162
for (String role : metadata.getPermissions().keySet()) {

marklogic-spark-connector/src/test/java/com/marklogic/spark/writer/WriteRowsTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,21 @@ void invalidPermissionsConfig() {
204204
"string: rest-reader,read,rest-writer", cause.getMessage());
205205
}
206206

207+
@Test
208+
void invalidPermissionsCapability() {
209+
SparkException ex = assertThrows(SparkException.class, () -> newWriter()
210+
.option(Options.WRITE_PERMISSIONS, "rest-reader,read,rest-writer,notvalid")
211+
.save());
212+
213+
Throwable cause = getCauseFromWriterException(ex);
214+
assertTrue(cause instanceof IllegalArgumentException);
215+
assertEquals("Unable to parse permissions string: rest-reader,read,rest-writer,notvalid; cause: Not a valid capability: NOTVALID",
216+
cause.getMessage(), "When a capability is invalid, the Java Client throws an error message that refers " +
217+
"to the enum class. This likely won't make sense to a connector or Flux user. So the connector is " +
218+
"expected to massage the error a bit by identifying the invalid capability without referencing " +
219+
"Java classes.");
220+
}
221+
207222
@Test
208223
void dontAbortOnFailure() {
209224
AtomicInteger successCount = new AtomicInteger();

marklogic-spark-langchain4j/build.gradle

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
dependencies {
22
implementation project(":marklogic-spark-api")
33

4-
implementation (project(":marklogic-langchain4j")) {
4+
// API dependency so that public class in marklogic-langchain4j - specifically Util - can be accessed
5+
// in the connector.
6+
api (project(":marklogic-langchain4j")) {
57
// These need to be excluded here so they don't sneak in when this module is depended on by the
68
// marklogic-spark-connector module.
79
exclude group: "com.fasterxml.jackson.core"

marklogic-spark-langchain4j/src/main/java/com/marklogic/spark/langchain4j/DocumentTextSplitterFactory.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,17 @@ private static DocumentTextSplitter makeTextSplitter(Context context) {
7070

7171
private static ChunkAssembler makeChunkAssembler(Context context) {
7272
DocumentMetadataHandle metadata = new DocumentMetadataHandle();
73+
7374
if (context.hasOption(Options.WRITE_SPLITTER_SIDECAR_COLLECTIONS)) {
7475
metadata.getCollections().addAll(context.getStringOption(Options.WRITE_SPLITTER_SIDECAR_COLLECTIONS).split(","));
7576
}
77+
7678
if (context.hasOption(Options.WRITE_SPLITTER_SIDECAR_PERMISSIONS)) {
7779
String value = context.getStringOption(Options.WRITE_SPLITTER_SIDECAR_PERMISSIONS);
78-
metadata.getPermissions().addFromDelimitedString(value);
80+
com.marklogic.langchain4j.Util.addPermissionsFromDelimitedString(metadata.getPermissions(), value);
7981
} else if (context.hasOption(Options.WRITE_PERMISSIONS)) {
8082
String value = context.getStringOption(Options.WRITE_PERMISSIONS);
81-
metadata.getPermissions().addFromDelimitedString(value);
83+
com.marklogic.langchain4j.Util.addPermissionsFromDelimitedString(metadata.getPermissions(), value);
8284
}
8385

8486
return new DefaultChunkAssembler(new ChunkConfig.Builder()

0 commit comments

Comments
 (0)