Bug 54588 - CellFormat throws NPE for standard "Accounting Type" formatted cells
Summary: CellFormat throws NPE for standard "Accounting Type" formatted cells
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.7-FINAL
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-20 00:49 UTC by Trejkaz (pen name)
Modified: 2015-11-02 05:25 UTC (History)
1 user (show)



Attachments
How some of these cells are rendered in Excel (1.58 KB, image/png)
2013-02-22 01:16 UTC, sits
Details
Number format of cell as shown in Excel 2010 (32.74 KB, image/png)
2013-02-22 01:31 UTC, sits
Details
Example Excel file (25.00 KB, application/vnd.ms-excel)
2013-02-22 01:37 UTC, sits
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Trejkaz (pen name) 2013-02-20 00:49:23 UTC
This is something we're seeing in customer data.

    @Test
    public void testCellFormat() {
        CellFormat format = CellFormat.getInstance("_(*\"-\"??_);");
        // not sure if this assertion is right, but we'll figure that out once
        // the call completes without throwing an exception...
        assertThat(format.apply(135.45).text, is(equalTo("135.45")));
    }

I can't figure out what's specifically wrong with the format but I don't know what this format means, either. If someone figures out what it is, perhaps the bug summary could be made more specific.

Possibly related to Bug 53494, although in the case of that one, I can reproducibly show that it's the locale code causing it. In this case, there is no locale code, but there is other stuff which I don't know what it is.
Comment 1 Nick Burch 2013-02-20 12:04:48 UTC
Could you please let us know what the format string shows as in Excel? With backslashes and quotes in your test, it's hard to be sure which bits have been escaped by you, and which bits were escaped in Excel, so it's harder to work out what it should be
Comment 2 Trejkaz (pen name) 2013-02-20 12:12:30 UTC
Nothing was escaped by Excel in the example provided there. I caught the format string in the debugger while it was failing on the real file.

In the debug output it was just: _(*"-"??_);

Then I obviously had to add the backslashes to make it valid Java...
Comment 3 Trejkaz (pen name) 2013-02-20 12:16:14 UTC
Incidentally, the stack trace is on the same line as Bug 53494, so even though the two look totally unrelated, it could be the same bug. In both cases there are existing unit tests which reproduce the problem in isolation... so it shouldn't be too hard to find it, right?
Comment 4 sits 2013-02-22 00:41:30 UTC
Hi Nick.. is this something you can fix?  It would be great to see this resolved.. thanks.
Comment 5 Nick Burch 2013-02-22 00:51:29 UTC
I've been unable to get my copy of Excel to format a cell into anything sensible using this format string, so I can't work out what POI should be doing to start with

Are you able to supply an example of the raw value shown in Excel, and what it formats to as a display value, when using this format string? Or event better, maybe two or three (possibly involving a negative number)?
Comment 6 Trejkaz (pen name) 2013-02-22 01:01:25 UTC
I don't really have a copy of Excel to test this with, but...

In OpenOffice I can see it formatting them like normal numbers:
  (positive example) 126,789.82
  (negative example) -3,521.94

In Preview, I get something visually a bit odder:
  (positive example) _-126,789.82_-
  (negative example) -  3,521.94    (two spaces between the - and 3)
Comment 7 Trejkaz (pen name) 2013-02-22 01:05:56 UTC
Reading the docs:

_(*"-"??_);

   _(    - a space, but the width of the (
   *     - repeat the next character to fill the column width
   "-"   - seems to be equivalent to -, unless double quotes escaped it somehow
   ??    - minimum of two significant zeroes
   _)    - a space, but the width of the )
   ;

Since it's *"-" I would expect that it pads with -... but it seems to be padded with spaces instead, so I don't know... maybe the docs are just wrong. I'm not an expert on Excel, I just know that this format throws an exception.
Comment 8 sits 2013-02-22 01:16:43 UTC
Created attachment 29977 [details]
How some of these cells are rendered in Excel
Comment 9 sits 2013-02-22 01:31:26 UTC
Created attachment 29978 [details]
Number format of cell as shown in Excel 2010

FWIW.. right-clicking one of these cells and choosing "Format cells..." or "Number format..." shows the item formatted as type "Accounting" in Excel 2010.  So perhaps it is a built-in format type?
Comment 10 sits 2013-02-22 01:37:42 UTC
Created attachment 29979 [details]
Example Excel file

Now that I know it is a standard "accounting type", I can attach an example Excel file.  See attached.
Comment 11 sits 2013-02-25 06:02:30 UTC
Hi Nick - just curious, were you able to reproduce this with my sample Excel file that uses the "accounting type" for a cell value?
Comment 12 Nick Burch 2015-10-25 21:49:16 UTC
It has taken a while... But I think these should now be working thanks to the accountancy-with-currency support and tests added for #58536

Would you be able to take a look with a nightly build from 20151026 onwards / 3.14 beta 1 once released, and see if it now works correctly for you too?
Comment 13 Trejkaz (pen name) 2015-10-25 23:47:26 UTC
I checked the ticket on our side which referred to our one and apparently we had updated POI around 25 June 2013. Supposedly whatever we updated to at that point in time had fixed the issue for us already...
Comment 14 Nick Burch 2015-10-25 23:52:13 UTC
If you could grab a recent nightly build, and check that my work hasn't somehow un-fixed this for you (we didn't seem to have much in the way of accountancy unit tests before...), that'd be great and we can close the bug!
Comment 15 Trejkaz (pen name) 2015-10-26 00:59:00 UTC
I wish it were still easy to do that... but now that our build has been rejigged to use Gradle, it seems a lot harder to temporarily test a nightly release of a jar.

Unless POI's nightlies happen to be in a repo somewhere?
Comment 16 Nick Burch 2015-10-26 12:02:42 UTC
Nightlies are produced by Jenkins, and available at https://builds.apache.org/job/POI/lastSuccessfulBuild/artifact/build/dist/

I gather that a line like this should let you include the jars from the nightly bin build in gradle:

dependencies {
    compile fileTree(dir: 'libs', include: ['*.jar'])
}
Comment 17 Trejkaz (pen name) 2015-10-28 00:55:56 UTC
What our build has now:
    poi: [
        'poi:poi:3.9-N1.0',
        'poi:poi-ooxml:3.9-N1.0',
        'poi:poi-ooxml-schemas:3.9-N1.0',
        'poi:poi-scratchpad:3.9-N1.0',
    ]

What I tried (1):

    poi: [
            fileTree(dir: '/Users/daniel/Downloads/poi-3.14-beta1', include: ['*.jar'])
    ],

What I tried (2):

    poi: fileTree(dir: '/Users/daniel/Downloads/poi-3.14-beta1', include: ['*.jar']),

What I got:
    > Cannot convert the provided notation to an object of type ModuleVersionSelector: /Users/tester/Downloads/poi-3.14-beta1/poi-3.14-beta1-20151027.jar.
      The following types/formats are supported:
        - Instances of ModuleVersionSelector.
        - String or CharSequence values, for example 'org.gradle:gradle-core:1.0'.
        - Maps, for example [group: 'org.gradle', name:'gradle-core', version: '1.0'].
        - Collections or arrays of any other supported format. Nested collections/arrays will be flattened.
Comment 18 Dominik Stadler 2015-10-28 09:50:33 UTC
I use the following when testing pre-release versions:

dependencies {
	compile files('/tmp/poi-3.13/ooxml-lib/xmlbeans-2.6.0.jar')
	compile files('/tmp/poi-3.13/poi-3.13-20150929.jar')
	compile files('/tmp/poi-3.13/poi-ooxml-3.13-20150929.jar')
	compile files('/tmp/poi-3.13/poi-ooxml-schemas-3.13-20150929.jar')
}

These lines are in the normal dependency/configuration-section, I am not sure how you include the "poi: [...]" stuff in your build, unfortunately Groove/Gradle is very dynamic and allows to do all sorts of stuff.
Comment 19 Trejkaz (pen name) 2015-10-30 04:19:31 UTC
In one of the builds referring to it, it's:

    dependencies {
      // ...other dependencies...
      compile libraries.poi
      // ...other dependencies...
    }

So you would think that I could do this:

    ext.libraries = [
      // ...other libraries...

      poi: [
        files('.../poi-3.14-beta1/poi-3.14-beta1-20151027.jar'),
        files('.../poi-3.14-beta1/poi-3.14-ooxml-20151027.jar'),
        files('.../poi-3.14-beta1/poi-3.14-ooxml-schemas-20151027.jar'),
        files('.../poi-3.14-beta1/poi-3.14-scratchpad-20151027.jar'),
      ],

      // ...other libraries...
    }

And then this would be equivalent to putting an array of files() directly onto the compile. And yet, I get the same error. So I dunno, maybe it just isn't possible unless it's in a repo somewhere.
Comment 20 Dominik Stadler 2015-10-30 17:10:34 UTC
I'm not an expert on Gradle myself, but I would use the snippet that I posted previously in the dependencies-section directly, not in the poi-library-definition, i.e.

    dependencies {
      // ...other dependencies...
        compile files('poi-3.14-beta1/poi-3.14-beta1-20151027.jar')
        compile files('poi-3.14-beta1/poi-3.14-ooxml-20151027.jar')
        compile files('poi-3.14-beta1/poi-3.14-ooxml-schemas-20151027.jar')
        compile files('poi-3.14-beta1/poi-3.14-scratchpad-20151027.jar')
      // ...other dependencies...
    }

instead of 

    dependencies {
      // ...other dependencies...
      compile libraries.poi
      // ...other dependencies...
    }

Note: You might need to add a line for the XmlBeans-jar as well in this case.
Comment 21 Trejkaz (pen name) 2015-11-01 23:54:25 UTC
Turns out putting it directly in still gives me the same error. Still investigating. Gradle has really been a huge time sink... :/
Comment 22 Trejkaz (pen name) 2015-11-02 01:35:09 UTC
Okay, I finally managed to get the jars on the classpath, but there are compilation failures. So this is not something I'm going to be able to easily test. We'll just have to find out whether it works in the future when someone has time to rewrite all the code for the new version.
Comment 23 Dominik Stadler 2015-11-02 05:25:45 UTC
Ok, so I am closing this as FIXED anyway, keep us posted about your findings and if the compile-errors seem to be caused by an incorrect change in POI be sure to create new bug entries.