From 9bac8270f4f3f9e640d4633c4987eacbbc1d4ede Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Tue, 31 Jan 2006 05:40:51 +0000 Subject: [PATCH] AR-383: Optimized overwrite behaviour for ContentWriter. Content will only be duplicated for the writes if explicitly requesting a random-access file without the truncate option git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@2254 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../smb/server/repo/ContentDiskDriver.java | 4 +- .../smb/server/repo/ContentNetworkFile.java | 74 +++++--- .../repo/content/AbstractContentAccessor.java | 75 +++++++-- .../content/AbstractContentReadWriteTest.java | 54 ++++-- .../repo/content/AbstractContentReader.java | 115 ++++++++++++- .../repo/content/AbstractContentWriter.java | 158 ++++++++++++++++-- .../repo/content/RandomAccessContent.java | 50 ------ .../content/filestore/FileContentReader.java | 64 ++++--- .../content/filestore/FileContentStore.java | 22 ++- .../content/filestore/FileContentWriter.java | 105 ++++-------- .../NoRandomAccessFileContentStoreTest.java | 56 +++++++ .../cmr/repository/ContentAccessor.java | 4 + .../service/cmr/repository/ContentReader.java | 14 ++ .../service/cmr/repository/ContentWriter.java | 18 ++ 14 files changed, 571 insertions(+), 242 deletions(-) delete mode 100644 source/java/org/alfresco/repo/content/RandomAccessContent.java create mode 100644 source/java/org/alfresco/repo/content/filestore/NoRandomAccessFileContentStoreTest.java diff --git a/source/java/org/alfresco/filesys/smb/server/repo/ContentDiskDriver.java b/source/java/org/alfresco/filesys/smb/server/repo/ContentDiskDriver.java index 320c5be061..5568e57b24 100644 --- a/source/java/org/alfresco/filesys/smb/server/repo/ContentDiskDriver.java +++ b/source/java/org/alfresco/filesys/smb/server/repo/ContentDiskDriver.java @@ -930,7 +930,7 @@ public class ContentDiskDriver implements DiskInterface, IOCtlInterface // Create the network file - NetworkFile netFile = ContentNetworkFile.createFile(nodeService, contentService, cifsHelper, nodeRef, params); + NetworkFile netFile = ContentNetworkFile.createFile(transactionService, nodeService, contentService, cifsHelper, nodeRef, params); // Create a file state for the open file @@ -1039,7 +1039,7 @@ public class ContentDiskDriver implements DiskInterface, IOCtlInterface NodeRef nodeRef = cifsHelper.createNode(deviceRootNodeRef, path, true); // create the network file - NetworkFile netFile = ContentNetworkFile.createFile(nodeService, contentService, cifsHelper, nodeRef, params); + NetworkFile netFile = ContentNetworkFile.createFile(transactionService, nodeService, contentService, cifsHelper, nodeRef, params); // Add a file state for the new file/folder diff --git a/source/java/org/alfresco/filesys/smb/server/repo/ContentNetworkFile.java b/source/java/org/alfresco/filesys/smb/server/repo/ContentNetworkFile.java index 4a9f089934..7473139a55 100644 --- a/source/java/org/alfresco/filesys/smb/server/repo/ContentNetworkFile.java +++ b/source/java/org/alfresco/filesys/smb/server/repo/ContentNetworkFile.java @@ -29,14 +29,17 @@ import org.alfresco.filesys.server.filesys.FileOpenParams; import org.alfresco.filesys.server.filesys.NetworkFile; import org.alfresco.i18n.I18NUtil; import org.alfresco.model.ContentModel; -import org.alfresco.repo.content.RandomAccessContent; import org.alfresco.repo.content.filestore.FileContentReader; +import org.alfresco.repo.transaction.TransactionUtil; +import org.alfresco.repo.transaction.TransactionUtil.TransactionWork; import org.alfresco.service.cmr.repository.ContentAccessor; import org.alfresco.service.cmr.repository.ContentData; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentService; +import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.transaction.TransactionService; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -52,6 +55,7 @@ public class ContentNetworkFile extends NetworkFile { private static final Log logger = LogFactory.getLog(ContentNetworkFile.class); + private TransactionService transactionService; private NodeService nodeService; private ContentService contentService; private NodeRef nodeRef; @@ -65,17 +69,12 @@ public class ContentNetworkFile extends NetworkFile // Flag to indicate if the file channel is writable private boolean writableChannel; - + /** * Helper method to create a {@link NetworkFile network file} given a node reference. - * - * @param serviceRegistry - * @param filePathCache used to speed up repeated searches - * @param nodeRef the node representing the file or directory - * @param params the parameters dictating the path and other attributes with which the file is being accessed - * @return Returns a new instance of the network file */ public static ContentNetworkFile createFile( + TransactionService transactionService, NodeService nodeService, ContentService contentService, CifsHelper cifsHelper, @@ -88,7 +87,7 @@ public class ContentNetworkFile extends NetworkFile // TODO: Check access writes and compare to write requirements // create the file - ContentNetworkFile netFile = new ContentNetworkFile(nodeService, contentService, nodeRef, path); + ContentNetworkFile netFile = new ContentNetworkFile(transactionService, nodeService, contentService, nodeRef, path); // set relevant parameters if (params.isReadOnlyAccess()) { @@ -151,10 +150,16 @@ public class ContentNetworkFile extends NetworkFile return netFile; } - private ContentNetworkFile(NodeService nodeService, ContentService contentService, NodeRef nodeRef, String name) + private ContentNetworkFile( + TransactionService transactionService, + NodeService nodeService, + ContentService contentService, + NodeRef nodeRef, + String name) { super(name); setFullName(name); + this.transactionService = transactionService; this.nodeService = nodeService; this.contentService = contentService; this.nodeRef = nodeRef; @@ -259,6 +264,10 @@ public class ContentNetworkFile extends NetworkFile // Indicate that we have a writable channel to the file writableChannel = true; + + // get the writable channel + // TODO: Decide on truncation strategy + channel = ((ContentWriter) content).getFileChannel(false); } else { @@ -272,17 +281,10 @@ public class ContentNetworkFile extends NetworkFile // Indicate that we only have a read-only channel to the data writableChannel = false; + + // get the read-only channel + channel = ((ContentReader) content).getFileChannel(); } - // wrap the channel accessor, if required - if (!(content instanceof RandomAccessContent)) - { - // TODO: create a temp, random access file and put a FileContentWriter on it - // barf for now - throw new AlfrescoRuntimeException("Can only use a store that supplies randomly accessible channel"); - } - RandomAccessContent randAccessContent = (RandomAccessContent) content; - // get the channel - we can only make this call once - channel = randAccessContent.getChannel(); } @Override @@ -296,20 +298,38 @@ public class ContentNetworkFile extends NetworkFile { return; } - else if (modified) // file was modified + + + if (modified) // file was modified { - // close it - channel.close(); - channel = null; - // write properties - ContentData contentData = content.getContentData(); - nodeService.setProperty(nodeRef, ContentModel.PROP_CONTENT, contentData); + // execute the close (with possible replication listeners, etc) and the + // node update in the same transaction. A transaction will be started + // by the nodeService anyway, so it is merely widening the transaction + // boundaries. + TransactionWork closeWork = new TransactionWork() + { + public Object doWork() throws Exception + { + // close the channel + // close it + channel.close(); + channel = null; + // update node properties + ContentData contentData = content.getContentData(); + nodeService.setProperty(nodeRef, ContentModel.PROP_CONTENT, contentData); + // done + return null; + } + }; + TransactionUtil.executeInUserTransaction(transactionService, closeWork); } else { // close it - it was not modified channel.close(); channel = null; + // no transaction used here. Any listener operations against this (now unused) content + // are irrelevant. } } diff --git a/source/java/org/alfresco/repo/content/AbstractContentAccessor.java b/source/java/org/alfresco/repo/content/AbstractContentAccessor.java index 1f7d1ef207..a22b3a3a75 100644 --- a/source/java/org/alfresco/repo/content/AbstractContentAccessor.java +++ b/source/java/org/alfresco/repo/content/AbstractContentAccessor.java @@ -26,10 +26,10 @@ import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; import java.util.List; -import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.repo.transaction.TransactionUtil; import org.alfresco.service.cmr.repository.ContentAccessor; import org.alfresco.service.cmr.repository.ContentData; +import org.alfresco.service.cmr.repository.ContentIOException; import org.alfresco.service.cmr.repository.ContentStreamListener; import org.alfresco.service.transaction.TransactionService; import org.apache.commons.logging.Log; @@ -139,17 +139,36 @@ public abstract class AbstractContentAccessor implements ContentAccessor this.encoding = encoding; } + /** + * Generate a callback instance of the {@link FileChannel FileChannel}. + * + * @param directChannel the delegate that to perform the actual operations + * @param listeners the listeners to call + * @return Returns a new channel that functions just like the original, except + * that it issues callbacks to the listeners + * @throws ContentIOException + */ + protected FileChannel getCallbackFileChannel( + FileChannel directChannel, + List listeners) + throws ContentIOException + { + FileChannel ret = new CallbackFileChannel(directChannel, listeners); + // done + return ret; + } + /** * Advise that listens for the completion of specific methods on the * {@link java.nio.channels.ByteChannel} interface. * * @author Derek Hulley */ - protected class ByteChannelCallbackAdvise implements AfterReturningAdvice + protected class ChannelCloseCallbackAdvise implements AfterReturningAdvice { private List listeners; - public ByteChannelCallbackAdvise(List listeners) + public ChannelCloseCallbackAdvise(List listeners) { this.listeners = listeners; } @@ -173,11 +192,6 @@ public abstract class AbstractContentAccessor implements ContentAccessor // nothing to do return; } - // ensure that we are in a transaction - if (transactionService == null) - { - throw new AlfrescoRuntimeException("A transaction service is required when there are listeners present"); - } TransactionUtil.TransactionWork work = new TransactionUtil.TransactionWork() { public Object doWork() @@ -190,11 +204,26 @@ public abstract class AbstractContentAccessor implements ContentAccessor return null; } }; - TransactionUtil.executeInUserTransaction(transactionService, work); + if (transactionService != null) + { + // just create a transaction + TransactionUtil.executeInUserTransaction(transactionService, work); + } + else + { + try + { + work.doWork(); + } + catch (Exception e) + { + throw new ContentIOException("Failed to executed channel close callbacks", e); + } + } // done if (logger.isDebugEnabled()) { - logger.debug("Content listeners called: close"); + logger.debug("" + listeners.size() + " content listeners called: close"); } } } @@ -202,6 +231,10 @@ public abstract class AbstractContentAccessor implements ContentAccessor /** * Wraps a FileChannel to provide callbacks to listeners when the * channel is {@link java.nio.channels.Channel#close() closed}. + *

+ * This class is unfortunately necessary as the {@link FileChannel} doesn't have + * an single interface defining its methods, making it difficult to put an + * advice around the methods that require overriding. * * @author Derek Hulley */ @@ -253,7 +286,6 @@ public abstract class AbstractContentAccessor implements ContentAccessor // nothing to do return; } - // create the work to update the listeners TransactionUtil.TransactionWork work = new TransactionUtil.TransactionWork() { public Object doWork() @@ -266,14 +298,29 @@ public abstract class AbstractContentAccessor implements ContentAccessor return null; } }; - TransactionUtil.executeInUserTransaction(transactionService, work); + if (transactionService != null) + { + // just create a transaction + TransactionUtil.executeInUserTransaction(transactionService, work); + } + else + { + try + { + work.doWork(); + } + catch (Exception e) + { + throw new ContentIOException("Failed to executed channel close callbacks", e); + } + } // done if (logger.isDebugEnabled()) { - logger.debug("Content listeners called: close"); + logger.debug("" + listeners.size() + " content listeners called: close"); } } - + @Override public void force(boolean metaData) throws IOException { diff --git a/source/java/org/alfresco/repo/content/AbstractContentReadWriteTest.java b/source/java/org/alfresco/repo/content/AbstractContentReadWriteTest.java index c191d40b28..ecb5f24704 100644 --- a/source/java/org/alfresco/repo/content/AbstractContentReadWriteTest.java +++ b/source/java/org/alfresco/repo/content/AbstractContentReadWriteTest.java @@ -28,6 +28,8 @@ import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; import java.util.Set; +import javax.transaction.UserTransaction; + import junit.framework.TestCase; import org.alfresco.repo.transaction.DummyTransactionService; @@ -35,6 +37,9 @@ import org.alfresco.service.cmr.repository.ContentIOException; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentStreamListener; import org.alfresco.service.cmr.repository.ContentWriter; +import org.alfresco.service.transaction.TransactionService; +import org.alfresco.util.ApplicationContextHelper; +import org.springframework.context.ApplicationContext; /** * Abstract base class that provides a set of tests for implementations @@ -47,7 +52,11 @@ import org.alfresco.service.cmr.repository.ContentWriter; */ public abstract class AbstractContentReadWriteTest extends TestCase { + private static final ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); + + protected TransactionService transactionService; private String contentUrl; + private UserTransaction txn; public AbstractContentReadWriteTest() { @@ -58,6 +67,14 @@ public abstract class AbstractContentReadWriteTest extends TestCase public void setUp() throws Exception { contentUrl = AbstractContentStore.createNewUrl(); + transactionService = (TransactionService) ctx.getBean("TransactionService"); + txn = transactionService.getUserTransaction(); + txn.begin(); + } + + public void tearDown() throws Exception + { + txn.rollback(); } /** @@ -539,16 +556,8 @@ public abstract class AbstractContentReadWriteTest extends TestCase public void testRandomAccessWrite() throws Exception { ContentWriter writer = getWriter(); - if (!(writer instanceof RandomAccessContent)) - { - // not much to do here - return; - } - RandomAccessContent randomWriter = (RandomAccessContent) writer; - // check that we are allowed to write - assertTrue("Expected random access writing", randomWriter.canWrite()); - FileChannel fileChannel = randomWriter.getChannel(); + FileChannel fileChannel = writer.getFileChannel(true); assertNotNull("No channel given", fileChannel); // check that no other content access is allowed @@ -584,6 +593,22 @@ public abstract class AbstractContentReadWriteTest extends TestCase { assertEquals("Content doesn't match", content[i], buffer.get(i)); } + + // get a new writer from the store, using the existing content and perform a truncation check + ContentWriter writerTruncate = getStore().getWriter(writer.getReader(), AbstractContentStore.createNewUrl()); + assertEquals("Content size incorrect", 0, writerTruncate.getSize()); + // get the channel with truncation + FileChannel fcTruncate = writerTruncate.getFileChannel(true); + fcTruncate.close(); + assertEquals("Content not truncated", 0, writerTruncate.getSize()); + + // get a new writer from the store, using the existing content and perform a non-truncation check + ContentWriter writerNoTruncate = getStore().getWriter(writer.getReader(), AbstractContentStore.createNewUrl()); + assertEquals("Content size incorrect", 0, writerNoTruncate.getSize()); + // get the channel without truncation + FileChannel fcNoTruncate = writerNoTruncate.getFileChannel(false); + fcNoTruncate.close(); + assertEquals("Content was truncated", writer.getSize(), writerNoTruncate.getSize()); } /** @@ -599,16 +624,8 @@ public abstract class AbstractContentReadWriteTest extends TestCase byte[] bytes = content.getBytes(); writer.putContent(content); ContentReader reader = writer.getReader(); - if (!(reader instanceof RandomAccessContent)) - { - // not much to do here - return; - } - RandomAccessContent randomReader = (RandomAccessContent) reader; - // check that we are NOT allowed to write - assertFalse("Expected read-only random access", randomReader.canWrite()); - FileChannel fileChannel = randomReader.getChannel(); + FileChannel fileChannel = reader.getFileChannel(); assertNotNull("No channel given", fileChannel); // check that no other content access is allowed @@ -631,5 +648,6 @@ public abstract class AbstractContentReadWriteTest extends TestCase buffer.get(bytes); String checkContent = new String(bytes); assertEquals("Content read failure", content, checkContent); + fileChannel.close(); } } diff --git a/source/java/org/alfresco/repo/content/AbstractContentReader.java b/source/java/org/alfresco/repo/content/AbstractContentReader.java index 9cdbdd733f..e0c8cd2e06 100644 --- a/source/java/org/alfresco/repo/content/AbstractContentReader.java +++ b/source/java/org/alfresco/repo/content/AbstractContentReader.java @@ -26,15 +26,18 @@ import java.io.InputStreamReader; import java.io.OutputStream; import java.io.Reader; import java.nio.channels.Channels; +import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; import java.util.ArrayList; import java.util.List; import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.content.filestore.FileContentWriter; import org.alfresco.service.cmr.repository.ContentAccessor; import org.alfresco.service.cmr.repository.ContentIOException; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentStreamListener; +import org.alfresco.util.TempFileProvider; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.aop.framework.ProxyFactory; @@ -152,24 +155,38 @@ public abstract class AbstractContentReader extends AbstractContentAccessor impl protected abstract ReadableByteChannel getDirectReadableChannel() throws ContentIOException; /** - * Optionally override to supply an alternate callback channel. + * Create a channel that performs callbacks to the given listeners. * * @param directChannel the result of {@link #getDirectReadableChannel()} * @param listeners the listeners to call * @return Returns a channel * @throws ContentIOException */ - protected ReadableByteChannel getCallbackReadableChannel( + private ReadableByteChannel getCallbackReadableChannel( ReadableByteChannel directChannel, List listeners) throws ContentIOException { - // introduce an advistor to handle the callbacks to the listeners - ByteChannelCallbackAdvise advise = new ByteChannelCallbackAdvise(listeners); - ProxyFactory proxyFactory = new ProxyFactory(directChannel); - proxyFactory.addAdvice(advise); - ReadableByteChannel callbackChannel = (ReadableByteChannel) proxyFactory.getProxy(); + ReadableByteChannel callbackChannel = null; + if (directChannel instanceof FileChannel) + { + callbackChannel = getCallbackFileChannel((FileChannel) directChannel, listeners); + } + else + { + // introduce an advistor to handle the callbacks to the listeners + ChannelCloseCallbackAdvise advise = new ChannelCloseCallbackAdvise(listeners); + ProxyFactory proxyFactory = new ProxyFactory(directChannel); + proxyFactory.addAdvice(advise); + callbackChannel = (ReadableByteChannel) proxyFactory.getProxy(); + } // done + if (logger.isDebugEnabled()) + { + logger.debug("Created callback byte channel: \n" + + " original: " + directChannel + "\n" + + " new: " + callbackChannel); + } return callbackChannel; } @@ -195,6 +212,90 @@ public abstract class AbstractContentReader extends AbstractContentAccessor impl return channel; } + + /** + * @inheritDoc + */ + public FileChannel getFileChannel() throws ContentIOException + { + /* + * Where the underlying support is not present for this method, a temporary + * file will be used as a substitute. When the write is complete, the + * results are copied directly to the underlying channel. + */ + + // get the underlying implementation's best readable channel + channel = getReadableChannel(); + // now use this channel if it can provide the random access, otherwise spoof it + FileChannel clientFileChannel = null; + if (channel instanceof FileChannel) + { + // all the support is provided by the underlying implementation + clientFileChannel = (FileChannel) channel; + // debug + if (logger.isDebugEnabled()) + { + logger.debug("Content reader provided direct support for FileChannel: \n" + + " reader: " + this); + } + } + else + { + // No random access support is provided by the implementation. + // Spoof it by providing a 2-stage read from a temp file + File tempFile = TempFileProvider.createTempFile("random_read_spoof_", ".bin"); + FileContentWriter spoofWriter = new FileContentWriter(tempFile); + // pull the content in from the underlying channel + FileChannel spoofWriterChannel = spoofWriter.getFileChannel(false); + try + { + long spoofFileSize = this.getSize(); + spoofWriterChannel.transferFrom(channel, 0, spoofFileSize); + } + catch (IOException e) + { + throw new ContentIOException("Failed to copy from permanent channel to spoofed temporary channel: \n" + + " reader: " + this + "\n" + + " temp: " + spoofWriter, + e); + } + finally + { + try { spoofWriterChannel.close(); } catch (IOException e) {} + } + // get a reader onto the spoofed content + final ContentReader spoofReader = spoofWriter.getReader(); + // Attach a listener + // - ensure that the close call gets propogated to the underlying channel + ContentStreamListener spoofListener = new ContentStreamListener() + { + public void contentStreamClosed() throws ContentIOException + { + try + { + channel.close(); + } + catch (IOException e) + { + throw new ContentIOException("Failed to close underlying channel", e); + } + } + }; + spoofReader.addListener(spoofListener); + // we now have the spoofed up channel that the client can work with + clientFileChannel = spoofReader.getFileChannel(); + // debug + if (logger.isDebugEnabled()) + { + logger.debug("Content writer provided indirect support for FileChannel: \n" + + " writer: " + this + "\n" + + " temp writer: " + spoofWriter); + } + } + // the file is now available for random access + return clientFileChannel; + } + /** * @see Channels#newInputStream(java.nio.channels.ReadableByteChannel) */ diff --git a/source/java/org/alfresco/repo/content/AbstractContentWriter.java b/source/java/org/alfresco/repo/content/AbstractContentWriter.java index 9831618fef..06701d9c00 100644 --- a/source/java/org/alfresco/repo/content/AbstractContentWriter.java +++ b/source/java/org/alfresco/repo/content/AbstractContentWriter.java @@ -24,16 +24,20 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.nio.channels.Channels; +import java.nio.channels.FileChannel; +import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; import java.util.ArrayList; import java.util.List; import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.content.filestore.FileContentWriter; import org.alfresco.service.cmr.repository.ContentAccessor; import org.alfresco.service.cmr.repository.ContentIOException; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentStreamListener; import org.alfresco.service.cmr.repository.ContentWriter; +import org.alfresco.util.TempFileProvider; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.aop.framework.ProxyFactory; @@ -151,7 +155,7 @@ public abstract class AbstractContentWriter extends AbstractContentAccessor impl return false; } } - + /** * Provides low-level access to write content to the repository. *

@@ -165,24 +169,38 @@ public abstract class AbstractContentWriter extends AbstractContentAccessor impl protected abstract WritableByteChannel getDirectWritableChannel() throws ContentIOException; /** - * Optionally override to supply an alternate callback channel. - * + * Create a channel that performs callbacks to the given listeners. + * * @param directChannel the result of {@link #getDirectWritableChannel()} * @param listeners the listeners to call - * @return Returns a callback channel + * @return Returns a channel that executes callbacks * @throws ContentIOException */ - protected WritableByteChannel getCallbackWritableChannel( + private WritableByteChannel getCallbackWritableChannel( WritableByteChannel directChannel, List listeners) throws ContentIOException { - // proxy to add an advise - ByteChannelCallbackAdvise advise = new ByteChannelCallbackAdvise(listeners); - ProxyFactory proxyFactory = new ProxyFactory(directChannel); - proxyFactory.addAdvice(advise); - WritableByteChannel callbackChannel = (WritableByteChannel) proxyFactory.getProxy(); + WritableByteChannel callbackChannel = null; + if (directChannel instanceof FileChannel) + { + callbackChannel = getCallbackFileChannel((FileChannel) directChannel, listeners); + } + else + { + // introduce an advistor to handle the callbacks to the listeners + ChannelCloseCallbackAdvise advise = new ChannelCloseCallbackAdvise(listeners); + ProxyFactory proxyFactory = new ProxyFactory(directChannel); + proxyFactory.addAdvice(advise); + callbackChannel = (WritableByteChannel) proxyFactory.getProxy(); + } // done + if (logger.isDebugEnabled()) + { + logger.debug("Created callback byte channel: \n" + + " original: " + directChannel + "\n" + + " new: " + callbackChannel); + } return callbackChannel; } @@ -210,6 +228,126 @@ public abstract class AbstractContentWriter extends AbstractContentAccessor impl return channel; } + /** + * @inheritDoc + */ + public FileChannel getFileChannel(boolean truncate) throws ContentIOException + { + /* + * By calling this method, clients indicate that they wish to make random + * changes to the file. It is possible that the client might only want + * to update a tiny proportion of the file (truncate == false) or + * start afresh (truncate == true). + * + * Where the underlying support is not present for this method, a temporary + * file will be used as a substitute. When the write is complete, the + * results are copied directly to the underlying channel. + */ + + // get the underlying implementation's best writable channel + channel = getWritableChannel(); + // now use this channel if it can provide the random access, otherwise spoof it + FileChannel clientFileChannel = null; + if (channel instanceof FileChannel) + { + // all the support is provided by the underlying implementation + clientFileChannel = (FileChannel) channel; + // copy over the existing content, if required + if (!truncate && existingContentReader != null) + { + ReadableByteChannel existingContentChannel = existingContentReader.getReadableChannel(); + long existingContentLength = existingContentReader.getSize(); + // copy the existing content + try + { + clientFileChannel.transferFrom(existingContentChannel, 0, existingContentLength); + // copy complete + if (logger.isDebugEnabled()) + { + logger.debug("Copied content for random access: \n" + + " writer: " + this + "\n" + + " existing: " + existingContentReader); + } + } + catch (IOException e) + { + throw new ContentIOException("Failed to copy from existing content to enable random access: \n" + + " writer: " + this + "\n" + + " existing: " + existingContentReader, + e); + } + finally + { + try { existingContentChannel.close(); } catch (IOException e) {} + } + } + // debug + if (logger.isDebugEnabled()) + { + logger.debug("Content writer provided direct support for FileChannel: \n" + + " writer: " + this); + } + } + else + { + // No random access support is provided by the implementation. + // Spoof it by providing a 2-stage write via a temp file + File tempFile = TempFileProvider.createTempFile("random_write_spoof_", ".bin"); + final FileContentWriter spoofWriter = new FileContentWriter( + tempFile, // the file to write to + getExistingContentReader()); // this ensures that the existing content is pulled in + // Attach a listener + // - to ensure that the content gets loaded from the temp file once writing has finished + // - to ensure that the close call gets passed on to the underlying channel + ContentStreamListener spoofListener = new ContentStreamListener() + { + public void contentStreamClosed() throws ContentIOException + { + // the spoofed temp channel has been closed, so get a new reader for it + ContentReader spoofReader = spoofWriter.getReader(); + FileChannel spoofChannel = spoofReader.getFileChannel(); + // upload all the temp content to the real underlying channel + try + { + long spoofFileSize = spoofChannel.size(); + spoofChannel.transferTo(0, spoofFileSize, channel); + } + catch (IOException e) + { + throw new ContentIOException("Failed to copy from spoofed temporary channel to permanent channel: \n" + + " writer: " + this + "\n" + + " temp: " + spoofReader, + e); + } + finally + { + try { spoofChannel.close(); } catch (Throwable e) {} + try + { + channel.close(); + } + catch (IOException e) + { + throw new ContentIOException("Failed to close underlying channel", e); + } + } + } + }; + spoofWriter.addListener(spoofListener); + // we now have the spoofed up channel that the client can work with + clientFileChannel = spoofWriter.getFileChannel(truncate); + // debug + if (logger.isDebugEnabled()) + { + logger.debug("Content writer provided indirect support for FileChannel: \n" + + " writer: " + this + "\n" + + " temp writer: " + spoofWriter); + } + } + // the file is now available for random access + return clientFileChannel; + } + /** * @see Channels#newOutputStream(java.nio.channels.WritableByteChannel) */ diff --git a/source/java/org/alfresco/repo/content/RandomAccessContent.java b/source/java/org/alfresco/repo/content/RandomAccessContent.java deleted file mode 100644 index 9a1f39a5de..0000000000 --- a/source/java/org/alfresco/repo/content/RandomAccessContent.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright (C) 2005 Alfresco, Inc. - * - * Licensed under the Mozilla Public License version 1.1 - * with a permitted attribution clause. You may obtain a - * copy of the License at - * - * http://www.alfresco.org/legal/license.txt - * - * 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.alfresco.repo.content; - -import java.nio.channels.FileChannel; - -import org.alfresco.service.cmr.repository.ContentIOException; - -/** - * Supplementary interface for content readers and writers that allow random-access to - * the underlying content. - *

- * The use of this interface by a client may preclude the use of any other - * access to the underlying content - this depends on the underlying implementation. - * - * @author Derek Hulley - */ -public interface RandomAccessContent -{ - /** - * @return Returns true if the content can be written to - */ - public boolean canWrite(); - - /** - * Get a channel to access the content. The channel's behaviour is similar to that - * when a FileChannel is retrieved using {@link java.io.RandomAccessFile#getChannel()}. - * - * @return Returns a channel to access the content - * @throws ContentIOException - * - * @see #canWrite() - * @see java.io.RandomAccessFile#getChannel() - */ - public FileChannel getChannel() throws ContentIOException; -} diff --git a/source/java/org/alfresco/repo/content/filestore/FileContentReader.java b/source/java/org/alfresco/repo/content/filestore/FileContentReader.java index 333eea0fee..02d60f4242 100644 --- a/source/java/org/alfresco/repo/content/filestore/FileContentReader.java +++ b/source/java/org/alfresco/repo/content/filestore/FileContentReader.java @@ -17,20 +17,18 @@ package org.alfresco.repo.content.filestore; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.io.RandomAccessFile; -import java.nio.channels.FileChannel; +import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; import java.text.MessageFormat; -import java.util.List; -import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.repo.content.AbstractContentReader; import org.alfresco.repo.content.MimetypeMap; -import org.alfresco.repo.content.RandomAccessContent; import org.alfresco.service.cmr.repository.ContentIOException; import org.alfresco.service.cmr.repository.ContentReader; -import org.alfresco.service.cmr.repository.ContentStreamListener; import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.util.TempFileProvider; import org.apache.commons.logging.Log; @@ -43,11 +41,12 @@ import org.apache.commons.logging.LogFactory; * * @author Derek Hulley */ -public class FileContentReader extends AbstractContentReader implements RandomAccessContent +public class FileContentReader extends AbstractContentReader { private static final Log logger = LogFactory.getLog(FileContentReader.class); private File file; + private boolean allowRandomAccess; /** * Checks the existing reader provided and replaces it with a reader onto some @@ -118,6 +117,12 @@ public class FileContentReader extends AbstractContentReader implements RandomAc super(url); this.file = file; + allowRandomAccess = true; + } + + /* package */ void setAllowRandomAccess(boolean allow) + { + this.allowRandomAccess = allow; } /** @@ -170,7 +175,9 @@ public class FileContentReader extends AbstractContentReader implements RandomAc @Override protected ContentReader createReader() throws ContentIOException { - return new FileContentReader(this.file, getContentUrl()); + FileContentReader reader = new FileContentReader(this.file, getContentUrl()); + reader.setAllowRandomAccess(this.allowRandomAccess); + return reader; } @Override @@ -184,12 +191,23 @@ public class FileContentReader extends AbstractContentReader implements RandomAc throw new IOException("File does not exist"); } // create the channel - RandomAccessFile randomAccessFile = new RandomAccessFile(file, "r"); // won't create it - FileChannel channel = randomAccessFile.getChannel(); + ReadableByteChannel channel = null; + if (allowRandomAccess) + { + RandomAccessFile randomAccessFile = new RandomAccessFile(file, "r"); // won't create it + channel = randomAccessFile.getChannel(); + } + else + { + InputStream is = new FileInputStream(file); + channel = Channels.newChannel(is); + } // done if (logger.isDebugEnabled()) { - logger.debug("Opened channel to file: " + file); + logger.debug("Opened write channel to file: \n" + + " file: " + file + "\n" + + " random-access: " + allowRandomAccess); } return channel; } @@ -199,25 +217,6 @@ public class FileContentReader extends AbstractContentReader implements RandomAc } } - /** - * @param directChannel a file channel - */ - @Override - protected ReadableByteChannel getCallbackReadableChannel( - ReadableByteChannel directChannel, - List listeners) throws ContentIOException - { - if (!(directChannel instanceof FileChannel)) - { - throw new AlfrescoRuntimeException("Expected read channel to be a file channel"); - } - FileChannel fileChannel = (FileChannel) directChannel; - // wrap it - FileChannel callbackChannel = new CallbackFileChannel(fileChannel, listeners); - // done - return callbackChannel; - } - /** * @return Returns false as this is a reader */ @@ -225,11 +224,4 @@ public class FileContentReader extends AbstractContentReader implements RandomAc { return false; // we only allow reading } - - public FileChannel getChannel() throws ContentIOException - { - // go through the super classes to ensure that all concurrency conditions - // and listeners are satisfied - return (FileChannel) super.getReadableChannel(); - } } diff --git a/source/java/org/alfresco/repo/content/filestore/FileContentStore.java b/source/java/org/alfresco/repo/content/filestore/FileContentStore.java index a10c2402de..dcac15dd11 100644 --- a/source/java/org/alfresco/repo/content/filestore/FileContentStore.java +++ b/source/java/org/alfresco/repo/content/filestore/FileContentStore.java @@ -43,6 +43,7 @@ public class FileContentStore extends AbstractContentStore private File rootDirectory; private String rootAbsolutePath; + private boolean allowRandomAccess; /** * @param rootDirectory the root under which files will be stored. The @@ -60,6 +61,7 @@ public class FileContentStore extends AbstractContentStore } rootDirectory = rootDirectory.getAbsoluteFile(); rootAbsolutePath = rootDirectory.getAbsolutePath(); + allowRandomAccess = true; } public String toString() @@ -70,7 +72,23 @@ public class FileContentStore extends AbstractContentStore .append("]"); return sb.toString(); } - + + /** + * Stores may optionally produce readers and writers that support random access. + * Switch this off for this store by setting this to false. + *

+ * This switch is primarily used during testing to ensure that the system has the + * ability to spoof random access in cases where the store is unable to produce + * readers and writers that allow random access. Typically, stream-based access + * would be an example. + * + * @param allowRandomAccess true to allow random access, false to have it faked + */ + public void setAllowRandomAccess(boolean allowRandomAccess) + { + this.allowRandomAccess = allowRandomAccess; + } + /** * Generates a new URL and file appropriate to it. * @@ -193,6 +211,7 @@ public class FileContentStore extends AbstractContentStore { File file = makeFile(contentUrl); FileContentReader reader = new FileContentReader(file, contentUrl); + reader.setAllowRandomAccess(allowRandomAccess); // done if (logger.isDebugEnabled()) @@ -233,6 +252,7 @@ public class FileContentStore extends AbstractContentStore } // create the writer FileContentWriter writer = new FileContentWriter(file, contentUrl, existingContentReader); + writer.setAllowRandomAccess(allowRandomAccess); // done if (logger.isDebugEnabled()) diff --git a/source/java/org/alfresco/repo/content/filestore/FileContentWriter.java b/source/java/org/alfresco/repo/content/filestore/FileContentWriter.java index 753d785c55..00def14bbe 100644 --- a/source/java/org/alfresco/repo/content/filestore/FileContentWriter.java +++ b/source/java/org/alfresco/repo/content/filestore/FileContentWriter.java @@ -17,19 +17,16 @@ package org.alfresco.repo.content.filestore; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.io.RandomAccessFile; -import java.nio.channels.FileChannel; -import java.nio.channels.ReadableByteChannel; +import java.nio.channels.Channels; import java.nio.channels.WritableByteChannel; -import java.util.List; -import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.repo.content.AbstractContentWriter; -import org.alfresco.repo.content.RandomAccessContent; import org.alfresco.service.cmr.repository.ContentIOException; import org.alfresco.service.cmr.repository.ContentReader; -import org.alfresco.service.cmr.repository.ContentStreamListener; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -40,11 +37,12 @@ import org.apache.commons.logging.LogFactory; * * @author Derek Hulley */ -public class FileContentWriter extends AbstractContentWriter implements RandomAccessContent +public class FileContentWriter extends AbstractContentWriter { private static final Log logger = LogFactory.getLog(FileContentWriter.class); private File file; + private boolean allowRandomAccess; /** * Constructor that builds a URL based on the absolute path of the file. @@ -88,6 +86,12 @@ public class FileContentWriter extends AbstractContentWriter implements RandomAc super(url, existingContentReader); this.file = file; + allowRandomAccess = true; + } + + /* package */ void setAllowRandomAccess(boolean allow) + { + this.allowRandomAccess = allow; } /** @@ -118,7 +122,9 @@ public class FileContentWriter extends AbstractContentWriter implements RandomAc @Override protected ContentReader createReader() throws ContentIOException { - return new FileContentReader(this.file, getContentUrl()); + FileContentReader reader = new FileContentReader(this.file, getContentUrl()); + reader.setAllowRandomAccess(this.allowRandomAccess); + return reader; } @Override @@ -132,12 +138,23 @@ public class FileContentWriter extends AbstractContentWriter implements RandomAc throw new IOException("File exists - overwriting not allowed"); } // create the channel - RandomAccessFile randomAccessFile = new RandomAccessFile(file, "rw"); // will create it - FileChannel channel = randomAccessFile.getChannel(); + WritableByteChannel channel = null; + if (allowRandomAccess) + { + RandomAccessFile randomAccessFile = new RandomAccessFile(file, "rw"); // will create it + channel = randomAccessFile.getChannel(); + } + else + { + OutputStream os = new FileOutputStream(file); + channel = Channels.newChannel(os); + } // done if (logger.isDebugEnabled()) { - logger.debug("Opened channel to file: " + file); + logger.debug("Opened write channel to file: \n" + + " file: " + file + "\n" + + " random-access: " + allowRandomAccess); } return channel; } @@ -147,25 +164,6 @@ public class FileContentWriter extends AbstractContentWriter implements RandomAc } } - /** - * @param directChannel a file channel - */ - @Override - protected WritableByteChannel getCallbackWritableChannel( - WritableByteChannel directChannel, - List listeners) throws ContentIOException - { - if (!(directChannel instanceof FileChannel)) - { - throw new AlfrescoRuntimeException("Expected write channel to be a file channel"); - } - FileChannel fileChannel = (FileChannel) directChannel; - // wrap it - FileChannel callbackChannel = new CallbackFileChannel(fileChannel, listeners); - // done - return callbackChannel; - } - /** * @return Returns true always */ @@ -173,51 +171,4 @@ public class FileContentWriter extends AbstractContentWriter implements RandomAc { return true; // this is a writer } - - public FileChannel getChannel() throws ContentIOException - { - /* - * By calling this method, clients indicate that they wish to make random - * changes to the file. It is possible that the client might only want - * to update a tiny proportion of the file - or even none of it. Either - * way, the file must be as whole and complete as before it was accessed. - */ - - // go through the super classes to ensure that all concurrency conditions - // and listeners are satisfied - FileChannel channel = (FileChannel) super.getWritableChannel(); - // random access means that the the new content's starting point must be - // that of the existing content - ContentReader existingContentReader = getExistingContentReader(); - if (existingContentReader != null) - { - ReadableByteChannel existingContentChannel = existingContentReader.getReadableChannel(); - long existingContentLength = existingContentReader.getSize(); - // copy the existing content - try - { - channel.transferFrom(existingContentChannel, 0, existingContentLength); - // copy complete - if (logger.isDebugEnabled()) - { - logger.debug("Copied content for random access: \n" + - " writer: " + this + "\n" + - " existing: " + existingContentReader); - } - } - catch (IOException e) - { - throw new ContentIOException("Failed to copy from existing content to enable random access: \n" + - " writer: " + this + "\n" + - " existing: " + existingContentReader, - e); - } - finally - { - try { existingContentChannel.close(); } catch (IOException e) {} - } - } - // the file is now available for random access - return channel; - } } diff --git a/source/java/org/alfresco/repo/content/filestore/NoRandomAccessFileContentStoreTest.java b/source/java/org/alfresco/repo/content/filestore/NoRandomAccessFileContentStoreTest.java new file mode 100644 index 0000000000..39740e6db7 --- /dev/null +++ b/source/java/org/alfresco/repo/content/filestore/NoRandomAccessFileContentStoreTest.java @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2005 Alfresco, Inc. + * + * Licensed under the Mozilla Public License version 1.1 + * with a permitted attribution clause. You may obtain a + * copy of the License at + * + * http://www.alfresco.org/legal/license.txt + * + * 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.alfresco.repo.content.filestore; + +import java.io.File; + +import org.alfresco.repo.content.AbstractContentReadWriteTest; +import org.alfresco.repo.content.ContentStore; +import org.alfresco.util.TempFileProvider; + +/** + * Tests the file-based store when random access is not allowed, i.e. has to be spoofed. + * + * @see org.alfresco.repo.content.filestore.FileContentStore + * + * @author Derek Hulley + */ +public class NoRandomAccessFileContentStoreTest extends AbstractContentReadWriteTest +{ + private FileContentStore store; + + @Override + public void setUp() throws Exception + { + super.setUp(); + + // create a store that uses a subdirectory of the temp directory + File tempDir = TempFileProvider.getTempDir(); + store = new FileContentStore( + tempDir.getAbsolutePath() + + File.separatorChar + + getName()); + // disallow random access + store.setAllowRandomAccess(false); + } + + @Override + protected ContentStore getStore() + { + return store; + } +} diff --git a/source/java/org/alfresco/service/cmr/repository/ContentAccessor.java b/source/java/org/alfresco/service/cmr/repository/ContentAccessor.java index 14f88173d3..6d29a28f75 100644 --- a/source/java/org/alfresco/service/cmr/repository/ContentAccessor.java +++ b/source/java/org/alfresco/service/cmr/repository/ContentAccessor.java @@ -45,6 +45,10 @@ public interface ContentAccessor /** * Set the transaction provider that will be used when stream listeners are called. + * No transactions are started unless there are listeners present to be executed. + * For consistency, the execution of listeners will not be allowed to proceed + * unless this property has been set OR the channel close operations are executed + * within the context of a live transaction. * * @param transactionService a transaction provider */ diff --git a/source/java/org/alfresco/service/cmr/repository/ContentReader.java b/source/java/org/alfresco/service/cmr/repository/ContentReader.java index f29a957de6..35741fe82e 100644 --- a/source/java/org/alfresco/service/cmr/repository/ContentReader.java +++ b/source/java/org/alfresco/service/cmr/repository/ContentReader.java @@ -19,6 +19,7 @@ package org.alfresco.service.cmr.repository; import java.io.File; import java.io.InputStream; import java.io.OutputStream; +import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; /** @@ -86,6 +87,19 @@ public interface ContentReader extends ContentAccessor */ public ReadableByteChannel getReadableChannel() throws ContentIOException; + /** + * Provides read-only, random-access to the underlying content. In general, this method + * should be considered more expensive than the sequential-access method, + * {@link #getReadableChannel()}. + * + * @return Returns a random-access channel onto the content + * @throws ContentIOException + * + * @see #getReadableChannel() + * @see java.io.RandomAccessFile#getChannel() + */ + public FileChannel getFileChannel() throws ContentIOException; + /** * Get a stream to read from the underlying channel * diff --git a/source/java/org/alfresco/service/cmr/repository/ContentWriter.java b/source/java/org/alfresco/service/cmr/repository/ContentWriter.java index 0662317625..6249451a9b 100644 --- a/source/java/org/alfresco/service/cmr/repository/ContentWriter.java +++ b/source/java/org/alfresco/service/cmr/repository/ContentWriter.java @@ -19,6 +19,7 @@ package org.alfresco.service.cmr.repository; import java.io.File; import java.io.InputStream; import java.io.OutputStream; +import java.nio.channels.FileChannel; import java.nio.channels.WritableByteChannel; @@ -77,6 +78,23 @@ public interface ContentWriter extends ContentAccessor */ public WritableByteChannel getWritableChannel() throws ContentIOException; + /** + * Provides read-write, random-access to the underlying content. In general, this method + * should be considered more expensive than the sequential-access method, + * {@link #getWritableChannel()}. + *

+ * Underlying implementations use the truncate parameter to determine the + * most effective means of providing access to the content. + * + * @param truncate true to start with zero length content + * @return Returns a random-access channel onto the content + * @throws ContentIOException + * + * @see #getWritableChannel() + * @see java.io.RandomAccessFile#getChannel() + */ + public FileChannel getFileChannel(boolean truncate) throws ContentIOException; + /** * Get a stream to write to the underlying channel. *