Skip to content

XWIKI-24268: Hibernate attachment versioning store loses the initial 1.1 version when an attachment is first updated#5398

Open
vmassol wants to merge 2 commits intomasterfrom
XWIKI-24268
Open

XWIKI-24268: Hibernate attachment versioning store loses the initial 1.1 version when an attachment is first updated#5398
vmassol wants to merge 2 commits intomasterfrom
XWIKI-24268

Conversation

@vmassol
Copy link
Copy Markdown
Member

@vmassol vmassol commented Apr 20, 2026

Jira URL

https://jira.xwiki.org/browse/XWIKI-24268

Changes

See https://jira.xwiki.org/browse/XWIKI-24268

Executed Tests

  • New unit test
  • Manually tested by adding twice the same attachment and clicking on the revision link. Verify the 2 attachment revisions can be seen. All this after configuring XWiki as mentioned on the forum, i.e.:
xwiki.store.recyclebin.content.hint=hibernate
xwiki.store.attachment.hint=hibernate
xwiki.store.attachment.versioning.hint=hibernate
xwiki.store.attachment.recyclebin.content.hint=hibernate

xwiki.store.main.hint=hibernate
xwiki.store.versioning.hint=hibernate
xwiki.store.recyclebin.hint=hibernate
xwiki.store.attachment.recyclebin.hint=hibernate
xwiki.store.migration.manager.hint=hibernate

xwiki.recyclebin=1
storage.attachment.recyclebin=1
xwiki.store.versioning=1
xwiki.store.attachment.versioning=1
xwiki.store.rollbackattachmentwithdocuments=1

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • To be defined, ideally on all branches if the fix is ok

…1.1 version when an attachment is first updated
@vmassol vmassol requested a review from tmortagne April 20, 2026 09:42
Comment on lines +71 to +72
@SuppressWarnings("unchecked")
Query<Long> mockQuery = mock(Query.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@SuppressWarnings("unchecked")
Query<Long> mockQuery = mock(Query.class);
Query<Long> mockQuery = mock();

Shorter and doesn't trigger any warnings. It's more recent than most code that the coding agents are trained on I fear so they prefer the old version. It makes sense to have an explicit instruction like

Prefer Mockito methods with empty parameters like mock() instead of mock(Class.class) and
ArgumentCaptor.captor() instead of ArgumentCaptor.forClass(Class.class).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, it's not just the LLM that didn't know about it :) I didn't either. It's nice.

…1.1 version when an attachment is first updated

* Improved test thx to Michael
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.

2 participants