From 6c07c62a3e7d4addcc59148a132dbd3b2cb1804e Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Mon, 11 Nov 2013 14:30:00 -0500 Subject: [PATCH] OAK-392: Review ContentSession#createBlob Drop the separate BlobFactory and move the createBlob() method to Root. Document a more detailed contract for blob lifecycles. --- .../org/apache/jackrabbit/oak/api/BlobFactory.java | 41 ---------------------- .../java/org/apache/jackrabbit/oak/api/Root.java | 21 ++++++++--- .../apache/jackrabbit/oak/core/AbstractRoot.java | 15 ++------ .../apache/jackrabbit/oak/core/ImmutableRoot.java | 9 ++--- .../oak/plugins/value/ValueFactoryImpl.java | 18 ++++++---- .../external/DefaultSyncHandler.java | 2 +- .../jackrabbit/oak/AbstractSecurityTest.java | 2 +- .../jackrabbit/oak/core/ImmutableRootTest.java | 4 ++- .../AccessControlManagerImplTest.java | 2 +- .../security/user/query/UserQueryManagerTest.java | 2 +- .../authorization/accesscontrol/ACETest.java | 2 +- .../AbstractRestrictionProviderTest.java | 2 +- .../jackrabbit/oak/jcr/session/SessionContext.java | 2 +- 13 files changed, 46 insertions(+), 76 deletions(-) delete mode 100644 oak-core/src/main/java/org/apache/jackrabbit/oak/api/BlobFactory.java diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/api/BlobFactory.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/api/BlobFactory.java deleted file mode 100644 index 2632da7..0000000 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/api/BlobFactory.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * 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.api; - -import java.io.IOException; -import java.io.InputStream; - -/** - * BlobFactory... - * TODO review again if we really need/want to expose that in the OAK API - * TODO in particular exposing this interface (and Blob) requires additional thoughts on - * TODO - lifecycle of the factory, - * TODO - lifecycle of the Blob, - * TODO - access restrictions and how permissions are enforced on blob creation - * TODO - searchability, versioning and so forth - */ -public interface BlobFactory { - - /** - * Create a {@link Blob} from the given input stream. The input stream - * is closed after this method returns. - * @param inputStream The input stream for the {@code Blob} - * @return The {@code Blob} representing {@code inputStream} - * @throws java.io.IOException If an error occurs while reading from the stream - */ - Blob createBlob(InputStream inputStream) throws IOException; -} \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java index 5df93b5..f689bda 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/api/Root.java @@ -18,6 +18,9 @@ */ package org.apache.jackrabbit.oak.api; +import java.io.IOException; +import java.io.InputStream; + import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -153,13 +156,23 @@ public interface Root { QueryEngine getQueryEngine(); /** - * Returns the blob factory (TODO: review if that really belongs to the OAK-API. see also todos on BlobFactory) + * Reads (and closes) the given stream and returns a {@link Blob} that + * contains that binary. The returned blob will remain valid at least + * until the {@link ContentSession} of this root is closed, or longer + * if it has been committed as a part of a content update. + *

+ * The implementation may decide to persist the blob at any point + * during or between this method method call and a {@link #commit()} + * that includes the blob, but the blob will become visible to other + * sessions only after such a commit. * - * @return the blob factory. + * @param stream the stream for reading the binary + * @return the blob that was created + * @throws IOException if the stream could not be read */ @Nonnull - BlobFactory getBlobFactory(); - + Blob createBlob(@Nonnull InputStream stream) throws IOException; + /** * Get the {@code ContentSession} from which this root was acquired * diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java index f683508..0ca2aeb 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/AbstractRoot.java @@ -35,7 +35,6 @@ import javax.annotation.Nullable; import javax.security.auth.Subject; import org.apache.jackrabbit.oak.api.Blob; -import org.apache.jackrabbit.oak.api.BlobFactory; import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.ContentSession; import org.apache.jackrabbit.oak.api.QueryEngine; @@ -338,18 +337,10 @@ public abstract class AbstractRoot implements Root { }; } - @Nonnull - @Override - public BlobFactory getBlobFactory() { + @Override @Nonnull + public Blob createBlob(@Nonnull InputStream inputStream) throws IOException { checkLive(); - - return new BlobFactory() { - @Override - public Blob createBlob(InputStream inputStream) throws IOException { - checkLive(); - return store.createBlob(inputStream); - } - }; + return store.createBlob(checkNotNull(inputStream)); } //-----------------------------------------------------------< internal >--- diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java index 243b953..a30fc21 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableRoot.java @@ -21,9 +21,11 @@ package org.apache.jackrabbit.oak.core; import static com.google.common.base.Preconditions.checkArgument; import static org.apache.jackrabbit.oak.commons.PathUtils.elements; +import java.io.InputStream; + import javax.annotation.Nonnull; -import org.apache.jackrabbit.oak.api.BlobFactory; +import org.apache.jackrabbit.oak.api.Blob; import org.apache.jackrabbit.oak.api.ContentSession; import org.apache.jackrabbit.oak.api.QueryEngine; import org.apache.jackrabbit.oak.api.Root; @@ -117,9 +119,8 @@ public final class ImmutableRoot implements Root { }; } - @Nonnull - @Override - public BlobFactory getBlobFactory() { + @Override @Nonnull + public Blob createBlob(@Nonnull InputStream stream) { throw new UnsupportedOperationException(); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java index 7a22de6..aba0281 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueFactoryImpl.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.plugins.value; +import static com.google.common.base.Preconditions.checkNotNull; + import java.io.IOException; import java.io.InputStream; import java.math.BigDecimal; @@ -24,6 +26,7 @@ import java.net.URISyntaxException; import java.util.Calendar; import java.util.List; +import javax.annotation.Nonnull; import javax.jcr.Binary; import javax.jcr.Node; import javax.jcr.PropertyType; @@ -36,9 +39,9 @@ import javax.jcr.nodetype.NodeType; import com.google.common.collect.Lists; import org.apache.jackrabbit.oak.api.Blob; -import org.apache.jackrabbit.oak.api.BlobFactory; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.PropertyValue; +import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.namepath.JcrNameParser; import org.apache.jackrabbit.oak.namepath.JcrPathParser; import org.apache.jackrabbit.oak.namepath.NamePathMapper; @@ -59,19 +62,20 @@ import org.apache.jackrabbit.util.ISO8601; */ public class ValueFactoryImpl implements ValueFactory { - private final BlobFactory blobFactory; + private final Root root; private final NamePathMapper namePathMapper; /** * Creates a new instance of {@code ValueFactory}. * - * @param blobFactory The factory for creation of binary values + * @param root the root instance for creating binary values * @param namePathMapper The name/path mapping used for converting JCR names/paths to * the internal representation. */ - public ValueFactoryImpl(BlobFactory blobFactory, NamePathMapper namePathMapper) { - this.blobFactory = blobFactory; - this.namePathMapper = namePathMapper; + public ValueFactoryImpl( + @Nonnull Root root, @Nonnull NamePathMapper namePathMapper) { + this.root = checkNotNull(root); + this.namePathMapper = checkNotNull(namePathMapper); } /** @@ -274,7 +278,7 @@ public class ValueFactoryImpl implements ValueFactory { } private ValueImpl createBinaryValue(InputStream value) throws IOException { - Blob blob = blobFactory.createBlob(value); + Blob blob = root.createBlob(value); return new ValueImpl(BinaryPropertyState.binaryProperty("", blob), namePathMapper); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/DefaultSyncHandler.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/DefaultSyncHandler.java index 28722c3..a6c2e06 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/DefaultSyncHandler.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/DefaultSyncHandler.java @@ -68,7 +68,7 @@ public class DefaultSyncHandler implements SyncHandler { this.mode = mode; this.options = (options == null) ? ConfigurationParameters.EMPTY : options; - valueFactory = new ValueFactoryImpl(root.getBlobFactory(), NamePathMapper.DEFAULT); + valueFactory = new ValueFactoryImpl(root, NamePathMapper.DEFAULT); initialized = true; return true; } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java index 3468806..cf79854 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/AbstractSecurityTest.java @@ -184,7 +184,7 @@ public abstract class AbstractSecurityTest { } protected ValueFactory getValueFactory() { - return new ValueFactoryImpl(root.getBlobFactory(), getNamePathMapper()); + return new ValueFactoryImpl(root, getNamePathMapper()); } protected User getTestUser() throws Exception { diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/core/ImmutableRootTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/core/ImmutableRootTest.java index b6d9471..563d451 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/core/ImmutableRootTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/core/ImmutableRootTest.java @@ -19,6 +19,8 @@ package org.apache.jackrabbit.oak.core; import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; +import java.io.ByteArrayInputStream; + import org.apache.jackrabbit.oak.NodeStoreFixture; import org.apache.jackrabbit.oak.OakBaseTest; import org.apache.jackrabbit.oak.api.CommitFailedException; @@ -84,7 +86,7 @@ public class ImmutableRootTest extends OakBaseTest { } try { - root.getBlobFactory(); + root.createBlob(new ByteArrayInputStream(new byte[0])); fail(); } catch (UnsupportedOperationException e) { // success diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java index 36195f9..74ec690 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerImplTest.java @@ -113,7 +113,7 @@ public class AccessControlManagerImplTest extends AbstractAccessControlTest impl npMapper = new NamePathMapperImpl(nameMapper); acMgr = getAccessControlManager(npMapper); - valueFactory = new ValueFactoryImpl(root.getBlobFactory(), npMapper); + valueFactory = new ValueFactoryImpl(root, npMapper); NodeUtil rootNode = new NodeUtil(root.getTree("/"), npMapper); rootNode.addChild(testName, JcrConstants.NT_UNSTRUCTURED); diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java index 19e61e6..287fb75 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java @@ -53,7 +53,7 @@ public class UserQueryManagerTest extends AbstractSecurityTest { user = getTestUser(); queryMgr = new UserQueryManager(userMgr, namePathMapper, getUserConfiguration().getParameters(), root); - valueFactory = new ValueFactoryImpl(root.getBlobFactory(), namePathMapper); + valueFactory = new ValueFactoryImpl(root, namePathMapper); propertyName = "testProperty"; } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/accesscontrol/ACETest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/accesscontrol/ACETest.java index 0f7c7a2..e8a5398 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/accesscontrol/ACETest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/accesscontrol/ACETest.java @@ -77,7 +77,7 @@ public class ACETest extends AbstractAccessControlTest { return "TestPrincipal"; } }; - ValueFactory valueFactory = new ValueFactoryImpl(root.getBlobFactory(), namePathMapper); + ValueFactory valueFactory = new ValueFactoryImpl(root, namePathMapper); globValue = valueFactory.createValue("*"); nameValue = valueFactory.createValue("nt:file", PropertyType.NAME); nameValues = new Value[] { diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/AbstractRestrictionProviderTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/AbstractRestrictionProviderTest.java index e81959a..f377846 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/AbstractRestrictionProviderTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/AbstractRestrictionProviderTest.java @@ -63,7 +63,7 @@ public class AbstractRestrictionProviderTest extends AbstractSecurityTest implem public void before() throws Exception { super.before(); - valueFactory = new ValueFactoryImpl(root.getBlobFactory(), namePathMapper); + valueFactory = new ValueFactoryImpl(root, namePathMapper); globValue = valueFactory.createValue("*"); nameValue = valueFactory.createValue("nt:file", PropertyType.NAME); nameValues = new Value[] { diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java index 01db7ec..177d5a1 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java @@ -130,7 +130,7 @@ public class SessionContext implements NamePathMapper { this.namePathMapper = new NamePathMapperImpl( nameMapper, delegate.getIdManager()); this.valueFactory = new ValueFactoryImpl( - delegate.getRoot().getBlobFactory(), namePathMapper); + delegate.getRoot(), namePathMapper); } public final Map getAttributes() { -- 1.8.3.msysgit.0