From 1a55b22d8989073160f10a9e29e482abffea93c4 Mon Sep 17 00:00:00 2001 From: Rob Rudin Date: Wed, 24 Jan 2024 16:33:46 -0500 Subject: [PATCH 1/2] Revert "Merge pull request #1631 from marklogic/feature/update-error-message" This reverts commit dd2e8611ea1c7c30962d550ad41c6bc9609e4154. --- .../marklogic/client/impl/RowManagerImpl.java | 26 +++++++------------ .../client/test/rows/LockForUpdateTest.java | 8 +----- 2 files changed, 10 insertions(+), 24 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 aa8c6c412..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 @@ -33,8 +33,11 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.JsonNodeType; -import com.marklogic.client.*; import com.marklogic.client.DatabaseClientFactory.HandleFactoryRegistry; +import com.marklogic.client.MarkLogicBindingException; +import com.marklogic.client.MarkLogicIOException; +import com.marklogic.client.MarkLogicInternalException; +import com.marklogic.client.Transaction; import com.marklogic.client.document.DocumentWriteSet; import com.marklogic.client.expression.PlanBuilder; import com.marklogic.client.expression.PlanBuilder.Plan; @@ -419,22 +422,11 @@ private RESTServiceResultIterator submitPlan(PlanBuilderBaseImpl.RequestPlan req AbstractWriteHandle astHandle = requestPlan.getHandle(); List contentParams = requestPlan.getContentParams(); final String path = determinePath(); - try { - if (contentParams != null && !contentParams.isEmpty()) { - contentParams.add(new ContentParam(new PlanBuilderBaseImpl.PlanParamBase("query"), astHandle, null)); - return services.postMultipartForm(requestLogger, path, transaction, params, contentParams); - } - return services.postIteratedResource(requestLogger, path, transaction, params, astHandle); - } catch (FailedRequestException ex) { - String message = ex.getMessage(); - if (message != null && message.contains("RESTAPI-UPDATEFROMQUERY")) { - String betterMessage = "The Optic plan is attempting an update but was sent to the wrong REST API endpoint. " + - "You must invoke `withUpdate(true)` on the instance of com.marklogic.client.row.RowManager that you " + - "are using to submit the plan"; - throw new FailedRequestException(betterMessage, ex.getFailedRequest()); - } - throw ex; - } + if (contentParams != null && !contentParams.isEmpty()) { + contentParams.add(new ContentParam(new PlanBuilderBaseImpl.PlanParamBase("query"), astHandle, null)); + return services.postMultipartForm(requestLogger, path, transaction, params, contentParams); + } + return services.postIteratedResource(requestLogger, path, transaction, params, astHandle); } private PlanBuilderBaseImpl.RequestPlan checkPlan(Plan plan) { 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 82bbf950a..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 @@ -69,18 +69,12 @@ public void basicTest() { @Test void wrongEndpoint() { rowManager.withUpdate(false); - FailedRequestException ex = assertThrows( + 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." ); - - assertTrue(ex.getMessage().contains( - "The Optic plan is attempting an update but was sent to the wrong REST API endpoint. " + - "You must invoke `withUpdate(true)` on the instance of com.marklogic.client.row.RowManager " + - "that you are using to submit the plan."), - "Unexpected message: " + ex.getMessage()); } @Test From 8dd765f6351181eb81e84a2a6359ff6a86955224 Mon Sep 17 00:00:00 2001 From: Rob Rudin Date: Wed, 24 Jan 2024 16:34:15 -0500 Subject: [PATCH 2/2] Revert "Merge pull request #1630 from marklogic/feature/4119-optic-update" This reverts commit e4a7eaf5801db28fb0695f84d2fd4bc72f53e71f. --- .../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, 99 insertions(+), 134 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 57ce16087..e9d60150b 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,7 +62,6 @@ public class RowManagerImpl private RowStructure rowStructureStyle = null; private Integer optimize; private String traceLabel; - private boolean update; public RowManagerImpl(RESTServices services) { super(); @@ -125,13 +124,7 @@ public void setOptimize(Integer value) { this.optimize = value; } - @Override - public RowManager withUpdate(boolean update) { - this.update = update; - return this; - } - - @Override + @Override public RawPlanDefinition newRawPlanDefinition(JSONWriteHandle handle) { return new RawPlanDefinitionImpl(handle); } @@ -183,11 +176,7 @@ public T resultDoc(Plan plan, T resultsHandle, T .withColumnTypes(getDatatypeStyle()) .withOutput(getRowStructureStyle()) .getRequestParameters(); - return services.postResource(requestLogger, determinePath(), transaction, params, astHandle, resultsHandle); - } - - private String determinePath() { - return this.update ? "rows/update" : "rows"; + return services.postResource(requestLogger, "rows", transaction, params, astHandle, resultsHandle); } @Override @@ -302,9 +291,8 @@ public T explain(Plan plan, T resultsHandle) { RequestParameters params = new RequestParameters(); params.add("output", "explain"); - return services.postResource(requestLogger, determinePath(), null, params, astHandle, resultsHandle); + return services.postResource(requestLogger, "rows", null, params, astHandle, resultsHandle); } - @Override public T explainAs(Plan plan, Class as) { ContentHandle handle = handleFor(as); @@ -421,12 +409,11 @@ 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, path, transaction, params, contentParams); + return services.postMultipartForm(requestLogger, "rows", transaction, params, contentParams); } - return services.postIteratedResource(requestLogger, path, transaction, params, astHandle); + return services.postIteratedResource(requestLogger, "rows", 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 c78aae23b..d48cdfe9d 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,18 +87,6 @@ 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 8654d6cb1..ca5d7eaa7 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().withUpdate(true); + rowManager = Common.client.newRowManager(); 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 17f5b62f2..2304af8dd 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().withUpdate(true).execute(op + Common.client.newRowManager().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().withUpdate(true); + rowManager = Common.client.newRowManager(); 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 36834dda4..5f135acf9 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,7 +1,6 @@ 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; @@ -13,99 +12,89 @@ import java.util.List; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; @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 - 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()); - } + // 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()); + } } 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 7534681ae..30131a22e 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,8 +32,6 @@ 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 61bb1f249..cefa52c30 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"); - // 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"); + 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"); } @Test @@ -1247,9 +1247,8 @@ public void testAggregates() throws IOException { recordRowSet.close(); } - @Test - public void testMapper() { + public void testMapper() throws IOException, XPathExpressionException { RowManager rowMgr = Common.client.newRowManager(); PlanBuilder p = rowMgr.newPlanBuilder(); @@ -1261,11 +1260,13 @@ public void testMapper() { .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 = @@ -1278,11 +1279,13 @@ public void testMapper() { 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++; } } @@ -1316,7 +1319,7 @@ public void testColumnInfo() { } @Test - public void testGenerateView() { + public void testGenerateView() throws IOException { RowManager rowMgr = Common.client.newRowManager(); PlanBuilder p = rowMgr.newPlanBuilder();