From 081a3608e7d23d13edae64f9c32fa548645cc2d4 Mon Sep 17 00:00:00 2001 From: Thomas Mortagne Date: Tue, 14 Jan 2025 15:42:26 +0100 Subject: [PATCH] XWIKI-22782: Only save modified xobjects --- .../hibernate/HibernateConfiguration.java | 25 ++++++++ .../com/xpn/xwiki/objects/BaseCollection.java | 15 +++-- .../com/xpn/xwiki/objects/BaseElement.java | 64 ++++++++++++++++--- .../com/xpn/xwiki/objects/BaseObject.java | 5 +- .../com/xpn/xwiki/objects/BaseProperty.java | 55 ++++++++-------- .../xpn/xwiki/objects/classes/BaseClass.java | 24 +------ .../xwiki/objects/classes/PropertyClass.java | 3 + .../xpn/xwiki/store/XWikiHibernateStore.java | 42 ++++++++---- 8 files changed, 152 insertions(+), 81 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/store/hibernate/HibernateConfiguration.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/store/hibernate/HibernateConfiguration.java index 59ba176fc798..478f673dcc0c 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/store/hibernate/HibernateConfiguration.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/store/hibernate/HibernateConfiguration.java @@ -21,6 +21,8 @@ import java.util.Arrays; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.inject.Named; @@ -29,6 +31,9 @@ import org.apache.commons.lang3.StringUtils; import org.xwiki.component.annotation.Component; import org.xwiki.configuration.ConfigurationSource; +import org.xwiki.model.EntityType; +import org.xwiki.model.reference.EntityReference; +import org.xwiki.model.reference.EntityReferenceResolver; import com.xpn.xwiki.internal.XWikiCfgConfigurationSource; @@ -46,6 +51,10 @@ public class HibernateConfiguration @Named(XWikiCfgConfigurationSource.ROLEHINT) private ConfigurationSource xwikiConfiguration; + @Inject + @Named("relative") + private EntityReferenceResolver resolver; + private String path; /** @@ -164,4 +173,20 @@ public List getIgnoredMigrations() { return getList("xwiki.store.migration.ignored"); } + + /** + * @return the local references of the classes for which we should apply a save optimization (save only the modified + * ones) + * @since 17.1.0RC1 + * @since 16.10.3 + */ + public Set getOptimizedXObjectClasses() + { + List references = + this.xwikiConfiguration.getProperty("xwiki.store.hibernate.optimizedObjectSave.classes", List.class); + + return references != null + ? references.stream().map(r -> this.resolver.resolve(r, EntityType.DOCUMENT)).collect(Collectors.toSet()) + : null; + } } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseCollection.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseCollection.java index adc3a54fa5b1..6438560be232 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseCollection.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseCollection.java @@ -264,10 +264,6 @@ public PropertyInterface get(String name) throws XWikiException public void safeput(String name, PropertyInterface property) { addField(name, property); - if (property instanceof BaseProperty) { - ((BaseProperty) property).setObject(this); - ((BaseProperty) property).setName(name); - } } @Override @@ -537,8 +533,13 @@ public void addField(String name, PropertyInterface element) { this.fields.put(name, element); - if (element instanceof BaseElement) { - ((BaseElement) element).setOwnerDocument(getOwnerDocument()); + element.setName(name); + if (element instanceof BaseElement baseElement) { + baseElement.setOwnerDocument(getOwnerDocument()); + + if (element instanceof BaseProperty baseProperty) { + baseProperty.setObject(this); + } } } @@ -647,6 +648,8 @@ public BaseCollection clone() } collection.setFields(cfields); + collection.setDirty(isDirty()); + return collection; } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseElement.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseElement.java index c7887316cd3d..84eacfdf70d0 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseElement.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseElement.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.Serializable; import java.io.StringWriter; +import java.util.Objects; import javax.inject.Provider; @@ -37,6 +38,7 @@ import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.EntityReference; import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.stability.Unstable; import org.xwiki.store.merge.MergeManager; import org.xwiki.store.merge.MergeManagerResult; @@ -101,7 +103,35 @@ public abstract class BaseElement implements ElementI private EntityReferenceSerializer localUidStringEntityReferenceSerializer; private ContextualLocalizationManager localization; - + + private transient boolean dirty = true; + + /** + * @return true of the element was modified (or created) + * @since 17.1.0RC1 + * @since 16.10.3 + */ + @Unstable + public boolean isDirty() + { + return this.dirty; + } + + /** + * @param dirty true of the element was modified (or created) + * @since 17.1.0RC1 + * @since 16.10.3 + */ + @Unstable + public void setDirty(boolean dirty) + { + this.dirty = dirty; + + if (dirty && this.ownerDocument != null) { + this.ownerDocument.setMetaDataDirty(true); + } + } + /** * @return a merge manager instance. * @since 11.8RC1 @@ -161,12 +191,16 @@ public String getName() } @Override - public void setDocumentReference(DocumentReference reference) + public void setDocumentReference(DocumentReference documentReference) { - // If the name is already set then reset it since we're now using a reference - this.documentReference = reference; - this.name = null; - this.referenceCache = null; + if (!Objects.equals(documentReference, this.documentReference)) { + // If the name is already set then reset it since we're now using a reference + this.documentReference = documentReference; + this.name = null; + this.referenceCache = null; + + setDirty(true); + } } /** @@ -185,8 +219,12 @@ public void setName(String name) throw new IllegalStateException("BaseElement#setName could not be called when a reference has been set."); } - this.name = name; - this.referenceCache = null; + if (!StringUtils.equals(name, this.name)) { + this.name = name; + this.referenceCache = null; + + setDirty(true); + } } public String getPrettyName() @@ -349,6 +387,8 @@ public BaseElement clone() } element.setPrettyName(getPrettyName()); + + element.dirty = this.dirty; } catch (Exception e) { // This should not happen element = null; @@ -417,7 +457,13 @@ public boolean apply(ElementInterface newElement, boolean clean) */ public void setOwnerDocument(XWikiDocument ownerDocument) { - this.ownerDocument = ownerDocument; + if (this.ownerDocument != ownerDocument) { + this.ownerDocument = ownerDocument; + + if (this.ownerDocument != null && isDirty()) { + this.ownerDocument.setMetaDataDirty(true); + } + } } /** diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseObject.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseObject.java index a837d4910d30..954cc005f1f3 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseObject.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseObject.java @@ -19,7 +19,6 @@ */ package com.xpn.xwiki.objects; -import java.io.Serializable; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -44,7 +43,7 @@ import com.xpn.xwiki.objects.classes.PropertyClass; import com.xpn.xwiki.web.Utils; -public class BaseObject extends BaseCollection implements ObjectInterface, Serializable, Cloneable +public class BaseObject extends BaseCollection implements ObjectInterface, Cloneable { private static final long serialVersionUID = 1L; @@ -201,6 +200,8 @@ public BaseObject clone() // is expensive) object.setGuid(this.guid); + object.setDirty(isDirty()); + return object; } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseProperty.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseProperty.java index a10f4606ca63..4cebac6ce38d 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseProperty.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/BaseProperty.java @@ -19,7 +19,6 @@ */ package com.xpn.xwiki.objects; -import java.io.Serializable; import java.util.Objects; import org.apache.commons.lang3.ObjectUtils; @@ -42,8 +41,7 @@ */ // TODO: shouldn't this be abstract? toFormString and toText // will never work unless getValue is overriden -public class BaseProperty extends BaseElement - implements PropertyInterface, Serializable, Cloneable +public class BaseProperty extends BaseElement implements PropertyInterface, Cloneable { private static final long serialVersionUID = 1L; @@ -54,11 +52,6 @@ public class BaseProperty extends BaseElement private long id; - /** - * Set to true if value is not the same as the database value. - */ - private boolean isValueDirty = true; - @Override protected R createReference() { @@ -82,6 +75,10 @@ public BaseCollection getObject() public void setObject(BaseCollection object) { this.object = object; + + if (this.object != null && isDirty()) { + this.object.setDirty(true); + } } @Override @@ -124,9 +121,13 @@ public long getId() @Override public void setId(long id) { - // I hate this.. needed for hibernate to find the object - // when loading the collections.. - this.id = id; + if (id != this.id) { + // I hate this.. needed for hibernate to find the object + // when loading the collections.. + this.id = id; + + setDirty(true); + } } @Override @@ -155,11 +156,12 @@ public BaseProperty clone() cloneInternal(property); - property.isValueDirty = this.isValueDirty; property.ownerDocument = this.ownerDocument; property.setObject(getObject()); + property.setDirty(isDirty()); + return property; } @@ -347,10 +349,12 @@ public boolean apply(ElementInterface newProperty, boolean clean) /** * @return {@literal true} if the property value doesn't match the value in the database. * @since 4.3M2 + * @deprecated use {@link #isDirty()} instead */ + @Deprecated(since = "17.1.0RC1") public boolean isValueDirty() { - return this.isValueDirty; + return isDirty(); } /** @@ -360,7 +364,7 @@ public boolean isValueDirty() */ protected void setValueDirty(Object newValue) { - if (!this.isValueDirty && !Objects.equals(newValue, getValue())) { + if (!isDirty() && !Objects.equals(newValue, getValue())) { setValueDirty(true); } } @@ -368,30 +372,21 @@ protected void setValueDirty(Object newValue) /** * @param valueDirty Indicate if the dirty flag should be set or cleared. * @since 4.3M2 + * @deprecated use {@link #setDirty(boolean)} instead */ + @Deprecated(since = "17.1.0RC1") public void setValueDirty(boolean valueDirty) { - this.isValueDirty = valueDirty; - if (valueDirty && this.ownerDocument != null) { - this.ownerDocument.setMetaDataDirty(true); - } + setDirty(valueDirty); } - /** - * Set the owner document of this base property. - * - * @param ownerDocument The owner document. - * @since 4.3M2 - */ @Override - public void setOwnerDocument(XWikiDocument ownerDocument) + public void setDirty(boolean dirty) { - if (this.ownerDocument != ownerDocument) { - super.setOwnerDocument(ownerDocument); + super.setDirty(dirty); - if (ownerDocument != null && this.isValueDirty) { - ownerDocument.setMetaDataDirty(true); - } + if (dirty && this.object != null) { + this.object.setDirty(true); } } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/BaseClass.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/BaseClass.java index 340f2063fbeb..f6c8eea77108 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/BaseClass.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/BaseClass.java @@ -85,11 +85,6 @@ public class BaseClass extends BaseCollection implements Clas private String nameField; - /** - * Set to true if the class is modified from the database version of it. - */ - private boolean isDirty = true; - /** * Used to resolve a string into a proper Document Reference using the current document's reference to fill the * blanks, except for the page name for which the default page name is used instead and for the wiki name for which @@ -464,9 +459,10 @@ public BaseClass clone() bclass.setValidationScript(getValidationScript()); bclass.setNameField(getNameField()); - bclass.setDirty(this.isDirty); bclass.setOwnerDocument(this.ownerDocument); + bclass.setDirty(isDirty()); + return bclass; } @@ -1639,22 +1635,6 @@ public void setOwnerDocument(XWikiDocument ownerDocument) if (this.ownerDocument != null) { setDocumentReference(this.ownerDocument.getDocumentReference()); } - - if (ownerDocument != null && this.isDirty) { - ownerDocument.setMetaDataDirty(true); - } - } - } - - /** - * @param isDirty Indicate if the dirty flag should be set or cleared. - * @since 4.3M2 - */ - public void setDirty(boolean isDirty) - { - this.isDirty = isDirty; - if (isDirty && this.ownerDocument != null) { - this.ownerDocument.setMetaDataDirty(true); } } } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/PropertyClass.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/PropertyClass.java index 73461d6e96ed..25efe4a06ca0 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/PropertyClass.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/objects/classes/PropertyClass.java @@ -562,6 +562,9 @@ public PropertyClass clone() { PropertyClass pclass = (PropertyClass) super.clone(); pclass.setObject(getObject()); + + pclass.setDirty(isDirty()); + return pclass; } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateStore.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateStore.java index c69235f3f5d0..e1ef96deec6d 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateStore.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateStore.java @@ -205,6 +205,8 @@ public class XWikiHibernateStore extends XWikiHibernateBaseStore implements XWik */ private final Map spaceSavingLockMap = Collections.synchronizedMap(new ReferenceMap<>()); + private Set optimizedObjectClasses; + /** * This allows to initialize our storage engine. The hibernate config file path is taken from xwiki.cfg or directly * in the WEB-INF directory. @@ -255,6 +257,8 @@ public XWikiHibernateStore() public void initialize() throws InitializationException { this.registerLogoutListener(); + + this.optimizedObjectClasses = this.hibernateConfiguration.getOptimizedXObjectClasses(); } /** @@ -710,15 +714,24 @@ public void saveXWikiDoc(XWikiDocument doc, XWikiContext inputxcontext, boolean if (doc.hasElement(XWikiDocument.HAS_OBJECTS)) { // TODO: Delete all objects for which we don't have a name in the Map - for (List objects : doc.getXObjects().values()) { - for (BaseObject obj : objects) { - if (obj != null) { - obj.setDocumentReference(doc.getDocumentReference()); - /* If the object doesn't have a GUID, create it before saving */ - if (StringUtils.isEmpty(obj.getGuid())) { - obj.setGuid(null); + for (Map.Entry> entry : doc.getXObjects().entrySet()) { + List objects = entry.getValue(); + if (!objects.isEmpty()) { + // If the document is new, it does not make any sense to skip an object + // Get from the configuration the xobject classes on which to apply save optimization + boolean optimizedObjects = + !doc.isNew() && (this.optimizedObjectClasses == null || this.optimizedObjectClasses + .contains(entry.getKey().getLocalDocumentReference())); + for (BaseObject obj : objects) { + // Only save modified (or new) objects + if (obj != null && (!optimizedObjects || obj.isDirty())) { + obj.setDocumentReference(doc.getDocumentReference()); + /* If the object doesn't have a GUID, create it before saving */ + if (StringUtils.isEmpty(obj.getGuid())) { + obj.setGuid(null); + } + saveXWikiCollection(obj, context, false); } - saveXWikiCollection(obj, context, false); } } } @@ -1132,6 +1145,8 @@ public XWikiDocument loadXWikiDoc(XWikiDocument defaultDocument, XWikiContext in loadXWikiCollectionInternal(object, doc, context, false, true); } doc.setXObject(object.getNumber(), object); + // The object just been loaded so make sure it's considered clean + object.setDirty(false); } // AFAICT this was added as an emergency patch because loading of objects has proven @@ -1465,6 +1480,8 @@ public void saveXWikiCollection(BaseCollection object, XWikiContext inputxcontex } } + object.setDirty(false); + if (bTransaction) { endTransaction(context, true); } @@ -1628,6 +1645,8 @@ private void loadXWikiCollectionInternal(BaseCollection object1, XWikiDocument d } object.addField(name, property); + // The property just been loaded so make sure it's considered clean + property.setDirty(false); } } @@ -1752,15 +1771,14 @@ private void loadXWikiProperty(PropertyInterface property, XWikiContext context, try { session.load(property, (Serializable) property); // In Oracle, empty string are converted to NULL. Since an undefined property is not found at all, - // it is - // safe to assume that a retrieved NULL value should actually be an empty string. + // it is safe to assume that a retrieved NULL value should actually be an empty string. if (property instanceof BaseStringProperty) { BaseStringProperty stringProperty = (BaseStringProperty) property; if (stringProperty.getValue() == null) { stringProperty.setValue(""); } } - ((BaseProperty) property).setValueDirty(false); + ((BaseProperty) property).setDirty(false); } catch (ObjectNotFoundException e) { // Let's accept that there is no data in property tables but log it this.logger.error("No data for property [{}] of object id [{}]", property.getName(), @@ -1831,7 +1849,7 @@ private void saveXWikiPropertyInternal(final PropertyInterface property, final X session.save(property); } - ((BaseProperty) property).setValueDirty(false); + ((BaseProperty) property).setDirty(false); if (bTransaction) { endTransaction(context, true);