You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by th...@apache.org on 2011/02/02 10:51:39 UTC

svn commit: r1066397 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/data/FileDataStore.java test/java/org/apache/jackrabbit/core/data/TestAll.java test/java/org/apache/jackrabbit/core/data/WriteWhileReadingTest.java

Author: thomasm
Date: Wed Feb  2 09:51:39 2011
New Revision: 1066397

URL: http://svn.apache.org/viewvc?rev=1066397&view=rev
Log:
JCR-2872 DataStore: changing the modified date fails if the file is open for reading (Windows only)

Added:
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/WriteWhileReadingTest.java
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java?rev=1066397&r1=1066396&r2=1066397&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/data/FileDataStore.java Wed Feb  2 09:51:39 2011
@@ -21,6 +21,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.RandomAccessFile;
 import java.lang.ref.WeakReference;
 import java.security.DigestOutputStream;
 import java.security.MessageDigest;
@@ -151,19 +152,8 @@ public class FileDataStore implements Da
             }
             if (minModifiedDate != 0) {
                 // only check when running garbage collection
-                long lastModified = file.lastModified();
-                if (lastModified == 0) {
-                    throw new DataStoreException("Failed to read record modified date: " + identifier);
-                }
-                if (lastModified < minModifiedDate) {
-                    // in most cases we can write to the file system, that's why we
-                    // don't call File.canWrite earlier (to save a system call)
-                    if (file.canWrite()) {
-                	        // The current GC approach depends on this call succeeding
-                        if (!file.setLastModified(System.currentTimeMillis() + ACCESS_TIME_RESOLUTION)) {
-                            throw new DataStoreException("Failed to update record modified date: " + identifier);
-                        }
-                    }
+                if (getLastModified(file) < minModifiedDate) {
+                    setLastModified(file, System.currentTimeMillis() + ACCESS_TIME_RESOLUTION);
                 }
             }
             usesIdentifier(identifier);
@@ -238,18 +228,9 @@ public class FileDataStore implements Da
                                 + " (media read only?)");
                     }
                 } else {
-                    long lastModified = file.lastModified();
-                    if (lastModified == 0) {
-                        throw new DataStoreException("Failed to read record modified date: " + identifier);
-                    }
                     long now = System.currentTimeMillis();
-                    if (lastModified < now + ACCESS_TIME_RESOLUTION) {
-                        // The current GC approach depends on this call succeeding (for writable files)
-                        if (!file.setLastModified(now + ACCESS_TIME_RESOLUTION)) {
-                            if (file.canWrite()) {
-                                throw new DataStoreException("Failed to update record modified date: " + identifier);
-                            }
-                        }
+                    if (getLastModified(file) < now + ACCESS_TIME_RESOLUTION) {
+                        setLastModified(file, now + ACCESS_TIME_RESOLUTION);
                     }
                 }
                 if (file.length() != length) {
@@ -319,11 +300,15 @@ public class FileDataStore implements Da
         int count = 0;
         if (file.isFile() && file.exists() && file.canWrite()) {
             synchronized (this) {
-                long lastModified = file.lastModified();
-                if (lastModified == 0) {
-                    // Don't delete the file, since the lastModified date is uncertain!
-                    log.warn("Failed to read modification date for " + file.getAbsolutePath());
-                } else if (lastModified < min) {
+                long lastModified;
+                try {
+                    lastModified = getLastModified(file);
+                } catch (DataStoreException e) {
+                    log.warn("Failed to read modification date; file not deleted", e);
+                    // don't delete the file, since the lastModified date is uncertain
+                    lastModified = min;
+                }
+                if (lastModified < min) {
                     DataIdentifier id = new DataIdentifier(file.getName());
                     if (!inUse.containsKey(id)) {
                         if (log.isInfoEnabled()) {
@@ -420,4 +405,49 @@ public class FileDataStore implements Da
         // nothing to do
     }
 
+    /**
+     * Get the last modified date of a file.
+     *
+     * @param file the file
+     * @return the last modified date
+     * @throws DataStoreException if reading fails
+     */
+    private static long getLastModified(File file) throws DataStoreException {
+        long lastModified = file.lastModified();
+        if (lastModified == 0) {
+            throw new DataStoreException("Failed to read record modified date: " + file.getAbsolutePath());
+        }
+        return lastModified;
+    }
+
+    /**
+     * Set the last modified date of a file, if the file is writable.
+     *
+     * @param file the file
+     * @param time the new last modified date
+     * @throws DataStoreException if the file is writable but modifying the date fails
+     */
+    private static void setLastModified(File file, long time) throws DataStoreException {
+        if (!file.setLastModified(time)) {
+            if (!file.canWrite()) {
+                // if we can't write to the file, so garbage collection will also not delete it
+                // (read only files or file systems)
+                return;
+            }
+            try {
+                // workaround for Windows: if the file is already open for reading
+                // (in this or another process), then setting the last modified date
+                // doesn't work - see also JCR-2872
+                RandomAccessFile r = new RandomAccessFile(file, "rw");
+                try {
+                    r.setLength(r.length());
+                } finally {
+                    r.close();
+                }
+            } catch (IOException e) {
+                throw new DataStoreException("An IO Exception occurred while trying to set the last modified date: " + file.getAbsolutePath(), e);
+            }
+        }
+    }
+
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java?rev=1066397&r1=1066396&r2=1066397&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/TestAll.java Wed Feb  2 09:51:39 2011
@@ -34,6 +34,7 @@ public class TestAll extends TestCase {
      */
     public static Test suite() {
         TestSuite suite = new ConcurrentTestSuite("Data tests");
+
         suite.addTestSuite(DataStoreAPITest.class);
         suite.addTestSuite(LazyFileInputStreamTest.class);
         suite.addTestSuite(OpenFilesTest.class);
@@ -47,6 +48,8 @@ public class TestAll extends TestCase {
         suite.addTestSuite(CopyValueTest.class);
         suite.addTestSuite(TestTwoGetStreams.class);
         suite.addTestSuite(TempFileInputStreamTest.class);
+        suite.addTestSuite(WriteWhileReadingTest.class);
+
         return suite;
     }
 

Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/WriteWhileReadingTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/WriteWhileReadingTest.java?rev=1066397&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/WriteWhileReadingTest.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/data/WriteWhileReadingTest.java Wed Feb  2 09:51:39 2011
@@ -0,0 +1,55 @@
+/*
+ * 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.core.data;
+
+import java.io.InputStream;
+import javax.jcr.Node;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
+import org.apache.jackrabbit.test.AbstractJCRTest;
+
+/**
+ * Test that adding an entry works even if the the entry already exists and is still open for reading.
+ *
+ * This is a problem for the FileDataStore on Windows, see JCR-2872.
+ */
+public class WriteWhileReadingTest extends AbstractJCRTest {
+
+    private static final int STREAM_LENGTH = 32 * 1024;
+
+    public void test() throws Exception {
+        Node root = superuser.getRootNode();
+        ValueFactory vf = superuser.getValueFactory();
+
+        // store a binary in the data store
+        root.setProperty("p1", vf.createBinary(new RandomInputStream(1, STREAM_LENGTH)));
+        superuser.save();
+
+        // read from the binary, but don't close the file
+        Value v1 = root.getProperty("p1").getValue();
+        InputStream in = v1.getBinary().getStream();
+        in.read();
+
+        // store the same content at a different place -
+        // this will change the last modified date of the file
+        root.setProperty("p2", vf.createBinary(new RandomInputStream(1, STREAM_LENGTH)));
+        superuser.save();
+
+        in.close();
+    }
+
+}