#169 Data validation introduced 2010-04-09 is too stringent

closed
Tom Edelson
Engine (46)
5
2010-07-16
2010-07-12
Tom Edelson
No

This is a "new" bug, and by "new" I don't just mean "newly discovered". This bug was introduced into the jGnash 1.x code base since the last release, which was 1.12.0.

Specifically, it was introduced in the commit that created revision 2119, which was done on 2010-04-09, and which was part of the fix for bug 2977267 (2,977,267).

The title of the latter bug is "When loading data, check for recursive accounts".

In revision 2119, some code was added to method

loadSnapShot (final Engine engine)

of class

jgnash.engine.Data

What the new code in "loadSnapShot" does is to throw an IllegalArgumentException in certain conditions: it does so if the field "objectList" of the Data object does not "begin with" the root account. The "list of objects" held in that field is derived from the XML node named "objects", a child of the node "dataRoot", in the input data file. So in effect, an error is being thrown if the first node within the "objects" node is not a representation of the "root account" of one's jGnash data.

When that error is thrown, the program stops trying to load that file, so the file becomes effectively unreadable to the current version of the program; that is, as things currently stand, jGnash 1.13 would not be able to load any file in which the root account was not the first item in the XML "objects" list.

Today, I found out that this is true of some perfectly valid jGnash data files which were created by older versions of jGnash. An old data file of mine was loaded by jGnash 1.12.0 just fine, but can't be loaded by the 1.13 code as it currently exists in the repository. It stops trying to load the file after throwing the above-mentioned IllegalArgumentException, with this message:

<quote>

SEVERE: java.lang.IllegalArgumentException: Object list passed to
jgnash.engine.Data.loadSnapShot
does not begin with root account

</quote>

This error check was added to method "loadSnapShot" as a side effect of trying to do something graceful with certain *invalid* data files, namely, ones where there is a circularity in the list of accounts (some account is listed in the data file as being, in effect, and directly or indirectly, its own ancestor). So, in order to do improved error recovery during input of some invalid data files, we are now failing to load some [older] valid data files. This is less than ideal.

Discussion

  • Tom Edelson
    Tom Edelson
    2010-07-16

    • status: open --> closed
     
  • Tom Edelson
    Tom Edelson
    2010-07-16

    This has been fixed.

    For purposes of this bug, jGnash 1.x data files (*.jgnash.xml) can be divided into four classes, according to whether they have either, neither, or both of two independent properties. The first property is a circularity in the account structure: at least one account which is alleged to be its own ancestorr. (This is the problem addressed in bug 2977267 (2,977,267).) The second property is whether, in the "objects" list in the input file, the first entry represents the root account.

    So the four classes, with the program's reaction to each *before* the fix, are:

    1. No circularity, and object list does begin with root account: loaded normally.

    2. Circularity, and object list does begin with root account: warning message is produced, and one of the accounts involved in the circularity is attached directly to the root account. In other words, loading the file succeeds with the minimum possible loss of data. This is the behavior introduced in fixing bug 2977267.

    3. No circularity, and object list does not begin with root account: program was aborting the loading of the file as soon as it discovered the latter fact. "Severe" error message in log file, "Error loading file" pop-up in the GUI, and file is not effectively loaded at all.

    4. Circularity, and object list does not begin with root account: same behavior as # 3. The program never gets far enough, in the attempt to load the file, to discover the circularity.

    With this fix, the behavior is the same for classes 1 and 2. It is changed a little for class 4, and a lot for class 3. Specifically:

    If the object list does not begin with the root account, an informational message is written to the log file. It notes that this phenomenon usually indicates that the file was created by an older version of jGnash.

    Then loading proceeds normally. If no circularity is found (class 3), then the only user-visible difference in behavior, as compared with a "normal" file (class 1) is the informational message.

    If [the object list does not begin with the root account, *and*] a circularity is found (class 4), then the program cannot behave as in class 2 -- cannot attach the wayward accounts directly to the root account -- because, at this point, it does not know what the root account is. So there's a whole sequence of messages in the log file, of increasing severity: first the informational message about the object list not beginning with the root account (as in class 3); then the warning message that a circularity in the account structure has been found (as in class 2); and finally, "severe" messages indicating that in the presence of both of these conditions, the loading of this file will be abandoned. And so, as in the pre-fix behavior for both classes 3 and 4, there is an "Error loading file" pop-up in the GUI, and the data from the file is not made available to the user (the load fails).

    The Java classes modified in this fix, all of them in package "jgnash.engine", were Account, Data, and Engine.

    The fix created revision 2231 in the repository.