mreutegg commented on code in PR #863:
URL: https://github.com/apache/jackrabbit-oak/pull/863#discussion_r1190916780
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBExceptionTest.java:
##########
@@ -132,8 +137,125 @@ public void idInExceptionMessage() {
}
}
+ @Test
+ public void createOrUpdate16MBDoc() {
+
+ UpdateOp updateOp = new UpdateOp("/foo", true);
+ updateOp = create16MBProp(updateOp);
+ exceptionMsg = "Document to upsert is larger than 16777216";
+ try {
+ store.createOrUpdate(Collection.NODES, updateOp);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ }
+ }
+
+ @Test
+ public void update16MBDoc() {
+
+ String docName = "/foo";
+ UpdateOp updateOp = new UpdateOp(docName, true);
+ updateOp = create1MBProp(updateOp);
+ store.createOrUpdate(Collection.NODES, updateOp);
+ updateOp = create16MBProp(updateOp);
+ exceptionMsg = "Resulting document after update is larger than
16777216";
+ try {
+ store.createOrUpdate(Collection.NODES, updateOp);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ assertThat(e.getMessage(), containsString(docName));
+ }
+ }
+
+ @Test
+ public void multiCreateOrUpdate16MBDoc() {
+
+ List<UpdateOp> updateOps = new ArrayList<>();
+
+ UpdateOp op = new UpdateOp("/test", true);
+ op = create1MBProp(op);
+
+ store.createOrUpdate(Collection.NODES, op);
+
+ UpdateOp op1 = new UpdateOp("/foo", true);
+ op1 = create16MBProp(op1);
+
+ // Updating both doc with 16MB
+ op = create16MBProp(op);
+ updateOps.add(op);
+ updateOps.add(op1);
+ exceptionMsg = "Resulting document after update is larger than
16777216";
+
+ try {
+ store.createOrUpdate(Collection.NODES, updateOps);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ }
+ }
+
+ @Test
+ public void create16MBDoc() {
+
+ List<UpdateOp> updateOps = new ArrayList<>();
+
+ UpdateOp op1 = new UpdateOp("/test", true);
+ op1 = create1MBProp(op1);
+
+ UpdateOp op2 = new UpdateOp("/foo", false);
+ op2 = create1MBProp(op2);
+ op2 = create16MBProp(op2);
+
+ updateOps.add(op1);
+ updateOps.add(op2);
+ assertFalse(store.create(Collection.NODES, updateOps));
+ }
+
+ @Test
+ public void findAndUpdate16MBDoc() throws Exception {
+ UpdateOp op = new UpdateOp("/foo", true);
+ op = create1MBProp(op);
+ store.createOrUpdate(Collection.NODES, op);
+ op = create16MBProp(op);
+ exceptionMsg = "Resulting document after update is larger than
16777216";
+ try {
+ store.findAndUpdate(Collection.NODES, op);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ }
Review Comment:
Please add an assert that the message contains the document id.
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java:
##########
@@ -157,6 +161,10 @@ public class MongoDocumentStore implements DocumentStore {
*/
public static final long DEFAULT_THROTTLING_TIME_MS =
Long.getLong("oak.mongo.throttlingTime", 20);
+ /**
+ * Document size of 16MB is a limit in Mongo
+ */
+ private static final long SIZE_LIMIT = 16777216;
Review Comment:
Can you please remove these lines. `SIZE_LIMIT` is not used.
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBExceptionTest.java:
##########
@@ -132,8 +137,125 @@ public void idInExceptionMessage() {
}
}
+ @Test
+ public void createOrUpdate16MBDoc() {
+
+ UpdateOp updateOp = new UpdateOp("/foo", true);
+ updateOp = create16MBProp(updateOp);
+ exceptionMsg = "Document to upsert is larger than 16777216";
+ try {
+ store.createOrUpdate(Collection.NODES, updateOp);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ }
+ }
+
+ @Test
+ public void update16MBDoc() {
+
+ String docName = "/foo";
+ UpdateOp updateOp = new UpdateOp(docName, true);
+ updateOp = create1MBProp(updateOp);
+ store.createOrUpdate(Collection.NODES, updateOp);
+ updateOp = create16MBProp(updateOp);
+ exceptionMsg = "Resulting document after update is larger than
16777216";
+ try {
+ store.createOrUpdate(Collection.NODES, updateOp);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ assertThat(e.getMessage(), containsString(docName));
+ }
+ }
+
+ @Test
+ public void multiCreateOrUpdate16MBDoc() {
+
+ List<UpdateOp> updateOps = new ArrayList<>();
+
+ UpdateOp op = new UpdateOp("/test", true);
+ op = create1MBProp(op);
+
+ store.createOrUpdate(Collection.NODES, op);
+
+ UpdateOp op1 = new UpdateOp("/foo", true);
+ op1 = create16MBProp(op1);
+
+ // Updating both doc with 16MB
+ op = create16MBProp(op);
+ updateOps.add(op);
+ updateOps.add(op1);
+ exceptionMsg = "Resulting document after update is larger than
16777216";
+
+ try {
+ store.createOrUpdate(Collection.NODES, updateOps);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ }
+ }
+
+ @Test
+ public void create16MBDoc() {
+
+ List<UpdateOp> updateOps = new ArrayList<>();
+
+ UpdateOp op1 = new UpdateOp("/test", true);
+ op1 = create1MBProp(op1);
+
+ UpdateOp op2 = new UpdateOp("/foo", false);
+ op2 = create1MBProp(op2);
+ op2 = create16MBProp(op2);
+
+ updateOps.add(op1);
+ updateOps.add(op2);
+ assertFalse(store.create(Collection.NODES, updateOps));
Review Comment:
Can you please add an assert that error messages are logged? There's an
example how to do that in `LoggingDocumentStoreWrapperTest`.
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBExceptionTest.java:
##########
@@ -136,32 +138,118 @@ public void idInExceptionMessage() {
}
@Test
- @Ignore
- public void add16MBDoc() throws Exception {
- //DocumentStore docStore = openDocumentStore();
+ public void createOrUpdate16MBDoc() {
- UpdateOp updateOp = new UpdateOp("/", false);
+ UpdateOp updateOp = new UpdateOp("/", true);
+ updateOp = create16MBProp(updateOp);
- // create a 1 MB property
- char[] chars = new char[1024 * 1024];
+ exceptionMsg = "Document to upsert is larger than 1677721";
+ setExceptionMsg();
+ try {
+ store.createOrUpdate(Collection.NODES, updateOp);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
+ }
+ }
- Arrays.fill(chars, '0');
- String content = new String(chars);
+ @Test
+ public void multiCreateOrUpdate16MBDoc() {
+
+ List<UpdateOp> updateOps = new ArrayList<UpdateOp>();
+ UpdateOp updateOp = new UpdateOp("/", true);
+ updateOp = create1MBProp(updateOp);
+ UpdateOp updateOp1 = new UpdateOp("/", true);
+ updateOp1 = create1MBProp(updateOp1);
+
+ updateOps.add(updateOp);
+ updateOps.add(updateOp1);
+
+ store.createOrUpdate(Collection.NODES, updateOps);
+ updateOp1 = create16MBProp(updateOp1);
+ updateOps.add(updateOp1);
- //create more than 16MB properties
- for (int i = 0; i < 17; i++) {
- updateOp.set("property"+ Integer.toString(i), content);
+ exceptionMsg = "Resulting document after update is larger than
16777216";
+ setExceptionMsg();
+
+ try {
+ store.createOrUpdate(Collection.NODES, updateOps);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
Review Comment:
Please add an assert that the message contains the document id.
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBExceptionTest.java:
##########
@@ -132,8 +137,125 @@ public void idInExceptionMessage() {
}
}
+ @Test
+ public void createOrUpdate16MBDoc() {
+
+ UpdateOp updateOp = new UpdateOp("/foo", true);
+ updateOp = create16MBProp(updateOp);
+ exceptionMsg = "Document to upsert is larger than 16777216";
+ try {
+ store.createOrUpdate(Collection.NODES, updateOp);
+ fail("DocumentStoreException expected");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString(exceptionMsg));
Review Comment:
Please add an assert that the message contains the document id.
--
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]
