This is an automated email from the ASF dual-hosted git repository. ahuber pushed a commit to branch 3425-wicket.form.update.issue in repository https://gitbox.apache.org/repos/asf/causeway.git
commit d41ab20ca6d467785daa6ef5bece63bb2b7b7228 Author: Andi Huber <[email protected]> AuthorDate: Thu Apr 27 14:42:05 2023 +0200 CAUSEWAY-3425: fixes repaint decision logic --- .../causeway/commons/internal/base/_Refs.java | 2 +- .../managed/ParameterNegotiationModel.java | 11 ++++ .../spec/feature/ObjectActionParameter.java | 20 ++++++- .../viewer/wicket/model/models/ScalarModel.java | 26 ++++++-- .../components/actions/ActionParametersForm.java | 62 +++++++------------ .../ui/components/property/PropertyEditForm.java | 2 +- .../ui/components/scalars/ScalarPanelAbstract.java | 69 +++++++++------------- .../scalars/ScalarPanelFormFieldAbstract.html | 6 +- .../scalars/ScalarPanelSelectAbstract.java | 14 +---- 9 files changed, 104 insertions(+), 108 deletions(-) diff --git a/commons/src/main/java/org/apache/causeway/commons/internal/base/_Refs.java b/commons/src/main/java/org/apache/causeway/commons/internal/base/_Refs.java index 06a8289f15..86f651a748 100644 --- a/commons/src/main/java/org/apache/causeway/commons/internal/base/_Refs.java +++ b/commons/src/main/java/org/apache/causeway/commons/internal/base/_Refs.java @@ -196,7 +196,7 @@ public final class _Refs { return value++; } - public int gatAndDec() { + public int getAndDec() { return value--; } } diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/interactions/managed/ParameterNegotiationModel.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/interactions/managed/ParameterNegotiationModel.java index a19abc407b..2cfca6ce81 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/interactions/managed/ParameterNegotiationModel.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/interactions/managed/ParameterNegotiationModel.java @@ -20,6 +20,7 @@ package org.apache.causeway.core.metamodel.interactions.managed; import java.util.Objects; import java.util.Optional; +import java.util.function.UnaryOperator; import java.util.stream.IntStream; import org.springframework.lang.Nullable; @@ -235,6 +236,16 @@ public class ParameterNegotiationModel { paramModels.getElseFail(paramIndex).getBindableParamValue().setValue(emptyValue); } + public void updateParamValue(final int paramIndex, final @NonNull UnaryOperator<ManagedObject> updater) { + val bindableParamValue = paramModels.getElseFail(paramIndex).getBindableParamValue(); + val newParamValue = updater.apply(bindableParamValue.getValue()); + if (ManagedObjects.isNullOrUnspecifiedOrEmpty(newParamValue)) { + clearParamValue(paramIndex); + } else { + bindableParamValue.setValue(newParamValue); + } + } + @NonNull public ManagedObject adaptParamValuePojo(final int paramIndex, final @Nullable Object newParamValuePojo) { val paramMeta = getParamMetamodel(paramIndex); diff --git a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/feature/ObjectActionParameter.java b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/feature/ObjectActionParameter.java index 785a6632aa..1acc3a9506 100644 --- a/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/feature/ObjectActionParameter.java +++ b/core/metamodel/src/main/java/org/apache/causeway/core/metamodel/spec/feature/ObjectActionParameter.java @@ -18,6 +18,7 @@ */ package org.apache.causeway.core.metamodel.spec.feature; +import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; @@ -25,6 +26,7 @@ import org.springframework.lang.Nullable; import org.apache.causeway.applib.annotation.Domain; import org.apache.causeway.commons.collections.Can; +import org.apache.causeway.commons.internal.base._Refs; import org.apache.causeway.core.metamodel.consent.Consent; import org.apache.causeway.core.metamodel.consent.InteractionInitiatedBy; import org.apache.causeway.core.metamodel.facetapi.FeatureType; @@ -33,6 +35,7 @@ import org.apache.causeway.core.metamodel.interactions.ActionArgValidityContext; import org.apache.causeway.core.metamodel.interactions.InteractionHead; import org.apache.causeway.core.metamodel.interactions.managed.ParameterNegotiationModel; import org.apache.causeway.core.metamodel.object.ManagedObject; +import org.apache.causeway.core.metamodel.object.MmUnwrapUtils; import org.apache.causeway.core.metamodel.spec.ObjectSpecification; import org.apache.causeway.core.metamodel.spec.feature.memento.ActionParameterMemento; import org.apache.causeway.core.metamodel.util.Facets; @@ -140,13 +143,16 @@ extends ObjectFeature, CurrentHolder { @NonNull ManagedObject getDefault(ParameterNegotiationModel pendingArgs); /** + * Returns whether the pending parameter changed during reassessment. + * <p> * Reassesses the current parameter value, that is applying <i>defaults semantics</i>, * whenever a parameter this one depends on changes in the UI. Parameters * with higher index depend on those with lower index. * <p> * Reassessment can be switch off by means of {@link org.apache.causeway.applib.annotation.Parameter#dependentDefaultsPolicy()}. */ - default void reassessDefault(final ParameterNegotiationModel pendingArgs) { + default boolean reassessDefault(final ParameterNegotiationModel pendingArgs) { + val changeCount = _Refs.intRef(0); // this is just used as a flag, might as well use eg. AtomicBoolean val paramIndex = getParameterIndex(); val bindableParamDirtyFlag = pendingArgs.getBindableParamValueDirtyFlag(paramIndex); if(Facets.dependentDefaultsPolicy(this).isUpdateDependent() @@ -154,9 +160,17 @@ extends ObjectFeature, CurrentHolder { || ! bindableParamDirtyFlag.getValue().booleanValue() ) { // reassess defaults honoring defaults semantics val paramDefaultValue = this.getDefault(pendingArgs); - pendingArgs.setParamValue(paramIndex, paramDefaultValue); - bindableParamDirtyFlag.setValue(false); // clear dirty flag + pendingArgs.updateParamValue(paramIndex, paramOldValue->{ + val oldPojo = MmUnwrapUtils.single(paramOldValue); + val newPojo = MmUnwrapUtils.single(paramDefaultValue); + if(!Objects.equals(oldPojo, newPojo)) { + changeCount.getAndInc(); + } + return paramDefaultValue; + }); + bindableParamDirtyFlag.setValue(false); // clear dirty flag (signaling not edited by user in the UI) } + return changeCount.getValue()>0; } @NonNull default ManagedObject getEmpty() { diff --git a/viewers/wicket/model/src/main/java/org/apache/causeway/viewer/wicket/model/models/ScalarModel.java b/viewers/wicket/model/src/main/java/org/apache/causeway/viewer/wicket/model/models/ScalarModel.java index 9ba321d356..49eb17ce4a 100644 --- a/viewers/wicket/model/src/main/java/org/apache/causeway/viewer/wicket/model/models/ScalarModel.java +++ b/viewers/wicket/model/src/main/java/org/apache/causeway/viewer/wicket/model/models/ScalarModel.java @@ -19,6 +19,7 @@ package org.apache.causeway.viewer.wicket.model.models; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.OptionalInt; @@ -36,6 +37,7 @@ import org.apache.causeway.core.metamodel.commons.ScalarRepresentation; import org.apache.causeway.core.metamodel.interactions.managed.ManagedValue; import org.apache.causeway.core.metamodel.object.ManagedObject; import org.apache.causeway.core.metamodel.object.ManagedObjects; +import org.apache.causeway.core.metamodel.object.MmUnwrapUtils; import org.apache.causeway.core.metamodel.spec.feature.ObjectAction; import org.apache.causeway.core.metamodel.util.Facets; import org.apache.causeway.viewer.commons.model.hints.HasRenderingHints; @@ -133,19 +135,31 @@ implements HasRenderingHints, UiScalar, LinksProvider, FormExecutorContext { /** * Sets given ManagedObject as new proposed value. * (override, so we don't return the target model, we are chained to) + * <p> + * This happens during Wicket's {@code formComponent.updateModel()}, + * which updates the models first, before later calling {@code onUpdate(target)} + * to repaint those components, that have a changed/dirty model. + * <p> + * in other words: changed components get a chance to participate in the partial page update + * based on whether their models have changed */ @Override public final void setObject(final ManagedObject newValue) { - _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{ - _Debug.log("[PENDING MODEL] about to set new value: %s", newValue==null?"null":newValue.getPojo()); - }); + proposedValue().update(oldValue->{ - proposedValue().getValue().setValue(newValue); + _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{ + val oldPojo = MmUnwrapUtils.single(oldValue); + val newPojo = MmUnwrapUtils.single(newValue); + val changed = !Objects.equals(oldPojo, newPojo); + _Debug.log("[PENDING MODEL] about to update %s value: %s -> %s (changed=%b)", + this.isParameter() ? "parameter" : "property", + oldPojo, newPojo, changed); + }); - _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{ - _Debug.log("[PENDING MODEL] new value set to property"); + return newValue; }); + } @Override diff --git a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/actions/ActionParametersForm.java b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/actions/ActionParametersForm.java index e71cbcdccf..620e02be26 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/actions/ActionParametersForm.java +++ b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/actions/ActionParametersForm.java @@ -29,8 +29,6 @@ import org.apache.wicket.markup.repeater.RepeatingView; import org.apache.causeway.commons.functional.Either; import org.apache.causeway.commons.internal.base._Casts; -import org.apache.causeway.commons.internal.collections._Lists; -import org.apache.causeway.commons.internal.exceptions._Exceptions; import org.apache.causeway.viewer.commons.model.components.UiComponentType; import org.apache.causeway.viewer.commons.model.decorators.ConfirmDecorator.ConfirmDecorationModel; import org.apache.causeway.viewer.commons.model.layout.UiPlacementDirection; @@ -140,47 +138,27 @@ extends PromptFormAbstract<ActionModel> { // only updates subsequent parameter panels starting from (paramNumberUpdated + 1) final int skipCount = paramNumberUpdated + 1; // eg. if paramNumberUpdated=0 then skipCount=1 - val paramCount = updatedParamModel.getMetaModel().getAction().getParameterCount(); - val maxCapacity = paramCount - skipCount; // just an optimization, not strictly required - val paramOnlyUpdateRequestsHavingParamIndex = _Lists.<Integer>newArrayList(maxCapacity); - - val formRepaint = actionModel.streamPendingParamUiModels() - .skip(skipCount) - .map(paramModel->{ - - val paramIndexForReassessment = paramModel.getParameterIndex(); - val paramPanel = paramPanels.get(paramIndexForReassessment); - val actionParameter = paramModel.getMetaModel(); - - // updates the paramNegotiationModel - actionParameter.reassessDefault(paramNegotiationModel); - _Xray.reassessedDefault(paramIndexForReassessment, paramNegotiationModel); - - // repaint is calculated based on changes to the - val paramRepaint = paramPanel.updateIfNecessary(paramModel, Optional.of(target)); - if(paramRepaint.isParamOnly()) { - paramOnlyUpdateRequestsHavingParamIndex.add(paramIndexForReassessment); - } - - return paramRepaint; - }) - .reduce(Repaint.NOTHING, (a, b)->a.ordinal()>b.ordinal() ? a : b); - - switch (formRepaint) { - case ENTIRE_FORM: - target.add(this); - break; - case PARAM_ONLY: - paramOnlyUpdateRequestsHavingParamIndex.forEach(paramIndex->{ - val paramPanel = paramPanels.get(paramIndex); + actionModel.streamPendingParamUiModels() + .skip(skipCount) + .forEach(paramModel->{ + val paramIndexForReassessment = paramModel.getParameterIndex(); + val paramPanel = paramPanels.get(paramIndexForReassessment); + + var paramRepaint = + // potentially updates the paramNegotiationModel + Repaint.required(paramModel.getMetaModel().reassessDefault(paramNegotiationModel)); + + _Xray.reassessedDefault(paramIndexForReassessment, paramNegotiationModel); + + /* repaint is required, either because of a changed value during reassessment above + * or because visibility or usability have changed */ + paramRepaint = paramRepaint.max( + paramPanel.updateIfNecessary(paramModel, Optional.of(target))); + + if(paramRepaint.isRequired()) { paramPanel.repaint(target); - }); - break; - case NOTHING: - break; - default: - throw _Exceptions.unmatchedCase(formRepaint); - } + } + }); _Xray.afterParamFormUpdate(paramNumberUpdated, paramNegotiationModel); diff --git a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/property/PropertyEditForm.java b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/property/PropertyEditForm.java index b5a806e8cf..f227de28f9 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/property/PropertyEditForm.java +++ b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/property/PropertyEditForm.java @@ -32,7 +32,7 @@ import org.apache.causeway.viewer.wicket.ui.util.Wkt; import lombok.val; -class PropertyEditForm +final class PropertyEditForm extends PromptFormAbstract<ScalarPropertyModel> { private static final long serialVersionUID = 1L; diff --git a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java index 11257acf39..b6a7416787 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java +++ b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java @@ -22,7 +22,6 @@ import java.io.Serializable; import java.util.Collections; import java.util.EnumSet; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.function.Supplier; @@ -47,7 +46,6 @@ import org.apache.causeway.core.metamodel.commons.ScalarRepresentation; import org.apache.causeway.core.metamodel.facets.objectvalue.labelat.LabelAtFacet; import org.apache.causeway.core.metamodel.interactions.managed.InteractionVeto; import org.apache.causeway.core.metamodel.object.ManagedObject; -import org.apache.causeway.core.metamodel.object.ManagedObjects; import org.apache.causeway.core.metamodel.util.Facets; import org.apache.causeway.viewer.commons.model.components.UiComponentType; import org.apache.causeway.viewer.commons.model.decorators.FormLabelDecorator.FormLabelDecorationModel; @@ -93,17 +91,6 @@ implements ScalarModelChangeListener { TEXT_ONLY, } - /** - * Order matters: ascending order of precedence, eg. when used in reductions. - */ - public enum Repaint { - NOTHING, - PARAM_ONLY, - ENTIRE_FORM,; - public boolean isParamOnly() { return this == PARAM_ONLY; } - public boolean isEntireForm() { return this == ENTIRE_FORM; } - } - public enum RenderScenario { COMPACT, /** @@ -160,6 +147,28 @@ implements ScalarModelChangeListener { } + /** + * During AJAX requests, first the {@link ScalarModel} gets updated, + * then later, changed components get a chance to participate in the partial page update + * based on whether their models have changed. + * <p> + * This enum helps evaluate whether components using this model need repainting. + */ + public enum Repaint { + OPTIONAL, + REQUIRED; + public static Repaint required(final boolean needsRepainting) { + return needsRepainting ? Repaint.REQUIRED : Repaint.OPTIONAL; + } + public boolean isOptional() { return this == OPTIONAL; } + public boolean isRequired() { return this == REQUIRED; } + public Repaint max(final @NonNull Repaint other) { + return this.ordinal()>=other.ordinal() + ? this + : other; + } + } + // -- CONSTRUCTION /** @@ -473,7 +482,7 @@ implements ScalarModelChangeListener { return Collections.unmodifiableCollection(changeListeners); } - public void addChangeListener(final ScalarModelChangeListener listener) { + void addChangeListener(final ScalarModelChangeListener listener) { changeListeners.add(listener); } } @@ -609,17 +618,14 @@ implements ScalarModelChangeListener { * @param target - in case there's more to be repainted... * * @return - {@link Repaint} as a result of these pending arguments<ul> - * <li>{@link Repaint#NOTHING} if nothing changed</li> - * <li>{@link Repaint#PARAM_ONLY} if param value changed</li> - * <li>{@link Repaint#ENTIRE_FORM} if layout changed</li> + * <li>{@link Repaint#OPTIONAL} if nothing changed</li> + * <li>{@link Repaint#REQUIRED} if param value changed</li> * </ul> */ public Repaint updateIfNecessary( final @NonNull UiParameter paramModel, final @NonNull Optional<AjaxRequestTarget> target) { - val scalarModel = scalarModel(); - // visibility val visibilityBefore = isVisible() && isVisibilityAllowed(); val visibilityConsent = paramModel.getParameterNegotiationModel().getVisibilityConsent(paramModel.getParameterIndex()); @@ -637,33 +643,16 @@ implements ScalarModelChangeListener { onNotEditable(usabilityConsent.getReasonAsString().orElse(null), target); } - val paramValue = paramModel.getValue(); - val valueChanged = !Objects.equals(scalarModel.getObject(), paramValue); - - if(valueChanged) { - if(ManagedObjects.isNullOrUnspecifiedOrEmpty(paramValue)) { - scalarModel.setObject(null); - } else { - scalarModel.setObject(paramValue); - } - } - - // repaint the entire form if visibility has changed + // repaint the param form if visibility has changed if (!visibilityBefore || !visibilityAfter) { - return Repaint.ENTIRE_FORM; + return Repaint.REQUIRED; } - // repaint the param if usability has changed if (!usabilityAfter || !usabilityBefore) { - return Repaint.PARAM_ONLY; + return Repaint.REQUIRED; } - // also repaint the param if its pending arg has changed. - return valueChanged - ? Repaint.PARAM_ONLY - : Repaint.NOTHING; + return Repaint.OPTIONAL; } - - } diff --git a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelFormFieldAbstract.html b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelFormFieldAbstract.html index e010bc47fa..169d4ab6df 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelFormFieldAbstract.html +++ b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelFormFieldAbstract.html @@ -75,7 +75,7 @@ <button type="button" class="btn btn-primary"> <i class="fa-solid fa-edit"></i></button> - <!-- additional buttons --> + <!-- additional buttons (scalarValueInlinePromptLink) --> <button wicket:id="scalarValueInlinePromptLink-buttons" class="btn" type="button"> </button> @@ -93,7 +93,7 @@ wicket:id="container-scalarValue" /> <div class="btn-group btn-group-sm additionalButtons" role="group"> - <!-- additional buttons --> + <!-- additional buttons (scalarValueView) --> <button wicket:id="scalarValueView-buttons" class="btn" type="button"> </button> @@ -111,7 +111,7 @@ <!-- honor bootstrap modal dialog z-index of 1055--> <span class="position-absolute top-0 end-0" style="z-index: 1080;"> <div class="btn-group btn-group-sm shadow" role="group"> - <!-- additional buttons --> + <!-- additional buttons (scalarValueInput) --> <button wicket:id="scalarValueInput-buttons" class="btn" type="button"> </button> diff --git a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelSelectAbstract.java b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelSelectAbstract.java index 4c53035902..fa5b65427c 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelSelectAbstract.java +++ b/viewers/wicket/ui/src/main/java/org/apache/causeway/viewer/wicket/ui/components/scalars/ScalarPanelSelectAbstract.java @@ -138,18 +138,8 @@ extends ScalarPanelFormFieldAbstract<ManagedObject> { final @NonNull UiParameter paramModel, final @NonNull Optional<AjaxRequestTarget> target) { - val repaint = super.updateIfNecessary(paramModel, target); - final boolean choicesUpdated = updateChoices(this.select2); - - if (repaint == Repaint.NOTHING) { - if (choicesUpdated) { - return Repaint.PARAM_ONLY; - } else { - return Repaint.NOTHING; - } - } else { - return repaint; - } + return super.updateIfNecessary(paramModel, target) + .max(Repaint.required(updateChoices(this.select2))); } private boolean updateChoices(final @Nullable Select2 select2) {
