NIFI-14426: Add support for HSSF format in ExcelReader processor

[zhtk]

Summary

NIFI-14426: Add support for HSSF format in ExcelReader processor

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using mvn clean install -P contrib-check
  • JDK 21

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[exceptionfactory]

Thanks @zhtk, can you rebase this pull request from the updated main branch? That will address an unrelated unit test failure.

[zhtk]

Yes, rebased @exceptionfactory

[exceptionfactory]

Thanks for working on this @zhtk. The general approach looks straightforward. Although there are potential memory implications for the older XLS format, this seems to be the best implementation strategy for now.

I noted one particular issue related to password handling for XLS files, and the lack of thread safety. As mentioned, I think the best option for now is to not support encrypted XLS, and let the read operation fail.

[zhtk]

After reading whole discussion I decided to remove type detection and to add Input File Type property. There are 2 reasons for that:

1. In case of passworded Excel file, to determine file type the whole Flow File stream must be read. At the moment the stream is resettable only up to 1024 * 1024 mark (i.e. in createRecordReader there is a call in.mark(1024 * 1024);). Reseting stream on files larger than 1 MB could be problematic. Working around this problem would require contributions to both POI and StreamingReader libraries.
2. Performance characteristics of reading XLS files is different than XLSX.

[dan-s1]

@zhtk From message below it seems you must rebase in order to resolve conflicts.

[dan-s1]

@zhtk great work so far. I had a few more suggestions.

[dan-s1]

@zhtk Can you please fix the conflicts you have locally and then push?

[zhtk]

TBH I don't see conflicts, github interface says: "No conflicts with base branch. Changes can be cleanly merged.", and local merge also was performed cleanly. To make sure everything is ok, I have rebased and squashed all commits on the branch, hope now it's okay

[dan-s1]

TBH I don't see conflicts, github interface says: "No conflicts with base branch. Changes can be cleanly merged.", and local merge also was performed cleanly. To make sure everything is ok, I have rebased and squashed all commits on the branch, hope now it's okay

Things look okay now with the force push. Thanks!

[dan-s1]

I noticed one more thing (hopefully the last :) )

[dan-s1]

@exceptionfactory I approve these changes. Is there anything else you see which needs changing?

[exceptionfactory]

@exceptionfactory I approve these changes. Is there anything else you see which needs changing?

Thanks, I will follow up soon.

[exceptionfactory]

Thanks for bringing this to completion @zhtk, and thanks for the reviews along the way @dan-s1, the latest version looks good.

I was going to note that the displayName() element on the property is not necessary, but I see that follows the other properties for now, so removing the displayName() across the board can be handled separately.

+1 merging

[zhtk]

I also thank you for reviews and for patience in getting these changes merged 🙇