Skip to content

XWIKI-2470: Improve saving to not save data that has not changed #4680

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 21, 2025
Merged

XWIKI-2470: Improve saving to not save data that has not changed #4680

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2bf06b
XWIKI-2470: Improve saving to not save data that has not changed
mflorea Oct 17, 2025
0575080
XWIKI-2470: Improve saving to not save data that has not changed
mflorea Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 85 additions & 57 deletions xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,9 @@ public class SaveAction extends EditAction
private static final Logger LOGGER = LoggerFactory.getLogger(SaveAction.class);

/**
* The key to retrieve the saved object version from the context.
* The key used to store the JSON answer for the save action on the XWiki context.
*/
private static final String SAVED_OBJECT_VERSION_KEY = "SaveAction.savedObjectVersion";

/**
* The context key to know if a document has been merged for saving it.
*/
private static final String MERGED_DOCUMENTS = "SaveAction.mergedDocuments";
static final String JSON_ANSWER_KEY = "SaveAction.jsonAnswer";

/**
* Parameter value used with forceSave to specify that the merge should be performed even if there is conflicts.
Expand Down Expand Up @@ -238,64 +233,97 @@ public boolean save(XWikiContext context) throws XWikiException
tdoc.readFromForm(form, context);
}

// Remove the redirect object if the save request doesn't update it. This allows users to easily overwrite
// redirect place-holders that are created when we move pages around.
if (tdoc.getXObject(RedirectClassDocumentInitializer.REFERENCE) != null
&& request.getParameter("XWiki.RedirectClass_0_location") == null) {
tdoc.removeXObjects(RedirectClassDocumentInitializer.REFERENCE);
}

// There are cases (e.g. when leaving a realtime collaboration session) when we don't want to create a new
// document revision unless there are actual changes. This is indicated by setting the preventEmptyRevision
// request parameter to true. Changing the document authors marks the document metadata as dirty, which is why
// we need to remember the dirty state before updating the authors, if we want to prevent empty revisions. Note
// that the document can be further modified by the save event listeners called below, but we expect them to set
// a version summary comment if they want to force the save even when preventEmptyRevision is true.
boolean dirtyBeforeSettingAuthors = tdoc.isContentDirty() || tdoc.isMetaDataDirty();

// Update the document authors.
UserReference currentUserReference = this.currentUserResolver.resolve(CurrentUserReference.INSTANCE);
tdoc.getAuthors().setOriginalMetadataAuthor(currentUserReference);
request.getEffectiveAuthor().ifPresent(tdoc.getAuthors()::setEffectiveMetadataAuthor);

if (tdoc.isNew()) {
tdoc.getAuthors().setCreator(currentUserReference);
}

// Make sure we have at least the meta data dirty status
tdoc.setMetaDataDirty(true);

// Validate the document if we have xvalidate=1 in the request
if ("1".equals(request.getParameter("xvalidate"))) {
boolean validationResult = tdoc.validate(context);
// If the validation fails we should show the "Inline form" edit mode
if (validationResult == false) {
// Set display context to 'edit'
context.put("display", "edit");
// Set the action used by the "Inline form" edit mode as the context action. See #render(XWikiContext).
context.setAction(tdoc.getDefaultEditMode(context));
// Set the document in the context
context.put("doc", doc);
context.put("cdoc", tdoc);
context.put("tdoc", tdoc);
// Force the "Inline form" edit mode.
getCurrentScriptContext().setAttribute("editor", "inline", ScriptContext.ENGINE_SCOPE);

return true;
}
// Validate the document if we have xvalidate=1 in the request.
if ("1".equals(request.getParameter("xvalidate")) && !tdoc.validate(context)) {
// Validation failed. Redirect to "Inline form" edit mode.
// Set display context to "edit".
context.put("display", "edit");
// Set the action used by the "Inline form" edit mode as the context action. See #render(XWikiContext).
context.setAction(tdoc.getDefaultEditMode(context));
// Set the document in the context.
context.put("doc", doc);
context.put("cdoc", tdoc);
context.put("tdoc", tdoc);
// Force the "Inline form" edit mode.
getCurrentScriptContext().setAttribute("editor", "inline", ScriptContext.ENGINE_SCOPE);
return true;
}

// Remove the redirect object if the save request doesn't update it. This allows users to easily overwrite
// redirect place-holders that are created when we move pages around.
if (tdoc.getXObject(RedirectClassDocumentInitializer.REFERENCE) != null
&& request.getParameter("XWiki.RedirectClass_0_location") == null) {
tdoc.removeXObjects(RedirectClassDocumentInitializer.REFERENCE);
}
// Prepare the JSON answer that will be sent to the editor in case of an async request. We put it on the XWiki
// context so that other parts of the code (e.g. event listeners) can add more information if needed.
Map<String, String> jsonAnswer = new LinkedHashMap<>();
context.put(JSON_ANSWER_KEY, jsonAnswer);

// We only proceed on the check between versions in case of AJAX request, so we currently stay in the edit form
// This can be improved later by displaying a nice UI with some merge options in a sync request.
// For now we don't want our user to loose their changes.
if (isConflictCheckEnabled() && Utils.isAjaxRequest(context)
&& request.getParameter("previousVersion") != null) {
if (isConflictingWithVersion(context, originalDoc, tdoc)) {
return true;
}
// Detect merge conflicts. We do this only if the previous version is known, otherwise a 3-way merge is not
// possible, and the save request is asynchronous, i.e. the user is waiting in edit mode for the save result. If
// a merge conflict is detected the editor will display the merge conflict modal.
if (isConflictCheckEnabled() && Utils.isAjaxRequest(context) && request.getParameter("previousVersion") != null
&& isConflictingWithVersion(context, originalDoc, tdoc)) {
return true;
}

// Make sure the user is allowed to make this modification
// Make sure the user is allowed to save the document. This triggers an event whose listeners can block (cancel)
// the save if needed. We trigger the event even if the document is not dirty (i.e. doesn't need to be saved)
// because:
// * the listeners can change the document (i.e. even if the document is not dirty now, it may become dirty
// after the event listeners are called)
// * the listeners may want to block a hierarchy template from being applied, even if the root document is not
// itself affected.
xwiki.checkSavingDocument(context.getUserReference(), tdoc, tdoc.getComment(), tdoc.isMinorEdit(), context);

// We get the comment to be used from the document
// It was read using readFromForm
xwiki.saveDocument(tdoc, tdoc.getComment(), tdoc.isMinorEdit(), context);
this.temporaryAttachmentSessionsManager.removeUploadedAttachments(tdoc.getDocumentReference());
// Note that users may save an existing document without making any changes just to update the author, e.g. to
// change access rights. In this or other similar cases the version summary comment can be used to justify the
// action, which is why we force the save if a comment is provided. We consider preventEmptyRevision to be false
// by default for backward compatibility. This is currently used by realtime collaboration to avoid creating
// empty revisions when leaving the editing session.
if (tdoc.isNew() || dirtyBeforeSettingAuthors || StringUtils.isNotEmpty(tdoc.getComment())
|| !"true".equals(request.getParameter("preventEmptyRevision"))) {
// Make sure we have at least the meta data dirty otherwise the version is not incremented.
tdoc.setMetaDataDirty(true);

// We take the version summary comment from the document being saved because it may have been modified by an
// event listener after it was copied from the EditForm by the call to readFromForm.
xwiki.saveDocument(tdoc, tdoc.getComment(), tdoc.isMinorEdit(), context);

// Return the new version number to the editor that triggered the save.
jsonAnswer.put("newVersion", tdoc.getRCSVersion().toString());
} else {
// Let the editor know that the document was not saved because there were no changes.
jsonAnswer.put("noChanges", "true");
}

context.put(SAVED_OBJECT_VERSION_KEY, tdoc.getRCSVersion());
// Cleanup temporary attachments that were uploaded while editing the document. We do this even if we didn't
// create a new revision (e.g. if there were no changes) because those attachments are not needed anymore (e.g.
// the user may have discarded the changes that lead to the upload of those attachments).
this.temporaryAttachmentSessionsManager.removeUploadedAttachments(tdoc.getDocumentReference());

// If a template hierarchy is specified we start a job to copy the child pages. Even if we didn't create a new
// revision for the root document (because there were no changes) the child pages may still need to be created
// or updated, because the template can be applied on existing documents, even on documents where the template
// was previously applied.
Job createJob = startCreateJob(tdoc.getDocumentReference(), form);
if (createJob != null) {
if (isAsync(request)) {
Expand Down Expand Up @@ -479,7 +507,7 @@ private boolean isConflictingWithVersion(XWikiContext context, XWikiDocument ori
// then we pursue to save the document.
if (FORCE_SAVE_MERGE.equals(request.getParameter("forceSave"))
|| !mergeDocumentResult.hasConflicts()) {
context.put(MERGED_DOCUMENTS, "true");
getJSONAnswer(context).put("mergedDocument", "true");
return false;

// If we got merge conflicts and we don't want to force it, then we record the conflict in
Expand Down Expand Up @@ -509,6 +537,12 @@ private boolean isConflictingWithVersion(XWikiContext context, XWikiDocument ori
return false;
}

@SuppressWarnings("unchecked")
Map<String, String> getJSONAnswer(XWikiContext context)
{
return (Map<String, String>) context.get(JSON_ANSWER_KEY);
}

@Override
public boolean action(XWikiContext context) throws XWikiException
{
Expand All @@ -529,13 +563,7 @@ public boolean action(XWikiContext context) throws XWikiException

// forward to view
if (isAjaxRequest) {
Map<String, String> jsonAnswer = new LinkedHashMap<>();
Version newVersion = (Version) context.get(SAVED_OBJECT_VERSION_KEY);
jsonAnswer.put("newVersion", newVersion.toString());
if ("true".equals(context.get(MERGED_DOCUMENTS))) {
jsonAnswer.put("mergedDocument", "true");
}
answerJSON(context, HttpStatus.SC_OK, jsonAnswer);
answerJSON(context, HttpStatus.SC_OK, getJSONAnswer(context));
} else {
sendRedirect(context.getResponse(), Utils.getRedirect("view", context));
}
Expand Down
25 changes: 4 additions & 21 deletions ...rm-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/web/SaveAndContinueAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package com.xpn.xwiki.web;

import java.io.IOException;
import java.util.LinkedHashMap;
import java.util.Map;

import javax.inject.Inject;
Expand All @@ -32,7 +31,6 @@
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.suigeneris.jrcs.rcs.Version;
import org.xwiki.component.annotation.Component;

import com.xpn.xwiki.XWikiContext;
Expand All @@ -53,16 +51,6 @@ public class SaveAndContinueAction extends XWikiAction
/** Key for storing the wrapped action in the context. */
private static final String WRAPPED_ACTION_CONTEXT_KEY = "SaveAndContinueAction.wrappedAction";

/**
* The key to retrieve the saved object version from the context.
*/
private static final String SAVED_OBJECT_VERSION_KEY = "SaveAction.savedObjectVersion";

/**
* The context key to know if a document has been merged for saving it.
*/
private static final String MERGED_DOCUMENTS = "SaveAction.mergedDocuments";

/**
* The default context value to put with {@link #MERGED_DOCUMENTS} key.
*/
Expand Down Expand Up @@ -175,15 +163,10 @@ public boolean action(XWikiContext context) throws XWikiException

// If this is an ajax request, no need to redirect.
if (isAjaxRequest) {
Version newVersion = (Version) context.get(SAVED_OBJECT_VERSION_KEY);

// in case of property update, SaveAction has not been called, so we don't get the new version.
if (newVersion != null) {
Map<String, String> jsonAnswer = new LinkedHashMap<>();
jsonAnswer.put("newVersion", newVersion.toString());
if (MERGED_DOCUMENTS_VALUE.equals(context.get(MERGED_DOCUMENTS))) {
jsonAnswer.put("mergedDocument", MERGED_DOCUMENTS_VALUE);
}
@SuppressWarnings("unchecked")
Map<String, String> jsonAnswer = (Map<String, String>) context.get(SaveAction.JSON_ANSWER_KEY);
// In case of property update, SaveAction is not called, so there's no JSON answer.
if (jsonAnswer != null) {
answerJSON(context, HttpStatus.SC_OK, jsonAnswer);
} else {
context.getResponse().setStatus(HttpServletResponse.SC_NO_CONTENT);
Expand Down
11 changes: 7 additions & 4 deletions ...-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/web/SaveActionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import javax.inject.Named;
Expand Down Expand Up @@ -197,7 +198,7 @@ void validSave() throws Exception
when(mockForm.getTemplate()).thenReturn("");

assertFalse(saveAction.save(this.context));
assertEquals(new Version("1.2"), this.context.get("SaveAction.savedObjectVersion"));
assertEquals(Map.of("newVersion", "1.2"), saveAction.getJSONAnswer(context));

verify(mockAuthors).setOriginalMetadataAuthor(this.currentUserReference);
verify(mockAuthors).setEffectiveMetadataAuthor(this.effectiveAuthor);
Expand All @@ -220,7 +221,7 @@ void validSaveNewTranslation() throws Exception
when(mockRequest.getParameter("previousVersion")).thenReturn("1.1");
when(mockRequest.getParameter("isNew")).thenReturn("true");
assertFalse(saveAction.save(this.context));
assertEquals(new Version("1.1"), this.context.get("SaveAction.savedObjectVersion"));
assertEquals(Map.of("newVersion", "1.1"), saveAction.getJSONAnswer(context));
verify(this.xWiki).checkSavingDocument(eq(USER_REFERENCE), any(XWikiDocument.class), eq(""), eq(false),
eq(this.context));
verify(this.xWiki).saveDocument(any(XWikiDocument.class), eq(""), eq(false), eq(this.context));
Expand All @@ -244,7 +245,7 @@ void validSaveOldTranslation() throws Exception
when(mockClonedDocument.getRCSVersion()).thenReturn(new Version("1.4"));
when(mockClonedDocument.getComment()).thenReturn("My Changes");
assertFalse(saveAction.save(this.context));
assertEquals(new Version("1.4"), this.context.get("SaveAction.savedObjectVersion"));
assertEquals(Map.of("newVersion", "1.4"), saveAction.getJSONAnswer(context));
verify(this.xWiki).checkSavingDocument(USER_REFERENCE, mockClonedDocument, "My Changes", false, this.context);
verify(this.xWiki).saveDocument(mockClonedDocument, "My Changes", false, this.context);
}
Expand Down Expand Up @@ -275,7 +276,7 @@ void validSaveRequestImageUploadAndConflictCheck() throws Exception
when(mockDocument.getObjectDiff("1.1", "1.2", context)).thenReturn(Collections.emptyList());

assertFalse(saveAction.save(this.context));
assertEquals(new Version("1.2"), this.context.get("SaveAction.savedObjectVersion"));
assertEquals(Map.of("newVersion", "1.2"), saveAction.getJSONAnswer(context));

verify(mockAuthors).setOriginalMetadataAuthor(this.currentUserReference);
verify(mockAuthors).setEffectiveMetadataAuthor(this.effectiveAuthor);
Expand All @@ -288,6 +289,7 @@ void validSaveRequestImageUploadAndConflictCheck() throws Exception
@Test
void saveFromTemplate() throws Exception
{
when(mockClonedDocument.getRCSVersion()).thenReturn(new Version("3.2"));
when(this.mockForm.getTemplate()).thenReturn("TemplateSpace.TemplateDocument");
DocumentReference templateReference =
new DocumentReference(context.getWikiId(), "TemplateSpace", "TemplateDocument");
Expand Down Expand Up @@ -329,6 +331,7 @@ void saveSectionWithAttachmentUpload() throws Exception
String comment = "Some comment";
when(sectionDoc.getComment()).thenReturn(comment);
when(sectionDoc.isMinorEdit()).thenReturn(true);
when(mockClonedDocument.getRCSVersion()).thenReturn(new Version("4.1"));

assertFalse(this.saveAction.save(this.context));

Expand Down
4 changes: 2 additions & 2 deletions .../org/xwiki/realtime/test/po/Coeditor.java → ...iki/realtime/test/po/CoeditorElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
* @since 16.10.6
* @since 17.3.0RC1
*/
public class Coeditor extends BaseElement
public class CoeditorElement extends BaseElement
{
private WebElement container;

Expand All @@ -39,7 +39,7 @@ public class Coeditor extends BaseElement
*
* @param container the WebElement used to display the coeditor
*/
public Coeditor(WebElement container)
public CoeditorElement(WebElement container)
{
this.container = container;
}
Expand Down
8 changes: 4 additions & 4 deletions ...realtime-test-pageobjects/src/main/java/org/xwiki/realtime/test/po/CoeditorsDropdown.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,19 @@ private WebElement getToggle()
/**
* @return the list of users that are currently editing the page in realtime
*/
public List<Coeditor> getCoeditors()
public List<CoeditorElement> getCoeditors()
{
return getDriver().findElements(By.cssSelector(".realtime-users-dropdown .realtime-user")).stream()
.map(Coeditor::new).toList();
.map(CoeditorElement::new).toList();
}

/**
* @param coeditorId the coeditor identifier
* @return the coeditor with the specified identifier
*/
public Coeditor getCoeditor(String coeditorId)
public CoeditorElement getCoeditor(String coeditorId)
{
return new Coeditor(
return new CoeditorElement(
getDriver().findElement(By.cssSelector(".realtime-users-dropdown[data-id='" + coeditorId + "']")));
}
}
Loading