Appendium / flatpack

CSV/Tab Delimited and Fixed Length Parser and Writer
http://flatpack.sf.net
Apache License 2.0
57 stars 20 forks source link

isRecordID NPE if using absolute (absolute should should move also currentRecord) #9

Closed SvenNold closed 9 years ago

SvenNold commented 10 years ago

Hi

sorry for using this contact but the issue tracker on sourceforge looks pretty dead. I forced an update for camel-flatpack component to directly use flatpack jar from maven central (before they used servicemix wrapper). But the test cases fail, I identified a change on DefaultDataSet, which causes this error.

In 3.2.0 it was

public boolean isRecordID(String recordID)
  {
    String rowID = ((Row)this.rows.get(this.pointer)).getMdkey();
    if (rowID == null) {
      rowID = "detail";
    }
    return rowID.equals(recordID);
  }

In 3.4.1 it is:

public boolean isRecordID(final String recordID) {
        return currentRecord.isRecordID(recordID);
    }

Problem is that in method absolute the pointer gets changed, but not the currentRecord, which causes a NPE.

Could you provide a fix so that I can push forward to use flatpack 3.4.X for the camel-flatpack component.

benoitx commented 10 years ago

Thanks for raising this. I'll investigate (never user the absolute method). Would you have a unit test that could show this and I'll use it as a basis for the patch. Many thanks

SvenNold commented 10 years ago

Please use the current camel-flatpack (https://github.com/apache/camel/tree/master/components/camel-flatpack) you need to modify the pom.xml to use your lib:

    <dependency>
  <groupId>net.sf.flatpack</groupId>
  <artifactId>flatpack</artifactId>
  <version>3.4.1</version>
</dependency>

Instead of the service mix one.

benoitx commented 10 years ago

Would this change to absolute fix the issue you think:

Thanks

    /**
     * Sets the absolute position of the record pointer
     *
     * @param localPointer
     *            - int
     * @exception IndexOutOfBoundsException
     */
    @Override
    public void absolute(final int localPointer) {
        if (localPointer < 0 || localPointer > rows.size() - 1) {
            throw new IndexOutOfBoundsException("INVALID POINTER LOCATION: " + localPointer);
        }

        pointer = localPointer;
        currentRecord = new RowRecord(rows.get(pointer), metaData, parser.isColumnNamesCaseSensitive(), pzConvertProps, strictNumericParse,
                upperCase, lowerCase, parser.isNullEmptyStrings());
    }
SvenNold commented 10 years ago

At least it seems to pass the tests. Could you please provide a 3.4.2 version on maven central, so I could get this version proposed for next version 2.14.0 of camel.

benoitx commented 10 years ago

I will try to do a release soon. Might have to wait a couple of days.

Thanks

benoitx commented 10 years ago

oh, you want a 3.4.2 (i.e. NOT JDK8? which would be 4.0.1)

SvenNold commented 10 years ago

3.4.2 would be great. I will slowly start the discussion about integrating the 4.0.1 with jdk8 support, which won't be such a p-i-a as soon camel-flatpack directly references your lib ;)

Not too much changes at once otherwise it gets declined.

SvenNold commented 10 years ago

Cannot find the suggested 3.4.2-SNAPSHOT @ https://oss.sonatype.org/content/repositories/snapshots/net/sf/flatpack/flatpack/

benoitx commented 10 years ago

Yes, the Travis-CI changes were in master.

This is now ready

https://oss.sonatype.org/content/repositories/snapshots/net/sf/flatpack/flatpack/3.4.2-SNAPSHOT/

Please give it a go

many thanks

B

SvenNold commented 10 years ago

Great!!:

Tests run: 36, Failures: 0, Errors: 0, Skipped: 0

Please release it mvn central so that I can proceed.

Thanks

benoitx commented 10 years ago

Hi

I believe that it is release in Central now.

Kindly let me know

Thanks

SvenNold commented 10 years ago

Thanks that was very fast. I already got CAMEL-7776 resolved, starting from camel v2.14 flatpack will be directly referenced. Nice job again!

-------- Ursprüngliche Nachricht -------- Von: Benoit Xhenseval notifications@github.com Datum: 04.09.2014 17:49 (GMT+01:00) An: Appendium/flatpack flatpack@noreply.github.com Cc: Sven Nold Sven.Nold@isb-ag.de Betreff: Re: [flatpack] isRecordID NPE if using absolute (absolute should should move also currentRecord) (#9)

Hi

I believe that it is release in Central now.

Kindly let me know

Thanks

Reply to this email directly or view it on GitHubhttps://github.com/Appendium/flatpack/issues/9#issuecomment-54486530.

benoitx commented 10 years ago

Thank you for raising the issue.

I'll be releasing 4.0 soon

Benoit


Important Notice This communication contains information that is considered confidential and may also be privileged. It is for the exclusive use of the intended recipient(s). If you are not the intended recipient(s) please note that any form of distribution, copying or use of this communication or the information in it is strictly prohibited and may be unlawful. If you have received this communication in error please return it to the sender and delete the original

On 6 Sep 2014, at 17:30, SvenNold notifications@github.com wrote:

Thanks that was very fast. I already got CAMEL-7776 resolved, starting from camel v2.14 flatpack will be directly referenced. Nice job again!

-------- Ursprüngliche Nachricht -------- Von: Benoit Xhenseval notifications@github.com Datum: 04.09.2014 17:49 (GMT+01:00) An: Appendium/flatpack flatpack@noreply.github.com Cc: Sven Nold Sven.Nold@isb-ag.de Betreff: Re: [flatpack] isRecordID NPE if using absolute (absolute should should move also currentRecord) (#9)

Hi

I believe that it is release in Central now.

Kindly let me know

Thanks

Reply to this email directly or view it on GitHub< https://github.com/Appendium/flatpack/issues/9#issuecomment-54486530>.

— Reply to this email directly or view it on GitHub https://github.com/Appendium/flatpack/issues/9#issuecomment-54719451.