From 7314516850021a157497c1fc83b796a01a98b29a Mon Sep 17 00:00:00 2001 From: ra0077 Date: Tue, 15 Mar 2016 14:22:52 +0100 Subject: [PATCH 1/5] bug59153_CSVDataSetFilesExceptions More clear message for CSV Data Set with filename empty wrong filename (not readable, not exist) I don't have made formating to help reviewing If anybody is ok, I will format it Antonio --- src/core/org/apache/jmeter/services/FileServer.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/core/org/apache/jmeter/services/FileServer.java b/src/core/org/apache/jmeter/services/FileServer.java index cba4d458ed0..7c2b77ed8e0 100644 --- a/src/core/org/apache/jmeter/services/FileServer.java +++ b/src/core/org/apache/jmeter/services/FileServer.java @@ -250,17 +250,17 @@ public void reserveFile(String filename, String charsetName, String alias) { * Creates an association between a filename and a File inputOutputObject, * and stores it for later use - unless it is already stored. * - * @param filename - relative (to base) or absolute file name (must not be null) + * @param filename - relative (to base) or absolute file name (must not be null or empty) * @param charsetName - the character set encoding to use for the file (may be null) * @param alias - the name to be used to access the object (must not be null) * @param hasHeader true if the file has a header line describing the contents * @return the header line; may be null * @throws EOFException if eof reached - * @throws IllegalArgumentException if header could not be read + * @throws IllegalArgumentException if header could not be read or filename is null or empty */ public synchronized String reserveFile(String filename, String charsetName, String alias, boolean hasHeader) { - if (filename == null){ - throw new IllegalArgumentException("Filename must not be null"); + if (filename == null || filename.isEmpty()){ + throw new IllegalArgumentException("Filename must not be null or empty"); } if (alias == null){ throw new IllegalArgumentException("Alias must not be null"); @@ -418,6 +418,7 @@ private BufferedReader getReader(String alias, boolean recycle, boolean firstLin } private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOException { + if (fileEntry.file.exists() && fileEntry.file.canRead() && fileEntry.file.isFile()) { FileInputStream fis = new FileInputStream(fileEntry.file); InputStreamReader isr = null; // If file encoding is specified, read using that encoding, otherwise use default platform encoding @@ -428,6 +429,9 @@ private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOExcept isr = new InputStreamReader(fis); } return new BufferedReader(isr); + } else { + throw new IllegalArgumentException("File "+ fileEntry.file.getName()+ " must exist and be readable"); + } } public synchronized void write(String filename, String value) throws IOException { From 0a683bb08a1d3ef4fc0ab465538d603089fab9ce Mon Sep 17 00:00:00 2001 From: ra0077 Date: Thu, 17 Mar 2016 08:05:03 +0100 Subject: [PATCH 2/5] bug59153_CSVDataSetFilesExceptions More clear message for CSV Data Set with filename empty wrong filename (not readable, not exist) New commit with formating and swap else branch Antonio --- .../apache/jmeter/services/FileServer.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/org/apache/jmeter/services/FileServer.java b/src/core/org/apache/jmeter/services/FileServer.java index 7c2b77ed8e0..9a2b601a829 100644 --- a/src/core/org/apache/jmeter/services/FileServer.java +++ b/src/core/org/apache/jmeter/services/FileServer.java @@ -418,20 +418,20 @@ private BufferedReader getReader(String alias, boolean recycle, boolean firstLin } private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOException { - if (fileEntry.file.exists() && fileEntry.file.canRead() && fileEntry.file.isFile()) { - FileInputStream fis = new FileInputStream(fileEntry.file); - InputStreamReader isr = null; - // If file encoding is specified, read using that encoding, otherwise use default platform encoding - String charsetName = fileEntry.charSetEncoding; - if(!JOrphanUtils.isBlank(charsetName)) { - isr = new InputStreamReader(fis, charsetName); - } else { - isr = new InputStreamReader(fis); - } - return new BufferedReader(isr); - } else { + if (!fileEntry.file.exists() || !fileEntry.file.canRead() || !fileEntry.file.isFile()) { throw new IllegalArgumentException("File "+ fileEntry.file.getName()+ " must exist and be readable"); - } + } else { + FileInputStream fis = new FileInputStream(fileEntry.file); + InputStreamReader isr = null; + // If file encoding is specified, read using that encoding, otherwise use default platform encoding + String charsetName = fileEntry.charSetEncoding; + if(!JOrphanUtils.isBlank(charsetName)) { + isr = new InputStreamReader(fis, charsetName); + } else { + isr = new InputStreamReader(fis); + } + return new BufferedReader(isr); + } } public synchronized void write(String filename, String value) throws IOException { From 029e240e4d8027c60523c0b9803bde46629718fd Mon Sep 17 00:00:00 2001 From: ra0077 Date: Thu, 17 Mar 2016 11:44:25 +0100 Subject: [PATCH 3/5] bug59153_CSVDataSetFilesExceptions exists() removed and else removed Antonio --- .../apache/jmeter/services/FileServer.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/core/org/apache/jmeter/services/FileServer.java b/src/core/org/apache/jmeter/services/FileServer.java index 9a2b601a829..eb65793a3c5 100644 --- a/src/core/org/apache/jmeter/services/FileServer.java +++ b/src/core/org/apache/jmeter/services/FileServer.java @@ -418,21 +418,20 @@ private BufferedReader getReader(String alias, boolean recycle, boolean firstLin } private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOException { - if (!fileEntry.file.exists() || !fileEntry.file.canRead() || !fileEntry.file.isFile()) { + if (!fileEntry.file.canRead() || !fileEntry.file.isFile()) { throw new IllegalArgumentException("File "+ fileEntry.file.getName()+ " must exist and be readable"); + } + FileInputStream fis = new FileInputStream(fileEntry.file); + InputStreamReader isr = null; + // If file encoding is specified, read using that encoding, otherwise use default platform encoding + String charsetName = fileEntry.charSetEncoding; + if(!JOrphanUtils.isBlank(charsetName)) { + isr = new InputStreamReader(fis, charsetName); } else { - FileInputStream fis = new FileInputStream(fileEntry.file); - InputStreamReader isr = null; - // If file encoding is specified, read using that encoding, otherwise use default platform encoding - String charsetName = fileEntry.charSetEncoding; - if(!JOrphanUtils.isBlank(charsetName)) { - isr = new InputStreamReader(fis, charsetName); - } else { - isr = new InputStreamReader(fis); - } - return new BufferedReader(isr); - } - } + isr = new InputStreamReader(fis); + } + return new BufferedReader(isr); + } public synchronized void write(String filename, String value) throws IOException { FileEntry fileEntry = files.get(filename); From cf68e257baee086559c7938c282e430c01f6bca5 Mon Sep 17 00:00:00 2001 From: ra0077 Date: Thu, 17 Mar 2016 12:05:30 +0100 Subject: [PATCH 4/5] bug59153_CSVDataSetFilesExceptions Remove extra spaces Antonio --- src/core/org/apache/jmeter/services/FileServer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/org/apache/jmeter/services/FileServer.java b/src/core/org/apache/jmeter/services/FileServer.java index eb65793a3c5..0b777ac96f5 100644 --- a/src/core/org/apache/jmeter/services/FileServer.java +++ b/src/core/org/apache/jmeter/services/FileServer.java @@ -431,7 +431,7 @@ private BufferedReader createBufferedReader(FileEntry fileEntry) throws IOExcept isr = new InputStreamReader(fis); } return new BufferedReader(isr); - } + } public synchronized void write(String filename, String value) throws IOException { FileEntry fileEntry = files.get(filename); From e6772ccc23efc50ed7fb765edd0f289463a2d26a Mon Sep 17 00:00:00 2001 From: ra0077 Date: Thu, 17 Mar 2016 14:37:44 +0100 Subject: [PATCH 5/5] bug59153_CSVDataSetFilesExceptions Fix uniarty tests Antonio --- test/src/org/apache/jmeter/config/TestCVSDataSet.java | 10 ++++++---- .../src/org/apache/jmeter/services/TestFileServer.java | 6 ++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/src/org/apache/jmeter/config/TestCVSDataSet.java b/test/src/org/apache/jmeter/config/TestCVSDataSet.java index 9411d9d7324..d3617c6bb4e 100644 --- a/test/src/org/apache/jmeter/config/TestCVSDataSet.java +++ b/test/src/org/apache/jmeter/config/TestCVSDataSet.java @@ -62,10 +62,12 @@ public void testopen() throws Exception { csv.setFilename("No.such.filename"); csv.setVariableNames("a,b,c"); csv.setDelimiter(","); - csv.iterationStart(null); - assertEquals("",threadVars.get("a")); - assertEquals("",threadVars.get("b")); - assertEquals("",threadVars.get("c")); + try { + csv.iterationStart(null); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException ignored) { + } + csv = new CSVDataSet(); csv.setFilename(findTestPath("testfiles/testempty.csv")); diff --git a/test/src/org/apache/jmeter/services/TestFileServer.java b/test/src/org/apache/jmeter/services/TestFileServer.java index 28a0889b1bb..e1ac4ba3af9 100644 --- a/test/src/org/apache/jmeter/services/TestFileServer.java +++ b/test/src/org/apache/jmeter/services/TestFileServer.java @@ -130,15 +130,13 @@ public void testHeaderMissingFile() throws Exception { try { FS.reserveFile(missing,charsetName,alias,true); fail("Expected IllegalArgumentException"); - } catch (IllegalArgumentException e) { - assertTrue("Expected FNF", e.getCause() instanceof java.io.FileNotFoundException); + } catch (IllegalArgumentException ignored) { } // Ensure second invocation gets same behaviour try { FS.reserveFile(missing,charsetName,alias,true); fail("Expected IllegalArgumentException"); - } catch (IllegalArgumentException e) { - assertTrue("Expected FNF", e.getCause() instanceof java.io.FileNotFoundException); + } catch (IllegalArgumentException ignored) { } }