JCR-2903: Session.importXml should close the input stream (as to JSR …

…283/JCR 2.0) git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/trunk@1077927 13f79535-47bb-0310-9956-ffa450edef68
uwej711 · Mar 4, 2011 · c63a588 · c63a588
1 parent e4916db
commit c63a588
Show file tree

Hide file tree

Showing 19 changed files with 286 additions and 39 deletions.
diff --git a/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/ValueFactoryImpl.java b/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/value/ValueFactoryImpl.java
@@ -16,6 +16,7 @@
  */
 package org.apache.jackrabbit.core.value;
 
+import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.core.data.DataIdentifier;
 import org.apache.jackrabbit.core.data.DataStore;
 import org.apache.jackrabbit.spi.commons.conversion.NamePathResolver;
@@ -79,6 +80,9 @@ public Binary createBinary(InputStream stream) throws RepositoryException {
             }
         } catch (IOException e) {
             throw new RepositoryException(e);
+        } finally {
+            // JCR-2903
+            IOUtils.closeQuietly(stream);
         }
     }
 
@@ -116,6 +120,9 @@ public Value createValue(InputStream value) {
             throw new RuntimeException(ex);
         } catch (RepositoryException ex) {
             throw new RuntimeException(ex);
+        } finally {
+            // JCR-2903
+            IOUtils.closeQuietly(value);
         }
     }
 

diff --git a/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/AbstractSession.java b/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/AbstractSession.java
@@ -351,6 +351,11 @@ public void importXML(
             } else {
                 throw new InvalidSerializedDataException("XML parse error", e);
             }
+        } finally {
+            // JCR-2903
+            if (in != null) {
+                try { in.close(); } catch (IOException ignore) {}
+            }
         }
     }
 

diff --git a/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/AbstractWorkspace.java b/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/commons/AbstractWorkspace.java
@@ -61,6 +61,11 @@ public void importXML(
             } else {
                 throw new InvalidSerializedDataException("XML parse error", e);
             }
+        } finally {
+            // JCR-2903
+            if (in != null) {
+                try { in.close(); } catch (IOException ignore) {}
+            }
         }
     }
 

diff --git a/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/value/AbstractValueFactory.java b/jackrabbit-jcr-commons/src/main/java/org/apache/jackrabbit/value/AbstractValueFactory.java
@@ -88,7 +88,12 @@ public Value createValue(double value) {
      * {@inheritDoc}
      */
     public Value createValue(InputStream value) {
-        return new BinaryValue(value);
+        try {
+            return new BinaryValue(value);
+        } finally {
+            // JCR-2903
+            try { value.close(); } catch (IOException ignore) {}
+        }
     }
 
     /**
@@ -171,6 +176,9 @@ public Binary createBinary(InputStream stream) throws RepositoryException {
             return new BinaryImpl(stream);
         } catch (IOException e) {
             throw new RepositoryException("failed to create Binary instance", e);
+        } finally {
+            // JCR-2903
+            try { stream.close(); } catch (IOException ignore) {}
         }
     }
 

diff --git a/jackrabbit-jcr-rmi/src/main/java/org/apache/jackrabbit/rmi/client/ClientSession.java b/jackrabbit-jcr-rmi/src/main/java/org/apache/jackrabbit/rmi/client/ClientSession.java
@@ -333,6 +333,9 @@ public void importXML(String path, InputStream xml, int mode)
             remote.importXML(path, buffer.toByteArray(), mode);
         } catch (RemoteException ex) {
             throw new RemoteRepositoryException(ex);
+        } finally {
+            // JCR-2903
+            try { xml.close(); } catch (IOException ignore) {}
         }
     }
 

diff --git a/jackrabbit-jcr-rmi/src/main/java/org/apache/jackrabbit/rmi/client/ClientWorkspace.java b/jackrabbit-jcr-rmi/src/main/java/org/apache/jackrabbit/rmi/client/ClientWorkspace.java
@@ -231,6 +231,9 @@ public void importXML(String path, InputStream xml, int uuidBehaviour)
             remote.importXML(path, buffer.toByteArray(), uuidBehaviour);
         } catch (RemoteException ex) {
             throw new RemoteRepositoryException(ex);
+        } finally {
+            // JCR-2903
+            try { xml.close(); } catch (IOException ignore) {}
         }
     }
 

diff --git a/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/AbstractImportXmlTest.java b/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/AbstractImportXmlTest.java
@@ -242,7 +242,7 @@ protected void importXML(String absPath, Document document,
                 session.save();
             }
         } finally {
-            bin.close();
+            try { bin.close(); } catch (IOException ignore) {}
         }
     }
 

diff --git a/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/SerializationTest.java b/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/SerializationTest.java
@@ -18,6 +18,7 @@
 
 import org.apache.jackrabbit.test.AbstractJCRTest;
 import org.apache.jackrabbit.test.NotExecutableException;
+import org.apache.jackrabbit.test.api.util.InputStreamWrapper;
 import org.xml.sax.ContentHandler;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
@@ -145,7 +146,7 @@ public void doTestVersioningExceptionFileParent(boolean useWorkspace, boolean us
         } catch (VersionException e) {
             // success
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -160,7 +161,7 @@ public void doTestVersioningExceptionFileChild(boolean useWorkspace, boolean use
         } catch (VersionException e) {
             // success
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -216,7 +217,7 @@ public void doTestLockException(boolean useWorkspace, boolean useHandler)
             } catch (LockException e) {
                 // success
             } finally {
-                in.close();
+                try { in.close(); } catch (IOException ignore) {}
             }
         } else {
             log.println("Locking not supported.");
@@ -353,7 +354,7 @@ public void testSessionImportXmlExceptions() throws RepositoryException, IOExcep
         } catch (PathNotFoundException e) {
             // success
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -372,7 +373,7 @@ public void testWorkspaceImportXmlExceptions() throws RepositoryException, IOExc
         } catch (PathNotFoundException e) {
             // success
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -416,7 +417,7 @@ public void doTestOverwriteException(boolean useWorkspace, boolean useHandler)
                 }
             }
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -477,7 +478,7 @@ public void doTestNodeTypeConstraintViolation(boolean useWorkspace, boolean useH
                 }
             }
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -510,7 +511,7 @@ public void testSessionImportXml() throws RepositoryException, IOException {
             session.importXML(treeComparator.targetFolder, in,
                     ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW);
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
 
         // after logout/login, no nodes are in the session
@@ -533,7 +534,7 @@ public void testSessionGetContentHandler() throws Exception {
             exportRepository(SAVEBINARY, RECURSE);
             doImportNoSave(treeComparator.targetFolder, in, CONTENTHANDLER);
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
 
         // after logout/login, no nodes are in the session
@@ -545,11 +546,33 @@ public void testSessionGetContentHandler() throws Exception {
         treeComparator.compare(treeComparator.CHECK_EMPTY);
     }
 
+//----------------< tests input stream handling contract >----------------------
+
+    /**
+     * Tests whether <code>Session.importXML</code> and <code>Workspace.importXML</code>
+     * obey the stream handling contract.
+     */
+    public void testStreamHandling() throws RepositoryException, IOException {
+        exportRepository(SAVEBINARY, RECURSE);
+
+        InputStreamWrapper in = new InputStreamWrapper(new FileInputStream(file));
+        session.importXML(treeComparator.targetFolder, in,
+                ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW);
+        assertTrue("Session.importXML(..., InputStream, ...) is expected to close the passed input stream", in.isClosed());
+        session.refresh(false);
+
+        in = new InputStreamWrapper(new FileInputStream(file));
+        workspace.importXML(treeComparator.targetFolder, in,
+                ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW);
+        assertTrue("Workspace.importXML(..., InputStream, ...) is expected to close the passed input stream", in.isClosed());
+    }
+
+
 //----------------< import test helper >--------------------------------------------------------
     /**
      * Helper method which imports the given FileInputStream using Workspace or Session
-     * and via the methods importXML respective getImportContentHandler. Teh target node of the
-     * import is specified with its absolut path.
+     * and via the methods importXML respective getImportContentHandler. The target node of the
+     * import is specified with its absolute path.
      *
      * @param absPath
      * @param in
@@ -729,7 +752,7 @@ public void importRepository(boolean useHandler, boolean workspace) throws Excep
         } catch (SAXException e) {
             fail("Error while parsing the imported repository: " + e);
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 

diff --git a/...main/java/org/apache/jackrabbit/test/api/SetPropertyConstraintViolationExceptionTest.java b/...main/java/org/apache/jackrabbit/test/api/SetPropertyConstraintViolationExceptionTest.java
@@ -270,9 +270,7 @@ public void testBinaryProperty()
         } catch (ConstraintViolationException e) {
             // success
         } finally {
-            try {
-                in.close();
-            } catch (IOException ignore) {}
+            try { in.close(); } catch (IOException ignore) {}
         }
 
         // test of signature setProperty(String name, Value value)

diff --git a/...it-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/SetPropertyInputStreamTest.java b/...it-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/SetPropertyInputStreamTest.java
@@ -18,10 +18,12 @@
 
 import org.apache.jackrabbit.test.AbstractJCRTest;
 import org.apache.jackrabbit.test.NotExecutableException;
+import org.apache.jackrabbit.test.api.util.InputStreamWrapper;
 
 import javax.jcr.Node;
 import javax.jcr.Property;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.ByteArrayInputStream;
 
@@ -74,8 +76,8 @@ public void testNewInputStreamPropertySession() throws Exception {
             assertTrue("Setting property with Node.setProperty(String, InputStream) and Session.save() not working",
                     compareInputStreams(is1, in));
         } finally {
-            in.close();
-        }
+            try { in.close(); } catch (IOException ignore) {}
+         }
     }
 
     /**
@@ -93,8 +95,8 @@ public void testModifyInputStreamPropertySession() throws Exception {
             assertTrue("Modifying property with Node.setProperty(String, InputStream) and Session.save() not working",
                     compareInputStreams(is2, in));
         } finally {
-            in.close();
-        }
+            try { in.close(); } catch (IOException ignore) {}
+         }
     }
 
     /**
@@ -110,7 +112,7 @@ public void testNewInputStreamPropertyParent() throws Exception {
             assertTrue("Setting property with Node.setProperty(String, InputStream) and parentNode.save() not working",
                     compareInputStreams(is1, in));
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -129,8 +131,8 @@ public void testModifyInputStreamPropertyParent() throws Exception {
             assertTrue("Modifying property with Node.setProperty(String, InputStream) and parentNode.save() not working",
                     compareInputStreams(is2, in));
         } finally {
-            in.close();
-        }
+            try { in.close(); } catch (IOException ignore) {}
+         }
     }
 
     /**
@@ -173,6 +175,16 @@ public void testRemoveInputStreamPropertyParent() throws Exception {
                 testNode.hasProperty(propertyName1));
     }
 
+    /**
+     * Tests whether the passed input stream is closed.
+     * @throws Exception
+     */
+    public void testInputStreamClosed() throws Exception {
+        InputStreamWrapper in = new InputStreamWrapper(new ByteArrayInputStream(bytes1));
+        testNode.setProperty(propertyName1, in);
+        assertTrue("Node.setProperty(..., InputStream) is expected to close the passed input stream", in.isClosed());
+    }
+
     /**
      * helper function: InputStream comparison
      */

diff --git a/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/SetValueBinaryTest.java b/jackrabbit-jcr-tests/src/main/java/org/apache/jackrabbit/test/api/SetValueBinaryTest.java
@@ -110,7 +110,7 @@ public void testBinarySession() throws RepositoryException, IOException {
         try {
             compareStream(data, in);
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -127,7 +127,7 @@ public void testBinarySessionJcr2() throws RepositoryException, IOException {
             try {
                 compareStream(data, in);
             } finally {
-                in.close();
+                try { in.close(); } catch (IOException ignore) {}
             }
         } finally {
             bin.dispose();
@@ -144,13 +144,13 @@ public void testBinaryParent() throws RepositoryException, IOException {
             property1.setValue(in);
             node.save();
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
         in = property1.getValue().getStream();
         try {
             compareStream(data, in);
         } finally {
-            in.close();
+            try { in.close(); } catch (IOException ignore) {}
         }
     }
 
@@ -168,7 +168,7 @@ public void testBinaryParentJcr2() throws RepositoryException, IOException {
             try {
                 compareStream(data, in);
             } finally {
-                in.close();
+                try { in.close(); } catch (IOException ignore) {}
             }
         } finally {
             bin.dispose();

diff --git a/...rc/main/java/org/apache/jackrabbit/test/api/SetValueConstraintViolationExceptionTest.java b/...rc/main/java/org/apache/jackrabbit/test/api/SetValueConstraintViolationExceptionTest.java
@@ -102,9 +102,7 @@ public void testBinaryProperty()
         } catch (ConstraintViolationException e) {
             // success
         } finally {
-            try {
-                in.close();
-            } catch (IOException ignore) {}
+            try { in.close(); } catch (IOException ignore) {}
         }
 
         // test of signature setValue(Value value)