From 6e96334f2fdb2dfaa77b52905d605fa4221cda96 Mon Sep 17 00:00:00 2001 From: Thomas Mortagne Date: Tue, 17 Sep 2024 04:18:25 +0200 Subject: [PATCH] XWIKI-22511: The cluster can lead to an inconsistent state of the cached XWikiDocument instance --- .../AbstractXWikiEventConverter.java | 7 ++- .../converter/DocumentEventConverterTest.java | 57 ++++++++++++++++++- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/observation/remote/converter/AbstractXWikiEventConverter.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/observation/remote/converter/AbstractXWikiEventConverter.java index 1262b0d693e1..49a59921d718 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/observation/remote/converter/AbstractXWikiEventConverter.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/observation/remote/converter/AbstractXWikiEventConverter.java @@ -191,7 +191,7 @@ protected XWikiDocument unserializeDocument(Serializable remoteData) throws XWik XWikiDocument document = new XWikiDocument(docReference, locale); XWikiDocument origDoc = new XWikiDocument(docReference, origLocale); - // Force invalidating the cache to be sure it return (and keep) the right document + // Force invalidating the cache to be sure it returns (and keep) the right document if (xcontext.getWiki().getStore() instanceof XWikiCacheStore) { ((XWikiCacheStore) xcontext.getWiki().getStore()).invalidate(document); ((XWikiCacheStore) xcontext.getWiki().getStore()).invalidate(origDoc); @@ -207,6 +207,11 @@ protected XWikiDocument unserializeDocument(Serializable remoteData) throws XWik origDoc = getDocument(origDoc, origVersion, xcontext); } + // Clone the document to be sure we don't leave an inconsistent document state in the cache + // TODO: optimize when https://jira.xwiki.org/browse/XWIKI-22510 is fixed + document = document.clone(); + + // Set the original document expected by listeners (so that they can know what changed) document.setOriginalDocument(origDoc); return document; diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/internal/observation/remote/converter/DocumentEventConverterTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/internal/observation/remote/converter/DocumentEventConverterTest.java index a02536357c60..284545ce303b 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/internal/observation/remote/converter/DocumentEventConverterTest.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/internal/observation/remote/converter/DocumentEventConverterTest.java @@ -43,6 +43,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -63,14 +64,62 @@ class DocumentEventConverterTest @InjectComponentManager private MockitoComponentManager componentManager; + @Test + void testConvertCreatedDocument() throws Exception + { + DocumentReference documentReference = new DocumentReference("wiki", "space", "page"); + + // Setup of document which just been created and recived by a document event listener + XWikiDocument document = + this.oldcore.getSpyXWiki().getDocument(documentReference, this.oldcore.getXWikiContext()); + this.oldcore.getSpyXWiki().saveDocument(document, this.oldcore.getXWikiContext()); + document = document.clone(); + document.setOriginalDocument(new XWikiDocument(documentReference)); + + // local -> remote + + LocalEventData localEvent = new LocalEventData(); + localEvent.setEvent(new DocumentUpdatedEvent()); + localEvent.setSource(document); + localEvent.setData(this.oldcore.getXWikiContext()); + + RemoteEventData remoteEvent = this.converterManager.createRemoteEventData(localEvent); + + assertFalse(remoteEvent.getSource() instanceof XWikiDocument); + assertFalse(remoteEvent.getData() instanceof XWikiContext); + + // serialize/unserialize + ByteArrayOutputStream sos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream(sos); + oos.writeObject(remoteEvent); + ByteArrayInputStream sis = new ByteArrayInputStream(sos.toByteArray()); + ObjectInputStream ois = new ObjectInputStream(sis); + remoteEvent = (RemoteEventData) ois.readObject(); + + // remote -> local + + LocalEventData localEvent2 = this.converterManager.createLocalEventData(remoteEvent); + + assertTrue(localEvent2.getSource() instanceof XWikiDocument); + assertTrue(localEvent2.getData() instanceof XWikiContext); + assertEquals(documentReference, ((XWikiDocument) localEvent2.getSource()).getDocumentReference()); + assertEquals("space", ((XWikiDocument) localEvent2.getSource()).getSpaceName()); + assertEquals("page", ((XWikiDocument) localEvent2.getSource()).getPageName()); + assertTrue(((XWikiDocument) localEvent2.getSource()).getOriginalDocument().isNew()); + assertNotSame(this.oldcore.getSpyXWiki().getDocument(documentReference, this.oldcore.getXWikiContext()), + localEvent2.getSource()); + } + @Test void testConvertWithOriginalDocNull() throws Exception { + DocumentReference documentReference = new DocumentReference("wiki", "space", "page"); + // local -> remote LocalEventData localEvent = new LocalEventData(); - localEvent.setEvent(new DocumentUpdatedEvent(new DocumentReference("wiki", "space", "page"))); - localEvent.setSource(new XWikiDocument(new DocumentReference("wiki", "space", "page"))); + localEvent.setEvent(new DocumentUpdatedEvent()); + localEvent.setSource(new XWikiDocument(documentReference)); localEvent.setData(this.oldcore.getXWikiContext()); RemoteEventData remoteEvent = this.converterManager.createRemoteEventData(localEvent); @@ -92,9 +141,11 @@ void testConvertWithOriginalDocNull() throws Exception assertTrue(localEvent2.getSource() instanceof XWikiDocument); assertTrue(localEvent2.getData() instanceof XWikiContext); - assertEquals("wiki", ((XWikiDocument) localEvent2.getSource()).getWikiName()); + assertEquals(documentReference, ((XWikiDocument) localEvent2.getSource()).getDocumentReference()); assertEquals("space", ((XWikiDocument) localEvent2.getSource()).getSpaceName()); assertEquals("page", ((XWikiDocument) localEvent2.getSource()).getPageName()); assertTrue(((XWikiDocument) localEvent2.getSource()).getOriginalDocument().isNew()); + assertNotSame(this.oldcore.getSpyXWiki().getDocument(documentReference, this.oldcore.getXWikiContext()), + localEvent2.getSource()); } }