stefan-egli commented on code in PR #1619: URL: https://github.com/apache/jackrabbit-oak/pull/1619#discussion_r1713646956
########## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateFactory.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.plugins.document; + +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.commons.Compression; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class DocumentPropertyStateFactory { + + private static final Logger LOG = LoggerFactory.getLogger(DocumentPropertyStateFactory.class); + + public static PropertyState createPropertyState(DocumentNodeStore store, String name, String value, Compression compression) { + + if (compression != null && !compression.equals(Compression.NONE) && value.length() > CompressedDocumentPropertyState.getCompressionThreshold()) { Review Comment: The test still doesn't fail consistently. I'm referring to `DocumentPropertyStateFactoryTest.createPropertyStateWithDefaultCompression`. That is one test at least that should fail with the current code. The reason it doesn't fail is that it depends on test execution order. Some other test method in that class is executed first (eg `createPropertyStateWithCompressionThresholdNotExceeded`) and that sets the threshold, so that `createPropertyStateWithDefaultCompression` then runs on false assumptions. Things that require fixing: * createPropertyStateWithDefaultCompression should check that it actually uses the default - i.e. it should have an assertEquals before even executing the actual property creation (and that would currently fail, depending on test execution order) * in addition to such asserts in each method, there must also be a `@After` and a `@Before` that resets the threshold * plus the actual code fix -- 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]
