Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MCP: Tag Reports: java.lang.IllegalArgumentException: The maximum length of cell contents (text) is 32,767 characters #2146

Closed
3 tasks
Appar-singh opened this issue Dec 30, 2019 · 9 comments
Assignees

Comments

@Appar-singh
Copy link

Required Information

  • AEM Version, including Service Packs, Cumulative Fix Packs, etc: 6.4 & 6.5
  • ACS AEM Commons Version: 4.4.0
  • Reproducible on Latest? yes

Expected Behavior

Tag Reporter did generate a report for all tags in the system. The Admin should be able to view the report and download the report in Excel or CSV.

Actual Behavior

The Tag Reports was generated however download to excel gave an exception. View reports functionality works as expected.

Steps to Reproduce

If you have 100's of assets attached to some tags, the Excel Cell size would overflow the allowed excel cell size of : 32,767. The following exception would be encountered:

Error during requesting: '/var/acs-commons/mcp/instances/485F2250CCB2699B/_jcr_content/report.xlsx'

Error Message:

java.lang.IllegalArgumentException: The maximum length of cell contents (text) is 32,767 characters

Processing Info:

An option would be to generate the reports in CSV rather than .xls or .xlsx file formats to avoid the excel restriction of maximum cell size.

Links

Links to related assets, e.g. content packages containing test components

@Appar-singh Appar-singh changed the title java.lang.IllegalArgumentException: The maximum length of cell contents (text) is 32,767 characters MCP: Tag Reports: java.lang.IllegalArgumentException: The maximum length of cell contents (text) is 32,767 characters Dec 30, 2019
@klcodanr klcodanr self-assigned this Jan 2, 2020
@klcodanr
Copy link
Contributor

klcodanr commented Jan 2, 2020

nice issue @Appar-singh, let me look into this, I'm not sure / aware of a way to get MCPs to export in non-Excel format.

@badvision
Copy link
Contributor

We're rather at the mercy of what POI can handle since that's the library we're using. But all the same, it would make sense to just truncate each value to fewer than 4096 characters anyway since that's a lot of junk being shoved in there and indicates some other problem, I'm sure. Is truncation an acceptable solution @klcodanr ?

@klcodanr
Copy link
Contributor

klcodanr commented Jan 6, 2020

@badvision - Agreed. What I'm thinking to do is set a default max cell value of 4096 and then allow executors to override with a message indicating that the output is limited to a maximum of 32,767 characters.

@klcodanr
Copy link
Contributor

klcodanr commented Jan 6, 2020

@badvision I just submitted a PR: #2152 to resolve this issue.

@Appar-singh
Copy link
Author

@badvision @klcodanr

I am not sure if truncation is the right approach. This MCP utility is for generating reports. An example report would be for Tags. Each Tag may have few hundred assets attached to them. So in this case, a single cell would hold JCR Paths of few hundred assets. Truncation doesn't help if admins are trying to generate content reports from the system.

I am sure this is just one of the scenarios where truncation doesn't help and there might definitely be more use-cases.

@klcodanr
Copy link
Contributor

klcodanr commented Jan 6, 2020

@Appar-singh
Any thoughts on what approach would be better?

We can't really implement different formats without significant changes to the MCP core and even then you'd have wierd functionality where Excel still wouldn't work no matter what.

One thought would be to just add more cells and split the content into multiple cells either horizontally or vertically. Would that work for you?

@klcodanr klcodanr reopened this Jan 6, 2020
@Appar-singh
Copy link
Author

@klcodanr
Thanks for looking into it. I would have loved to have another button for Exporting to CSV. So that the option is on users to select the right format for export based on how much data they expect to see in the report. And the export to Excel functionality can continue to be out there until CSV export functionality is completed quality tested by the community.

If CSV is far fetched idea, then splitting cells vertically might help. This means that if the content of the cell is more than the max limit, we see a new row below that row, with rest of the cell data in the same column. It might have to be recursive, to make sure the report keeps splitting cells and adding rows until the data can be completely written.

@klcodanr
Copy link
Contributor

klcodanr commented Jan 7, 2020

@Appar-singh -- makes sense to me. I'm thinking I'll just have it repeat all of the data and add another status value (I'm thinking EXTENDED_DATA) so you can tell what tag it belongs to and know why the tag is repeated

@klcodanr
Copy link
Contributor

klcodanr commented Jan 7, 2020

@Appar-singh @badvision I updated the logic to create multiple rows to support overflow when the cell is too large. Note that they will have a different status as I indicated above so you can differentiate between the initial row and additional rows.

Please review here: #2154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants