Skip to content

fix: avoid recursive hashCode in write holder contexts#911

Open
skytin1004 wants to merge 1 commit intoapache:mainfrom
skytin1004:fix/889-write-holder-hashcode-recursion
Open

fix: avoid recursive hashCode in write holder contexts#911
skytin1004 wants to merge 1 commit intoapache:mainfrom
skytin1004:fix/889-write-holder-hashcode-recursion

Conversation

@skytin1004
Copy link
Copy Markdown
Contributor

@skytin1004 skytin1004 commented May 7, 2026

Purpose of the pull request

Related: #889

This PR addresses only the recursive hashCode() failure path discussed in #889 and #899.

While rechecking the original failure, I found that the custom converter reproduction appears to involve at least two separate concerns:

  1. recursive hashCode() evaluation in write holder/context objects
  2. global write converter lookup when a registered converter declares a non-null supportExcelTypeKey()

To keep the review focused, this PR deliberately handles only the first issue.

I reproduced the recursive hashCode() paths locally and added a focused regression test for them:
The regression test creates the recursive holder/context references directly and then calls hashCode(), so it covers this PR's failure path without depending on converter behavior.

  • WriteWorkbookHolder.hashCode() -> WorkbookWriteHandlerContext.hashCode() -> WriteWorkbookHolder.hashCode()
  • WriteWorkbookHolder.hashCode() -> initialized sheet holder map -> WriteSheetHolder.hashCode() -> parent workbook holder -> WriteWorkbookHolder.hashCode()

The fix excludes only the recursive runtime references from Lombok-generated equals/hashCode:

  • WriteWorkbookHolder.workbookWriteHandlerContext
  • WriteSheetHolder.parentWriteWorkbookHolder
  • WriteSheetHolder.hasBeenInitializedTable
  • WriteTableHolder.parentWriteSheetHolder

This PR intentionally does not change converter lookup behavior. I plan to handle that separately with a smaller follow-up test/fix, because it can fail independently when a global converter is registered with a non-null supportExcelTypeKey().

I verified the change locally by running:

  • the new WriteHolderHashCodeTest
  • the existing CustomConverterTest
  • the full fesod-sheet test suite

All tests passed.

Note

If this recursive hashCode() fix looks good and gets merged, I can follow up with a smaller PR focused only on the global write converter lookup case, so that the remaining part of #889 can be addressed clearly.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@skytin1004 skytin1004 marked this pull request as draft May 7, 2026 14:32
@skytin1004 skytin1004 marked this pull request as ready for review May 7, 2026 14:42
@skytin1004 skytin1004 force-pushed the fix/889-write-holder-hashcode-recursion branch from 269bfe5 to a448c15 Compare May 7, 2026 14:45
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

Successfully merging this pull request may close these issues.

1 participant