Re: svn commit: r1180004 - in /jakarta/jmeter/trunk: src/components/org/apache/jmeter/config/CSVDataSet.java xdocs/changes.xml

On 7 October 2011 12:51, sebb <[email protected]> wrote:
> On 7 October 2011 12:10,  <[email protected]> wrote:
>> Author: pmouawad
>> Date: Fri Oct  7 11:10:18 2011
>> New Revision: 1180004
>>
>> URL: http://svn.apache.org/viewvc?rev=1180004&view=rev
>> Log:
>> Bug 51988 - CSV Data Set Configuration does not resolve default delimiter for header parsing when variables field is empty
>>
>> Modified:
>>    jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java
>>    jakarta/jmeter/trunk/xdocs/changes.xml
>>
>> Modified: jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java
>> URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java?rev=1180004&r1=1180003&r2=1180004&view=diff
>> ==============================================================================
>> --- jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java (original)
>> +++ jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java Fri Oct  7 11:10:18 2011
>> @@ -100,74 +100,74 @@ public class CSVDataSet extends ConfigTe
>>     public void iterationStart(LoopIterationEvent iterEvent) {
>>         FileServer server = FileServer.getFileServer();
>>         final JMeterContext context = getThreadContext();
>> +        String delim = getDelimiter();
>> +        if (delim.startsWith("\\t")) { // $NON-NLS-1$
>
> This is a change from the previous code, which uses .equals().
> I don't think startsWith() is suitable here.
>
>> +            delim = "\t";// Make it easier to enter a Tab // $NON-NLS-1$
>> +        } else if (delim.length() == 0) {
>> +            log.warn("Empty delimiter converted to ','");
>> +            delim = ",";
>> +        }
>>         if (vars == null) {
>>             String _fileName = getFilename();
>>             String mode = getShareMode();
>>             int modeInt = CSVDataSetBeanInfo.getShareModeAsInt(mode);
>> -            switch(modeInt){
>> -                case CSVDataSetBeanInfo.SHARE_ALL:
>> -                    alias = _fileName;
>> -                    break;
>> -                case CSVDataSetBeanInfo.SHARE_GROUP:
>> -                    alias = _fileName+"@"+System.identityHashCode(context.getThreadGroup());
>> -                    break;
>> -                case CSVDataSetBeanInfo.SHARE_THREAD:
>> -                    alias = _fileName+"@"+System.identityHashCode(context.getThread());
>> -                    break;
>> -                default:
>> -                    alias = _fileName+"@"+mode; // user-specified key
>> -                    break;
>> +            switch (modeInt) {
>> +            case CSVDataSetBeanInfo.SHARE_ALL:
>> +                alias = _fileName;
>> +                break;
>> +            case CSVDataSetBeanInfo.SHARE_GROUP:
>> +                alias = _fileName + "@" + System.identityHashCode(context.getThreadGroup());
>> +                break;
>> +            case CSVDataSetBeanInfo.SHARE_THREAD:
>> +                alias = _fileName + "@" + System.identityHashCode(context.getThread());
>> +                break;
>> +            default:
>> +                alias = _fileName + "@" + mode; // user-specified key
>> +                break;
>>             }
>>             final String names = getVariableNames();
>> -            if (names == null || names.length()==0) {
>> +            if (names == null || names.length() == 0) {

Also, please don't change formatting when fixing bugs, unless directly
related - e.g. adding surrounding try/catch or if statement.

It makes it much harder to see what has actually changed.

>>                 String header = server.reserveFile(_fileName, getFileEncoding(), alias, true);
>>                 try {
>> -                    vars = CSVSaveService.csvSplitString(header, getDelimiter().charAt(0));
>> +                    vars = CSVSaveService.csvSplitString(header, delim.charAt(0));
>>                     firstLineIsNames = true;
>>                 } catch (IOException e) {
>> -                    log.warn("Could not split CSV header line",e);
>> +                    log.warn("Could not split CSV header line", e);
>>                 }
>>             } else {
>>                 server.reserveFile(_fileName, getFileEncoding(), alias);
>>                 vars = JOrphanUtils.split(names, ","); // $NON-NLS-1$
>>             }
>>         }
>> -            String delim = getDelimiter();
>> -            if (delim.equals("\\t")) { // $NON-NLS-1$
>> -                delim = "\t";// Make it easier to enter a Tab // $NON-NLS-1$
>> -            } else if (delim.length()==0){
>> -                log.warn("Empty delimiter converted to ','");
>> -                delim=",";
>> -            }
>> -            // TODO: fetch this once as per vars above?
>> -            JMeterVariables threadVars = context.getVariables();
>> -            String line = null;
>> +        // TODO: fetch this once as per vars above?
>> +        JMeterVariables threadVars = context.getVariables();
>> +        String line = null;
>> +        try {
>> +            line = server.readLine(alias, getRecycle(), firstLineIsNames);
>> +        } catch (IOException e) { // treat the same as EOF
>> +            log.error(e.toString());
>> +        }
>> +        if (line != null) {// i.e. not EOF
>>             try {
>> -                line = server.readLine(alias, getRecycle(), firstLineIsNames);
>> -            } catch (IOException e) { // treat the same as EOF
>> -                log.error(e.toString());
>> -            }
>> -            if (line!=null) {// i.e. not EOF
>> -                try {
>> -                    String[] lineValues = getQuotedData() ?
>> -                            CSVSaveService.csvSplitString(line, delim.charAt(0))
>> -                            : JOrphanUtils.split(line, delim, false);
>> -                            for (int a = 0; a < vars.length && a < lineValues.length; a++) {
>> -                                threadVars.put(vars[a], lineValues[a]);
>> -                            }
>> -                } catch (IOException e) { // Should only happen for quoting errors
>> -                   log.error("Unexpected error splitting '"+line+"' on '"+delim.charAt(0)+"'");
>> -                }
>> -                // TODO - report unused columns?
>> -                // TODO - provide option to set unused variables ?
>> -            } else {
>> -                if (getStopThread()) {
>> -                    throw new JMeterStopThreadException("End of file detected");
>> -                }
>> -                for (int a = 0; a < vars.length ; a++) {
>> -                    threadVars.put(vars[a], EOFVALUE);
>> +                String[] lineValues = getQuotedData() ?
>> +                        CSVSaveService.csvSplitString(line, delim.charAt(0))
>> +                        : JOrphanUtils.split(line, delim, false);
>> +                for (int a = 0; a < vars.length && a < lineValues.length; a++) {
>> +                    threadVars.put(vars[a], lineValues[a]);
>>                 }
>> +            } catch (IOException e) { // Should only happen for quoting errors
>> +                log.error("Unexpected error splitting '" + line + "' on '" + delim.charAt(0) + "'");
>> +            }
>> +            // TODO - report unused columns?
>> +            // TODO - provide option to set unused variables ?
>> +        } else {
>> +            if (getStopThread()) {
>> +                throw new JMeterStopThreadException("End of file detected");
>>             }
>> +            for (int a = 0; a < vars.length ; a++) {
>> +                threadVars.put(vars[a], EOFVALUE);
>> +            }
>> +        }
>>     }
>>
>>     /**
>>
>> Modified: jakarta/jmeter/trunk/xdocs/changes.xml
>> URL: http://svn.apache.org/viewvc/jakarta/jmeter/trunk/xdocs/changes.xml?rev=1180004&r1=1180003&r2=1180004&view=diff
>> ==============================================================================
>> --- jakarta/jmeter/trunk/xdocs/changes.xml (original)
>> +++ jakarta/jmeter/trunk/xdocs/changes.xml Fri Oct  7 11:10:18 2011
>> @@ -119,6 +119,7 @@ Mirror server now uses default port 8081
>>  <h3>General</h3>
>>  <ul>
>>  <li>Bug 51937 - JMeter does not handle missing TestPlan entry well</li>
>> +<li>Bug 51988 - CSV Data Set Configuration does not resolve default delimiter for header parsing when variables field is empty</li>
>>  </ul>
>>
>>  <!-- =================== Improvements =================== -->
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>>
>