ccavanaugh / jgnash

jGnash Personal Finance
http://ccavanaugh.github.io/jgnash/
Other
139 stars 80 forks source link

Test to reproduce wrong account balance after upgrade from 2.12 to 2.13 #5

Closed signed closed 9 years ago

signed commented 10 years ago

After upgrading from 2.12 to 2.13 I noticed that some account balances changed. I debugged into the sources and managed to create a minimal example to reproduce the problem (this pull request). I have a Transaction that contains two identical TransactionEntry (have the same hash). When the XML-File is loaded, only one remains in the Transaction.Set transactionEntries. When the file is stored, the TransactionEntry is lost for good.

With jgnash 2.12 this worked fine. TransactionEntries where stored within a List instead of a HashSet

I think the problem was introduced with 596a9b8ce2e8c5a91233d186ff2481067a17c044 Using a set significantly improves Hibernate update performance.

To reproduce the problem merge to master, adjust the EngineFactory call and run the test.

Any chance you can provide a patch for 2.13.x?

ccavanaugh commented 9 years ago

The duplicate hash was a bug, and you are correct that the change to a HashSet fixed the issue.

I suspect something went wrong with your file in an earlier release and impacted the correctness of your data. The fix or patch if needed would be to do exactly what jGnash is already doing which is to remove one of the duplicate entries.

I'm curious as to how your account balances were correct with the duplicates being factored in. Did they have the same hash but different amounts, comments, etc?

signed commented 9 years ago

The duplicate hash was a bug, and you are correct that the change to a HashSet fixed the issue.

To me this was no bug. It is a feature I am using. I actually bought two of the same kind within one transaction (in the sample file two bread from the baker). And because they are two of the same kind they do not vary in there values => same hasecode.

Perhaps I missunderstood the concept of the Transaction?

ccavanaugh commented 9 years ago

I believe you have the understanding of Transaction correct.

I've looked closer at the old code. It is a bug, and there is not a fix I can put in place. This release is almost 18 months old and the file format has changed significantly since then. The newer release 2.13.0+ dropped the hash value that is causing you issues and in doing so, fixed the bug so it can't occur again.

A possible work around to recover the old file is to alter the hash values of the TransactionEntry so they are unique before opening the file in 2.13.0. The hash value only needs to be unique within the Transaction.

Regards, Craig

On Wed, Sep 24, 2014 at 12:06 PM, signed notifications@github.com wrote:

The duplicate hash was a bug, and you are correct that the change to a HashSet fixed the issue.

To me this was no bug. It is a feature I am using. I actually bought two of the same kind within one transaction (in the sample file two bread from the baker). And because they are two of the same kind they do not vary in there values => same hasecode.

Perhaps I missunderstood the concept of the Transaction?

— Reply to this email directly or view it on GitHub https://github.com/ccavanaugh/jgnash/pull/5#issuecomment-56694013.