Hi Vincent, On Mon, Mar 23, 2009 at 2:04 AM, Vincent Massol <[email protected]> wrote:
On Mar 22, 2009, at 10:54 AM, asiri (SVN) wrote:
Author: asiri Date: 2009-03-22 10:54:52 +0100 (Sun, 22 Mar 2009) New Revision: 17895
Added: platform/core/trunk/xwiki-officeimporter/src/test/java/org/xwiki/ officeimporter/internal/OfficeImporterFileStorageTest.java Modified: platform/core/trunk/xwiki-officeimporter/src/main/java/org/xwiki/ officeimporter/internal/OfficeImporterFileStorage.java Log: XWIKI-3406: Office Importer not working under windows environments
* Fixed the problem with invalid file name characters.
Modified: platform/core/trunk/xwiki-officeimporter/src/main/java/org/ xwiki/officeimporter/internal/OfficeImporterFileStorage.java =================================================================== --- platform/core/trunk/xwiki-officeimporter/src/main/java/org/xwiki/ officeimporter/internal/OfficeImporterFileStorage.java 2009-03-22 07:16:13 UTC (rev 17894) +++ platform/core/trunk/xwiki-officeimporter/src/main/java/org/xwiki/ officeimporter/internal/OfficeImporterFileStorage.java 2009-03-22 09:54:52 UTC (rev 17895) @@ -50,13 +50,18 @@ private File outputFile = null;
/** + * A regular expression matching any invalid file name character.
I think this comment should be improved as these characters are not invalid on all OSes AFAIK.
Fixed.
+ */ + public static final String INVALID_FILE_NAME_CHARS = "[/\\\\:\\* \\?\"<>|]"; + + /** * Default constructor. * * @param tempDirName name of the temporary files directory. */ public OfficeImporterFileStorage(String tempDirName) { - tempDir = new File(System.getProperty("java.io.tmpdir"), tempDirName); + tempDir = new File(System.getProperty("java.io.tmpdir"), tempDirName.replaceAll(INVALID_FILE_NAME_CHARS, "-")); tempDir.mkdir();
hmm, shouldn't you ensure these files are deleted on jvm exit? (using deleteOnExit())
I have the dedicated method OfficeImporterFileStorage:cleanUp() for cleaning up the temporary storage. I use this to ensure that for each user for each request, a temporary storage is created and then destroyed right after the request is served. I can't wait for the jvm termination because these temporary files can be large and i must make sure that multiple requests do not use the same temporary directory for it's data.
Also note that all temporary files should be created through a common generic xwiki api. This has been discussed last week on the xwiki dev list I think. We were wondering where to put it and ApplicationContext was discussed. Using java.io.tmpdir is not completely correct in a servlet environment for ex...
I Agree.
inputFile = new File(tempDir, "input.tmp"); outputDir = new File(tempDir, "output");
Added: platform/core/trunk/xwiki-officeimporter/src/test/java/org/ xwiki/officeimporter/internal/OfficeImporterFileStorageTest.java =================================================================== --- platform/core/trunk/xwiki-officeimporter/src/test/java/org/xwiki/ officeimporter/internal/ OfficeImporterFileStorageTest.java (rev 0) +++ platform/core/trunk/xwiki-officeimporter/src/test/java/org/xwiki/ officeimporter/internal/OfficeImporterFileStorageTest.java 2009-03-22 09:54:52 UTC (rev 17895) @@ -0,0 +1,58 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.officeimporter.internal; + +import com.xpn.xwiki.test.AbstractXWikiComponentTestCase; + +/** + * Test calss for {@link OfficeImporterFileStorage}.
typo.
Fixed.
+ * + * @version $Id$ + * @since 1.9M1 + */ +public class OfficeImporterFileStorageTest extends AbstractXWikiComponentTestCase +{ + /** + * {@inheritDoc} + */ + protected void setUp() throws Exception + { + getComponentManager ().registerComponentDescriptor (MockDocumentAccessBridge.getComponentDescriptor()); + super.setUp(); + } + + /** + * Test filtering of invalid file name characters. + */ + public void testInvalidFileNameCharacterFiltering() + { + String[] invalidCharacters = {"/", "\\", ":", "*", "?", "\"", "<", ">", "|"}; + String filePath = "Temp%sDir"; + OfficeImporterFileStorage storage = null; + try { + for (String s : invalidCharacters) { + storage = new OfficeImporterFileStorage(String.format(filePath, s));
why not create a single string name containing all chars?
Fixed.
+ assertEquals("Temp-Dir", storage.getTempDir().getName()); + } + } catch (RuntimeException ex) { + fail(ex.getMessage());
See previous mail on this anti pattern for junit tests.
Fixed. Thanks. - Asiri