Bug 49244 - Support for validation for OOXML format
Summary: Support for validation for OOXML format
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.6-FINAL
Hardware: PC Windows XP
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-04 06:08 UTC by rk
Modified: 2010-05-17 02:50 UTC (History)
0 users



Attachments
Subversion patch file with changes for OOXML validation support. (25.53 KB, application/octet-stream)
2010-05-04 06:08 UTC, rk
Details
Revised patch with a cleaner approach (84.89 KB, patch)
2010-05-08 16:54 UTC, rk
Details | Diff
test-data/spreadsheet/DataValidations-49244.xlsx (10.45 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2010-05-08 16:57 UTC, rk
Details

Note You need to log in before you can comment on or make changes to this bug.
Description rk 2010-05-04 06:08:13 UTC
Created attachment 25395 [details]
Subversion patch file with changes for OOXML validation support.

Data validations are supported for the binary format. We'd like to have this feature for the OOXML format as well. I tried to see how to get this done and I've a working version that fits my needs for now. Please find attached a patch created using Subversion that contains all the changes I've made. If you deem it fit please accept it ( it might require further work to be release worthy. ).
Comment 1 Nick Burch 2010-05-05 11:29:47 UTC
First question - why did you need to add new things to HSSF's validation support? If it's common functionality, we should probably move it into SS, rather than hacking HSSF

Secondly, we should probably have a file with existing constraints in it, which we can unit test. Finally on testing, as well at the test that creates new validation, we should probably also have one that creates some validation, saves to a temp file, re-opens it, and tests that the correct values can be read back out

Any chance you could submit a new patch with these changes in?
Comment 2 rk 2010-05-05 14:41:46 UTC
Thanks for taking a look at this. This would require some thought in how best to add things to SS package which are at the same time compatible with current HSSF code as well.

As for improving test cases, I accept all points. I will try to submit another patch. We need this feature and at the same time don't want to maintain a custom version of POI code base. Will try to submit another patch.
Comment 3 rk 2010-05-08 16:54:23 UTC
Created attachment 25418 [details]
Revised patch with a cleaner approach

This revised patch addresses points raised in comment #1.
 - Introduced set of interfaces in ss package. 
 - Added a new test case.
Comment 4 rk 2010-05-08 16:57:28 UTC
Created attachment 25419 [details]
test-data/spreadsheet/DataValidations-49244.xlsx
Comment 5 rk 2010-05-12 16:47:48 UTC
Has anyone had a chance to take a look at the revised patch yet ?
Comment 6 Yegor Kozlov 2010-05-16 12:13:59 UTC
Thanks for the patch, I applied it in r944869 with some changes.

I noticed that you changed visibility of HSSFDataValidation._region to public and HSSFSheet._sheet to protected. I reverted it back to private, I guess this change was needed for debug purposes.

POI's ooxml-schemas.jar does not support generics, the patch didn't compile against ooxml-schemas-1.0.jar and I had to change usages of CTDataValidations.getDataValidationList() to  CTDataValidations.getDataValidationArray. Are you using a custom jar with OOXML schemas?

I also refactored test cases to follow POI conventions and added missing ASF license header.

Regards,
Yegor
Comment 7 rk 2010-05-17 02:50:31 UTC
I compiled the ooxml-schemas jar independently and put it the ooxml-lib folder to get going  ( I noticed later that there were targets to build the jars required in the ooxml-lib folder ). Guess that is where the differences crept in. 

Thanks for making the changes to get it in.