From 25281199d27bbedafc4f240b47157cfbc9ae1134 Mon Sep 17 00:00:00 2001 From: Rob Rudin Date: Fri, 5 Jan 2024 10:02:12 -0500 Subject: [PATCH] MLE-4119: User can now submit an Optic update plan Will submit a separate PR if/when the REST API is updated to throw a nicer error message. --- .../marklogic/client/impl/RowManagerImpl.java | 23 ++- .../com/marklogic/client/row/RowManager.java | 12 ++ .../test/rows/AbstractOpticUpdateTest.java | 2 +- .../test/rows/FromDocDescriptorsTest.java | 4 +- .../client/test/rows/LockForUpdateTest.java | 165 ++++++++++-------- .../rows/ResultRowsWithTimestampTest.java | 2 + .../client/test/rows/RowManagerTest.java | 25 ++- 7 files changed, 134 insertions(+), 99 deletions(-) diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/RowManagerImpl.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/RowManagerImpl.java index e9d60150b..57ce16087 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/RowManagerImpl.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/RowManagerImpl.java @@ -62,6 +62,7 @@ public class RowManagerImpl private RowStructure rowStructureStyle = null; private Integer optimize; private String traceLabel; + private boolean update; public RowManagerImpl(RESTServices services) { super(); @@ -124,7 +125,13 @@ public void setOptimize(Integer value) { this.optimize = value; } - @Override + @Override + public RowManager withUpdate(boolean update) { + this.update = update; + return this; + } + + @Override public RawPlanDefinition newRawPlanDefinition(JSONWriteHandle handle) { return new RawPlanDefinitionImpl(handle); } @@ -176,7 +183,11 @@ public T resultDoc(Plan plan, T resultsHandle, T .withColumnTypes(getDatatypeStyle()) .withOutput(getRowStructureStyle()) .getRequestParameters(); - return services.postResource(requestLogger, "rows", transaction, params, astHandle, resultsHandle); + return services.postResource(requestLogger, determinePath(), transaction, params, astHandle, resultsHandle); + } + + private String determinePath() { + return this.update ? "rows/update" : "rows"; } @Override @@ -291,8 +302,9 @@ public T explain(Plan plan, T resultsHandle) { RequestParameters params = new RequestParameters(); params.add("output", "explain"); - return services.postResource(requestLogger, "rows", null, params, astHandle, resultsHandle); + return services.postResource(requestLogger, determinePath(), null, params, astHandle, resultsHandle); } + @Override public T explainAs(Plan plan, Class as) { ContentHandle handle = handleFor(as); @@ -409,11 +421,12 @@ private String getRowFormat(T rowHandle) { private RESTServiceResultIterator submitPlan(PlanBuilderBaseImpl.RequestPlan requestPlan, RequestParameters params, Transaction transaction) { AbstractWriteHandle astHandle = requestPlan.getHandle(); List contentParams = requestPlan.getContentParams(); + final String path = determinePath(); if (contentParams != null && !contentParams.isEmpty()) { contentParams.add(new ContentParam(new PlanBuilderBaseImpl.PlanParamBase("query"), astHandle, null)); - return services.postMultipartForm(requestLogger, "rows", transaction, params, contentParams); + return services.postMultipartForm(requestLogger, path, transaction, params, contentParams); } - return services.postIteratedResource(requestLogger, "rows", transaction, params, astHandle); + return services.postIteratedResource(requestLogger, path, transaction, params, astHandle); } private PlanBuilderBaseImpl.RequestPlan checkPlan(Plan plan) { diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/row/RowManager.java b/marklogic-client-api/src/main/java/com/marklogic/client/row/RowManager.java index d48cdfe9d..c78aae23b 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/row/RowManager.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/row/RowManager.java @@ -87,6 +87,18 @@ public enum RowStructure{ARRAY, OBJECT} */ void setTraceLabel(String label); + /** + * As of MarkLogic 11.2, the "v1/rows/update" endpoint must be used in order to submit an Optic plan that performs + * an update. This method must be called with a value of {@code true} in order for that endpoint to be used instead + * of "v1/rows". You may later call this method with a value of {@code false} in order to submit a plan that does + * not perform an update. + * + * @param update set to {@code true} if submitting a plan that performs an update + * @return the instance of this class + * @since 6.5.0 + */ + RowManager withUpdate(boolean update); + /** * @return the label that will be used for all log messages associated with the "optic" trace event */ diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/AbstractOpticUpdateTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/AbstractOpticUpdateTest.java index ca5d7eaa7..8654d6cb1 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/AbstractOpticUpdateTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/AbstractOpticUpdateTest.java @@ -47,7 +47,7 @@ public void setup() { .evalAs(String.class); Common.client = Common.newClientBuilder().withUsername("writer-no-default-permissions").build(); - rowManager = Common.client.newRowManager(); + rowManager = Common.client.newRowManager().withUpdate(true); op = rowManager.newPlanBuilder(); } diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/FromDocDescriptorsTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/FromDocDescriptorsTest.java index 2304af8dd..17f5b62f2 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/FromDocDescriptorsTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/FromDocDescriptorsTest.java @@ -46,7 +46,7 @@ public void insertDocsWithUserWithDefaultCollectionsAndPermissions() { mapper.createObjectNode().put("hello", "two"))); Common.client = Common.newClientBuilder().withUsername(USER_WITH_DEFAULT_COLLECTIONS_AND_PERMISSIONS).build(); - Common.client.newRowManager().execute(op + Common.client.newRowManager().withUpdate(true).execute(op .fromDocDescriptors(op.docDescriptors(writeSet)) .write()); @@ -88,7 +88,7 @@ public void updateOnlyDocAsUserWithNoDefaults() { public void updateOnlyDocWithUserWithDefaultCollectionsAndPermissions() { // Set up client as user with default collections and permissions Common.client = Common.newClientBuilder().withUsername(USER_WITH_DEFAULT_COLLECTIONS_AND_PERMISSIONS).build(); - rowManager = Common.client.newRowManager(); + rowManager = Common.client.newRowManager().withUpdate(true); op = rowManager.newPlanBuilder(); verifyOnlyDocCanBeUpdatedWithoutLosingAnyMetadata(); diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/LockForUpdateTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/LockForUpdateTest.java index 5f135acf9..36834dda4 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/LockForUpdateTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/LockForUpdateTest.java @@ -1,6 +1,7 @@ package com.marklogic.client.test.rows; import com.fasterxml.jackson.databind.node.ArrayNode; +import com.marklogic.client.FailedRequestException; import com.marklogic.client.expression.PlanBuilder; import com.marklogic.client.io.DocumentMetadataHandle; import com.marklogic.client.io.JacksonHandle; @@ -12,89 +13,99 @@ import java.util.List; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; @ExtendWith(RequiresML11.class) public class LockForUpdateTest extends AbstractOpticUpdateTest { - @Test - public void basicTest() { - final String uri = "/acme/doc1.json"; - - // Write a document - rowManager.execute(op.fromDocDescriptors( - op.docDescriptor(newWriteOp(uri, new JacksonHandle(mapper.createObjectNode().put("hello", "world"))))) - .write()); - verifyJsonDoc(uri, doc -> assertEquals("world", doc.get("hello").asText())); - - // Construct a plan that will lock the URI and update its collection - PlanBuilder.ModifyPlan plan = op - .fromDocDescriptors( - op.docDescriptor(newWriteOp(uri, new DocumentMetadataHandle().withCollections("optic1"), null)) - ) - .lockForUpdate() - .write(op.docCols(null, op.xs.stringSeq("uri", "collections"))); - - // Run an eval that locks the URI and sleeps for 2 seconds, which will block the plan run below - new Thread(() -> { - Common.newServerAdminClient().newServerEval() - .javascript(String.format("declareUpdate(); " + - "xdmp.lockForUpdate('%s'); " + - "xdmp.sleep(2000); " + - "xdmp.documentSetCollections('%s', ['eval1']);", uri, uri)) - .evalAs(String.class); - }).start(); - - // Immediately run a plan that updates the collections as well; this should be blocked while the eval thread - // above completes - long start = System.currentTimeMillis(); - rowManager.execute(plan); - long duration = System.currentTimeMillis() - start; - System.out.println("DUR: " + duration); - - assertTrue(duration > 1500, + @Test + public void basicTest() { + final String uri = "/acme/doc1.json"; + + // Write a document + rowManager.execute(op.fromDocDescriptors( + op.docDescriptor(newWriteOp(uri, new JacksonHandle(mapper.createObjectNode().put("hello", "world"))))) + .write()); + verifyJsonDoc(uri, doc -> assertEquals("world", doc.get("hello").asText())); + + // Construct a plan that will lock the URI and update its collection + PlanBuilder.ModifyPlan plan = op + .fromDocDescriptors( + op.docDescriptor(newWriteOp(uri, new DocumentMetadataHandle().withCollections("optic1"), null)) + ) + .lockForUpdate() + .write(op.docCols(null, op.xs.stringSeq("uri", "collections"))); + + // Run an eval that locks the URI and sleeps for 2 seconds, which will block the plan run below + new Thread(() -> { + Common.newServerAdminClient().newServerEval() + .javascript(String.format("declareUpdate(); " + + "xdmp.lockForUpdate('%s'); " + + "xdmp.sleep(2000); " + + "xdmp.documentSetCollections('%s', ['eval1']);", uri, uri)) + .evalAs(String.class); + }).start(); + + // Immediately run a plan that updates the collections as well; this should be blocked while the eval thread + // above completes + long start = System.currentTimeMillis(); + rowManager.execute(plan); + long duration = System.currentTimeMillis() - start; + System.out.println("DUR: " + duration); + + assertTrue(duration > 1500, "Because the eval call slept for 2 seconds, the duration of the plan execution should be at least " + "1500ms, which is much longer than normal; it may not be at least 2 seconds due to the small delay in " + "the Java layer of executing the plan; duration: " + duration); - // Verify that the collections were set based on the plan, which should have run second - verifyMetadata(uri, metadata -> { - DocumentMetadataHandle.DocumentCollections colls = metadata.getCollections(); - assertEquals(1, colls.size()); - assertEquals("optic1", colls.iterator().next()); - }); - } - - @Test - public void uriColumnSpecified() { - List rows = resultRows(op - .fromDocUris("/optic/test/musician1.json") - .lockForUpdate(op.col("uri"))); - assertEquals(1, rows.size()); - } - - @Test - public void fromParamWithCustomUriColumn() { - ArrayNode paramValue = mapper.createArrayNode(); - paramValue.addObject().put("myUri", "/optic/test/musician1.json"); - - List rows = resultRows(op - .fromParam("bindingParam", "", op.colTypes(op.colType("myUri", "string"))) - .lockForUpdate(op.col("myUri")) - .bindParam("bindingParam", new JacksonHandle(paramValue), null)); - assertEquals(1, rows.size()); - } - - @Test - public void fromParamWithQualifiedUriColumn() { - ArrayNode paramValue = mapper.createArrayNode(); - paramValue.addObject().put("myUri", "/optic/test/musician1.json"); - - List rows = resultRows(op - .fromParam("bindingParam", "myQualifier", op.colTypes(op.colType("myUri", "string"))) - .lockForUpdate(op.viewCol("myQualifier", "myUri")) - .bindParam("bindingParam", new JacksonHandle(paramValue), null)); - assertEquals(1, rows.size()); - } + // Verify that the collections were set based on the plan, which should have run second + verifyMetadata(uri, metadata -> { + DocumentMetadataHandle.DocumentCollections colls = metadata.getCollections(); + assertEquals(1, colls.size()); + assertEquals("optic1", colls.iterator().next()); + }); + } + + @Test + void wrongEndpoint() { + rowManager.withUpdate(false); + assertThrows( + FailedRequestException.class, + () -> rowManager.execute(op.fromDocUris("/optic/test/musician1.json").lockForUpdate()), + "Hoping to update this assertion to verify that the server message tells the user to hit v1/rows/update " + + "instead; right now, it's mentioning using declareUpdate() which isn't applicable to a REST API user." + ); + } + + @Test + public void uriColumnSpecified() { + List rows = resultRows(op + .fromDocUris("/optic/test/musician1.json") + .lockForUpdate(op.col("uri"))); + assertEquals(1, rows.size()); + } + + @Test + public void fromParamWithCustomUriColumn() { + ArrayNode paramValue = mapper.createArrayNode(); + paramValue.addObject().put("myUri", "/optic/test/musician1.json"); + + List rows = resultRows(op + .fromParam("bindingParam", "", op.colTypes(op.colType("myUri", "string"))) + .lockForUpdate(op.col("myUri")) + .bindParam("bindingParam", new JacksonHandle(paramValue), null)); + assertEquals(1, rows.size()); + } + + @Test + public void fromParamWithQualifiedUriColumn() { + ArrayNode paramValue = mapper.createArrayNode(); + paramValue.addObject().put("myUri", "/optic/test/musician1.json"); + + List rows = resultRows(op + .fromParam("bindingParam", "myQualifier", op.colTypes(op.colType("myUri", "string"))) + .lockForUpdate(op.viewCol("myQualifier", "myUri")) + .bindParam("bindingParam", new JacksonHandle(paramValue), null)); + assertEquals(1, rows.size()); + } } diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/ResultRowsWithTimestampTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/ResultRowsWithTimestampTest.java index 30131a22e..7534681ae 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/ResultRowsWithTimestampTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/ResultRowsWithTimestampTest.java @@ -32,6 +32,8 @@ void deleteNewMusician() { @Test void testResultRowsWithPointInTimeQueryTimestamp() { + rowManager.withUpdate(false); + final RawQueryDSLPlan plan = rowManager.newRawQueryDSLPlan(new StringHandle("op.fromView('opticUnitTest', 'musician_ml10')")); JacksonHandle result = new JacksonHandle(); diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/RowManagerTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/RowManagerTest.java index cefa52c30..61bb1f249 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/RowManagerTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/test/rows/RowManagerTest.java @@ -552,14 +552,14 @@ public void testErrorWhileStreamingRows() { "occurred due to a bad request by the user, since the query was valid in the sense that it could be " + "executed"); - assertEquals("SQL-TABLENOTFOUND", ex.getServerMessage(), - "The server error message is expected to be the value of the 'ml-error-message' trailer"); - - assertEquals( - "Local message: failed to apply resource at rows: SQL-TABLENOTFOUND, Internal Server Error. Server Message: SQL-TABLENOTFOUND", - ex.getMessage(), - "The exception message is expected to be a formatted message containing the values of the 'ml-error-code' and " + - "'ml-error-message' trailers"); + // For 11-nightly, changed these to be less precise as the server error message is free to change between minor + // releases, thus making any equality assertions very fragile. + assertTrue(ex.getServerMessage().contains("SQL-TABLENOTFOUND"), + "The server error message is expected to be the value of the 'ml-error-message' trailer"); + assertTrue( + ex.getMessage().contains("SQL-TABLENOTFOUND"), + "The exception message is expected to be a formatted message containing the values of the 'ml-error-code' and " + + "'ml-error-message' trailers"); } @Test @@ -1247,8 +1247,9 @@ public void testAggregates() throws IOException { recordRowSet.close(); } + @Test - public void testMapper() throws IOException, XPathExpressionException { + public void testMapper() { RowManager rowMgr = Common.client.newRowManager(); PlanBuilder p = rowMgr.newPlanBuilder(); @@ -1260,13 +1261,11 @@ public void testMapper() throws IOException, XPathExpressionException { .limit(3) .map(p.resolveFunction(p.xs.QName("secondsMapper"), "/etc/optic/test/processors.sjs")); - int rowNum = 0; for (RowRecord row: rowMgr.resultRows(builtPlan)) { assertNotNull(row.getInt("rowNum")); assertNotNull(row.getString("city")); int seconds = row.getInt("seconds"); assertTrue(0 <= seconds && seconds < 60); - rowNum++; } builtPlan = @@ -1279,13 +1278,11 @@ public void testMapper() throws IOException, XPathExpressionException { p.xs.string("/etc/optic/test/processors.xqy") )); - rowNum = 0; for (RowRecord row: rowMgr.resultRows(builtPlan)) { assertNotNull(row.getInt("rowNum")); assertNotNull(row.getString("city")); int seconds = row.getInt("seconds"); assertTrue(0 <= seconds && seconds < 60); - rowNum++; } } @@ -1319,7 +1316,7 @@ public void testColumnInfo() { } @Test - public void testGenerateView() throws IOException { + public void testGenerateView() { RowManager rowMgr = Common.client.newRowManager(); PlanBuilder p = rowMgr.newPlanBuilder();