You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by mk...@apache.org on 2019/10/04 08:36:18 UTC

svn commit: r1867965 - in /jackrabbit/oak/trunk/oak-jcr/src: main/java/org/apache/jackrabbit/oak/jcr/session/ test/java/org/apache/jackrabbit/oak/jcr/session/

Author: mkataria
Date: Fri Oct  4 08:36:18 2019
New Revision: 1867965

URL: http://svn.apache.org/viewvc?rev=1867965&view=rev
Log:
OAK-8633: Add warn logs if we add/update a string property larger than 100KB (Node)

Added:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/OakJcrConstants.java
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/session/WarnLogStringPropertySizeTest.java
Modified:
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java?rev=1867965&r1=1867964&r2=1867965&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java Fri Oct  4 08:36:18 2019
@@ -127,6 +127,8 @@ public class NodeImpl<T extends NodeDele
      */
     private static final Logger LOG = LoggerFactory.getLogger(NodeImpl.class);
 
+    private final int logWarnStringSizeThreshold;
+
     @Nullable
     public static NodeImpl<? extends NodeDelegate> createNodeOrNull(
             @Nullable NodeDelegate delegate, @NotNull SessionContext context)
@@ -159,6 +161,9 @@ public class NodeImpl<T extends NodeDele
 
     public NodeImpl(T dlg, SessionContext sessionContext) {
         super(dlg, sessionContext);
+        logWarnStringSizeThreshold = Integer.getInteger(
+                OakJcrConstants.WARN_LOG_STRING_SIZE_THRESHOLD_KEY,
+                OakJcrConstants.DEFAULT_WARN_LOG_STRING_SIZE_THRESHOLD_VALUE);
     }
 
     //---------------------------------------------------------------< Item >---
@@ -1379,6 +1384,9 @@ public class NodeImpl<T extends NodeDele
         final String oakName = getOakPathOrThrow(checkNotNull(jcrName));
         final PropertyState state = createSingleState(
                 oakName, value, Type.fromTag(value.getType(), false));
+        if (value != null && value.getType() == PropertyType.STRING) {
+            logLargeStringProperties(jcrName, value.getString());
+        }
         return perform(new ItemWriteOperation<Property>("internalSetProperty") {
             @Override
             public void checkPreconditions() throws RepositoryException {
@@ -1403,6 +1411,12 @@ public class NodeImpl<T extends NodeDele
         });
     }
 
+    private void logLargeStringProperties(String propertyName, String value) throws RepositoryException {
+        if (value.length() > logWarnStringSizeThreshold) {
+            LOG.warn("String length: {} for property: {} at Node: {} is greater than configured value {}", value.length(), propertyName, this.getPath(), logWarnStringSizeThreshold);
+        }
+    }
+
     private Property internalSetProperty(
             final String jcrName, final Value[] values,
             final int type, final boolean exactTypeMatch)
@@ -1414,7 +1428,11 @@ public class NodeImpl<T extends NodeDele
         if (values.length > MV_PROPERTY_WARN_THRESHOLD) {
             LOG.warn("Large multi valued property [{}/{}] detected ({} values).",dlg.getPath(), jcrName, values.length);
         }
-
+        for (Value value : values) {
+            if (value != null && value.getType() == PropertyType.STRING) {
+                logLargeStringProperties(jcrName, value.getString());
+            }
+        }
         return perform(new ItemWriteOperation<Property>("internalSetProperty") {
             @Override
             public void checkPreconditions() throws RepositoryException {

Added: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/OakJcrConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/OakJcrConstants.java?rev=1867965&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/OakJcrConstants.java (added)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/OakJcrConstants.java Fri Oct  4 08:36:18 2019
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.jcr.session;
+
+public interface OakJcrConstants {
+    int DEFAULT_WARN_LOG_STRING_SIZE_THRESHOLD_VALUE = 102400;
+    String WARN_LOG_STRING_SIZE_THRESHOLD_KEY = "oak.repository.node.property.logWarnStringSizeThreshold";
+}

Added: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/session/WarnLogStringPropertySizeTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/session/WarnLogStringPropertySizeTest.java?rev=1867965&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/session/WarnLogStringPropertySizeTest.java (added)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/session/WarnLogStringPropertySizeTest.java Fri Oct  4 08:36:18 2019
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.jcr.session;
+
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import org.apache.jackrabbit.oak.commons.junit.TemporarySystemProperty;
+import org.apache.jackrabbit.oak.fixture.NodeStoreFixture;
+import org.apache.jackrabbit.oak.jcr.AbstractRepositoryTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+
+import javax.jcr.Node;
+import javax.jcr.Session;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * {@code WarnLogStringPropertySizeTest} checks if Warn log is bein added on adding
+ * large string properties
+ */
+public class WarnLogStringPropertySizeTest extends AbstractRepositoryTest {
+
+    @Rule
+    public TemporarySystemProperty temporarySystemProperty = new TemporarySystemProperty();
+
+    private final static String testStringPropertyKey = "testStringPropertyKey";
+    private final static String testLargeStringPropertyValue = "abcdefghijklmnopqrstuvwxyz";
+    private final static String testSmallStringPropertyValue = "abcd";
+    private final static String nodeImplLogger = NodeImpl.class.getName();
+    private final static String warnMessage = "String length: {} for property: {} at Node: {} is greater than configured value {}";
+    private static ListAppender<ILoggingEvent> listAppender = null;
+
+    public WarnLogStringPropertySizeTest(NodeStoreFixture fixture) {
+        super(fixture);
+    }
+
+    @Before
+    public void loggingAppenderStart() {
+        LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory();
+        listAppender = new ListAppender<>();
+        listAppender.start();
+        context.getLogger(nodeImplLogger).addAppender(listAppender);
+    }
+
+    @After
+    public void loggingAppenderStop() {
+        listAppender.stop();
+    }
+
+    @Test
+    public void noWarnLogOnAddingSmallStringProperties() throws Exception {
+        Session s = getAdminSession();
+        Node test = s.getRootNode().addNode("testSmall");
+        test.setProperty(testStringPropertyKey, testSmallStringPropertyValue);
+        assertFalse(isWarnMessagePresent(listAppender));
+    }
+
+    @Test
+    public void warnLogOnAddingLargeStringPropertiesWithCustomThreshold() throws Exception {
+        System.setProperty(OakJcrConstants.WARN_LOG_STRING_SIZE_THRESHOLD_KEY, "10");
+        Session s = getAdminSession();
+        Node test = s.getRootNode().addNode("testLarge");
+        test.setProperty(testStringPropertyKey, testLargeStringPropertyValue);
+        assertTrue(isWarnMessagePresent(listAppender));
+    }
+
+    private boolean isWarnMessagePresent(ListAppender<ILoggingEvent> listAppender) {
+        for (ILoggingEvent loggingEvent : listAppender.list) {
+            if (loggingEvent.getMessage().contains(warnMessage)) {
+                return true;
+            }
+        }
+        return false;
+    }
+}