You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2021/12/08 22:01:08 UTC

[isis] branch master updated: ISIS-2911: fixes FormExecutor to reload state from bookmark before applying changes

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git


The following commit(s) were added to refs/heads/master by this push:
     new 3c2de1d  ISIS-2911: fixes FormExecutor to reload state from bookmark before applying changes
3c2de1d is described below

commit 3c2de1d33016e594a45e41611d0534fb9093972c
Author: Andi Huber <ah...@apache.org>
AuthorDate: Wed Dec 8 23:00:53 2021 +0100

    ISIS-2911: fixes FormExecutor to reload state from bookmark before
    applying changes
---
 .../ActionParameterAutoCompleteFacetViaMethod.java |  3 ++-
 .../ActionParameterChoicesFacetViaMethod.java      |  2 +-
 .../method/PropertyChoicesFacetViaMethod.java      |  3 ++-
 .../interactions/managed/ManagedMember.java        |  2 +-
 .../managed/PropertyNegotiationModel.java          |  4 ++++
 .../isis/core/metamodel/spec/ManagedObject.java    | 22 ++++++++++++++++++++--
 .../isis/core/metamodel/spec/ManagedObjects.java   | 10 +++++++---
 .../viewers/jdo/wkt/InteractionTestJdoWkt.java     |  5 ++---
 .../viewers/jpa/wkt/InteractionTestJpaWkt.java     | 16 +++++++++++++---
 .../viewer/wicket/model/models/ScalarModel.java    |  4 ++--
 .../entity/icontitle/EntityIconAndTitlePanel.java  |  2 +-
 .../wicket/ui/panels/FormExecutorDefault.java      | 14 +++++++++-----
 12 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/autocomplete/method/ActionParameterAutoCompleteFacetViaMethod.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/autocomplete/method/ActionParameterAutoCompleteFacetViaMethod.java
index c1163ec..37804c3 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/autocomplete/method/ActionParameterAutoCompleteFacetViaMethod.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/autocomplete/method/ActionParameterAutoCompleteFacetViaMethod.java
@@ -88,7 +88,8 @@ implements ImperativeFacet {
         }
         val elementSpec = specForTypeElseFail(choicesType);
         val visibleChoices = ManagedObjects
-                .adaptMultipleOfTypeThenAttachThenFilterByVisibility(elementSpec, collectionOrArray, interactionInitiatedBy);
+                .adaptMultipleOfTypeThenRefetchThenFilterByVisibility(
+                        elementSpec, collectionOrArray, interactionInitiatedBy);
 
         return visibleChoices;
     }
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/choices/methodnum/ActionParameterChoicesFacetViaMethod.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/choices/methodnum/ActionParameterChoicesFacetViaMethod.java
index cea8738..daa249c 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/choices/methodnum/ActionParameterChoicesFacetViaMethod.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/param/choices/methodnum/ActionParameterChoicesFacetViaMethod.java
@@ -78,7 +78,7 @@ implements ImperativeFacet {
         }
 
         val visibleChoices = ManagedObjects
-                .adaptMultipleOfTypeThenAttachThenFilterByVisibility(
+                .adaptMultipleOfTypeThenRefetchThenFilterByVisibility(
                         requiredSpec, collectionOrArray, interactionInitiatedBy);
 
         return visibleChoices;
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/properties/choices/method/PropertyChoicesFacetViaMethod.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/properties/choices/method/PropertyChoicesFacetViaMethod.java
index c92ad5e..2650ec6 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/properties/choices/method/PropertyChoicesFacetViaMethod.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/properties/choices/method/PropertyChoicesFacetViaMethod.java
@@ -65,7 +65,8 @@ implements ImperativeFacet {
         val elementSpec = specForTypeElseFail(((FacetedMethod) getFacetHolder()).getType());
         val optionPojos = ManagedObjects.InvokeUtil.invoke(method, owningAdapter);
         val visibleChoices = ManagedObjects
-                .adaptMultipleOfTypeThenAttachThenFilterByVisibility(elementSpec, optionPojos, interactionInitiatedBy);
+                .adaptMultipleOfTypeThenRefetchThenFilterByVisibility(
+                        elementSpec, optionPojos, interactionInitiatedBy);
         return visibleChoices;
     }
 
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/ManagedMember.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/ManagedMember.java
index a49995e..56b1edd 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/ManagedMember.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/ManagedMember.java
@@ -98,7 +98,7 @@ implements ManagedFeature {
     public ManagedObject getOwner() {
         //XXX this is a safeguard
         // see also org.apache.isis.core.metamodel.interactions.managed.ManagedProperty.ManagedProperty(ManagedObject, OneToOneAssociation, Where)
-        EntityUtil.reattach(owner);
+        EntityUtil.refetch(owner);
         return owner;
     }
 
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/PropertyNegotiationModel.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/PropertyNegotiationModel.java
index b48b633..3dbc93f 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/PropertyNegotiationModel.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/PropertyNegotiationModel.java
@@ -26,6 +26,7 @@ import org.apache.isis.commons.internal.binding._Bindables;
 import org.apache.isis.commons.internal.binding._Observables;
 import org.apache.isis.commons.internal.binding._Observables.BooleanObservable;
 import org.apache.isis.commons.internal.binding._Observables.LazyObservable;
+import org.apache.isis.commons.internal.debug._Debug;
 import org.apache.isis.core.metamodel.consent.InteractionInitiatedBy;
 import org.apache.isis.core.metamodel.spec.ManagedObject;
 import org.apache.isis.core.metamodel.spec.ManagedObjects;
@@ -155,6 +156,9 @@ public class PropertyNegotiationModel implements ManagedValue {
     // -- SUBMISSION
 
     public void submit() {
+
+        _Debug.log("[PENDING MODEL] submit pending property value '%s' into owning object", getValue().getValue());
+
         managedProperty.modifyProperty(getValue().getValue());
         isCurrentValueAbsent.invalidate();
     }
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObject.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObject.java
index e0f8bbb..6611c5b 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObject.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObject.java
@@ -61,7 +61,7 @@ public interface ManagedObject {
     Object getPojo();
 
     /**
-     * Introduced, so we can re-attach detached entity pojos in place.
+     * Introduced, so we can re-fetch detached entity pojos in place.
      * @apiNote should be package private, and not publicly exposed
      * (but the <i>Java</i> language is not there yet)
      */
@@ -83,6 +83,24 @@ public interface ManagedObject {
      */
     Optional<Bookmark> getBookmarkRefreshed();
 
+    /**
+     * Reload current viewmodel object from memoized bookmark, otherwise does nothing.
+     */
+    default void reloadViewmodelFromMemoizedBookmark() {
+        val spec = getSpecification();
+        if(isBookmarkMemoized()
+                && spec.isViewModel()) {
+
+            val bookmark = getBookmark().get();
+            val viewModelClass = spec.getCorrespondingClass();
+
+            val recreatedViewmodel =
+                    getMetaModelContext().getFactoryService().viewModel(viewModelClass, bookmark);
+
+            replacePojo(old->recreatedViewmodel);
+        }
+    }
+
     boolean isBookmarkMemoized();
 
     default Supplier<ManagedObject> asProvider() {
@@ -341,7 +359,7 @@ public interface ManagedObject {
             pojo = replacer.apply(pojo);
         }
 
-
     }
 
+
 }
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObjects.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObjects.java
index b72fd7d..1a6018b 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObjects.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/spec/ManagedObjects.java
@@ -330,14 +330,14 @@ public final class ManagedObjects {
     /**
      * used eg. to adapt the result of supporting methods, that return choice pojos
      */
-    public static Can<ManagedObject> adaptMultipleOfTypeThenAttachThenFilterByVisibility(
+    public static Can<ManagedObject> adaptMultipleOfTypeThenRefetchThenFilterByVisibility(
             final @NonNull  ObjectSpecification elementSpec,
             final @Nullable Object collectionOrArray,
             final @NonNull  InteractionInitiatedBy interactionInitiatedBy) {
 
         return _NullSafe.streamAutodetect(collectionOrArray)
         .map(pojo->ManagedObject.of(elementSpec, pojo)) // pojo is nullable here
-        .peek(ManagedObjects.EntityUtil::reattach)
+        .peek(ManagedObjects.EntityUtil::refetch)
         .filter(ManagedObjects.VisibilityUtil.filterOn(interactionInitiatedBy))
         .collect(Can.toCan());
     }
@@ -428,6 +428,10 @@ public final class ManagedObjects {
         public void replacePojo(final UnaryOperator<Object> replacer) {
         }
 
+        @Override
+        public void reloadViewmodelFromMemoizedBookmark() {
+        }
+
     };
 
     // -- TITLE UTILITIES
@@ -618,7 +622,7 @@ public final class ManagedObjects {
             return managedObject;
         }
 
-        public static void reattach(final @Nullable ManagedObject managedObject) {
+        public static void refetch(final @Nullable ManagedObject managedObject) {
             if(isNullOrUnspecifiedOrEmpty(managedObject)) {
                 return;
             }
diff --git a/regressiontests/stable-viewers-jdo/src/test/java/org/apache/isis/testdomain/viewers/jdo/wkt/InteractionTestJdoWkt.java b/regressiontests/stable-viewers-jdo/src/test/java/org/apache/isis/testdomain/viewers/jdo/wkt/InteractionTestJdoWkt.java
index 18d66d9..a3a7dbf 100644
--- a/regressiontests/stable-viewers-jdo/src/test/java/org/apache/isis/testdomain/viewers/jdo/wkt/InteractionTestJdoWkt.java
+++ b/regressiontests/stable-viewers-jdo/src/test/java/org/apache/isis/testdomain/viewers/jdo/wkt/InteractionTestJdoWkt.java
@@ -24,7 +24,6 @@ import org.apache.wicket.ajax.AjaxEventBehavior;
 import org.apache.wicket.extensions.ajax.markup.html.IndicatingAjaxButton;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.TestPropertySource;
@@ -86,7 +85,7 @@ class InteractionTestJdoWkt extends RegressionTestAbstract {
         XrayUi.waitForShutdown();
     }
 
-    @Test @Disabled//WIP
+    @Test
     void load_viewmodel_with_referenced_entities_directly() {
 
         val pageParameters = call(()->{
@@ -110,7 +109,7 @@ class InteractionTestJdoWkt extends RegressionTestAbstract {
 
     }
 
-    @Test @Disabled//WIP
+    @Test
     void load_viewmodel_with_referenced_entities_via_action() {
         val pageParameters = call(()->{
             val testHomePage = new TestAppJdoWkt.TestHomePage();
diff --git a/regressiontests/stable-viewers-jpa/src/test/java/org/apache/isis/testdomain/viewers/jpa/wkt/InteractionTestJpaWkt.java b/regressiontests/stable-viewers-jpa/src/test/java/org/apache/isis/testdomain/viewers/jpa/wkt/InteractionTestJpaWkt.java
index c0826ce..220fda5 100644
--- a/regressiontests/stable-viewers-jpa/src/test/java/org/apache/isis/testdomain/viewers/jpa/wkt/InteractionTestJpaWkt.java
+++ b/regressiontests/stable-viewers-jpa/src/test/java/org/apache/isis/testdomain/viewers/jpa/wkt/InteractionTestJpaWkt.java
@@ -21,12 +21,14 @@ package org.apache.isis.testdomain.viewers.jpa.wkt;
 import javax.inject.Inject;
 
 import org.apache.wicket.ajax.AjaxEventBehavior;
+import org.apache.wicket.extensions.ajax.markup.html.IndicatingAjaxButton;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.TestPropertySource;
 
+import org.apache.isis.commons.internal.debug._Debug;
 import org.apache.isis.commons.internal.debug.xray.XrayUi;
 import org.apache.isis.core.config.presets.IsisPresets;
 import org.apache.isis.testdomain.RegressionTestAbstract;
@@ -41,6 +43,7 @@ import org.apache.isis.testdomain.viewers.jdo.wkt.TestAppJpaWkt;
 import org.apache.isis.viewer.wicket.ui.panels.PromptFormAbstract;
 
 import static org.apache.isis.testdomain.conf.Configuration_usingWicket.EntityPageTester.INLINE_PROMPT_FORM_FIELD;
+import static org.apache.isis.testdomain.conf.Configuration_usingWicket.EntityPageTester.INLINE_PROMPT_FORM_OK;
 import static org.apache.isis.testdomain.conf.Configuration_usingWicket.EntityPageTester.INVENTORY_NAME_PROPERTY_EDIT_INLINE_PROMPT_FORM;
 import static org.apache.isis.testdomain.conf.Configuration_usingWicket.EntityPageTester.INVENTORY_NAME_PROPERTY_EDIT_LINK;
 import static org.apache.isis.testdomain.conf.Configuration_usingWicket.EntityPageTester.OPEN_SAMPLE_ACTION;
@@ -146,18 +149,25 @@ class InteractionTestJpaWkt extends RegressionTestAbstract {
             wktTester.assertInventoryNameIs("Bookstore");
         });
 
-        // simulate change of a String property and form submit
+        // simulate change of a String property Name from 'Bookstore' -> 'Bookstore2'
         run(()->{
             val form = wktTester.newFormTester(INVENTORY_NAME_PROPERTY_EDIT_INLINE_PROMPT_FORM);
             form.setValue(INLINE_PROMPT_FORM_FIELD, "Bookstore2");
             form.submit();
         });
 
+        // simulate click on form OK button -> expected to trigger the framework's property change execution
+        run(()->{
+            wktTester.assertComponent(INLINE_PROMPT_FORM_OK, IndicatingAjaxButton.class);
+            wktTester.executeAjaxEvent(INLINE_PROMPT_FORM_OK, "click");
+        });
+
+        _Debug.log("[TEST] form submitted");
+
         // ... should yield a new Title containing 'Bookstore2'
         run(()->{
             wktTester.assertHeaderBrandText("Smoke Tests");
-            //FIXME
-            //wktTester.assertPageTitle("JpaInventoryJaxbVm; Bookstore2; 3 products");
+            wktTester.assertPageTitle("JpaInventoryJaxbVm; Bookstore2; 3 products");
             wktTester.assertFavoriteBookIs(BookDto.sample());
             wktTester.assertInventoryNameIs("Bookstore2");
         });
diff --git a/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java b/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java
index 1a9188e..7e3e6aa 100644
--- a/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java
+++ b/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ScalarModel.java
@@ -146,13 +146,13 @@ implements HasRenderingHints, ScalarUiModel, LinksProvider, FormExecutorContext
     public final void setObject(final ManagedObject newValue) {
 
         _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{
-            _Debug.log("about to set new value: %s", newValue.getPojo());
+            _Debug.log("[PENDING MODEL] about to set new value: %s", newValue.getPojo());
         });
 
         proposedValue().getValue().setValue(newValue);
 
         _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{
-            _Debug.log("new value set to property");
+            _Debug.log("[PENDING MODEL] new value set to property");
         });
     }
 
diff --git a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/entity/icontitle/EntityIconAndTitlePanel.java b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/entity/icontitle/EntityIconAndTitlePanel.java
index 4d85427..66468b1 100644
--- a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/entity/icontitle/EntityIconAndTitlePanel.java
+++ b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/entity/icontitle/EntityIconAndTitlePanel.java
@@ -152,7 +152,7 @@ extends PanelAbstract<ManagedObject, ObjectAdapterModel> {
 
         if(targetAdapter != null) {
 
-            EntityUtil.reattach(targetAdapter);
+            EntityUtil.refetch(targetAdapter);
 
             val redirectToAdapter = entityModel.getTypeOfSpecification().lookupFacet(ProjectionFacet.class)
                     .map(projectionFacet->projectionFacet.projected(targetAdapter))
diff --git a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/panels/FormExecutorDefault.java b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/panels/FormExecutorDefault.java
index eba1387..0ee9b98 100644
--- a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/panels/FormExecutorDefault.java
+++ b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/panels/FormExecutorDefault.java
@@ -89,6 +89,9 @@ implements FormExecutor {
 
         try {
 
+            _Debug.log("[EXECUTOR] reload");
+            formExecutorContext.getParentObject().reloadViewmodelFromMemoizedBookmark();
+
             final Optional<Recognition> invalidReasonIfAny = Recognition.of(
                     Category.CONSTRAINT_VIOLATION,
                     actionOrPropertyModel
@@ -107,7 +110,7 @@ implements FormExecutor {
                         act->act.getFriendlyName(),
                         prop->prop.getFriendlyName());
 
-                _Debug.log("execute %s ...", whatIsExecuted);
+                _Debug.log("[EXECUTOR] execute %s ...", whatIsExecuted);
             });
 
             //
@@ -124,6 +127,7 @@ implements FormExecutor {
             // (The DB exception might actually be thrown by the flush() that follows.
             //
             //XXX triggers BookmarkedObjectWkt.getObjectAndReAttach() down the call-stack
+            //XXX applies the pending property
             val resultAdapter = actionOrPropertyModel.fold(
                     act->act.executeActionAndReturnResult(),
                     prop->prop.applyValueThenReturnOwner());
@@ -135,7 +139,7 @@ implements FormExecutor {
                         act->act.getFriendlyName(),
                         prop->prop.getFriendlyName());
 
-                _Debug.log("resultAdapter created for %s", whatIsExecuted);
+                _Debug.log("[EXECUTOR] resultAdapter created for %s", whatIsExecuted);
             });
 
             // if we are in a nested dialog/form, that supports an action parameter,
@@ -154,7 +158,7 @@ implements FormExecutor {
             }
 
             _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{
-                _Debug.log("interpret result ...");
+                _Debug.log("[EXECUTOR] interpret result ...");
             });
 
             //XXX triggers ManagedObject.getBookmarkRefreshed()
@@ -165,7 +169,7 @@ implements FormExecutor {
                             .toEntityPage(resultAdapter));
 
             _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{
-                _Debug.log("handle result ...");
+                _Debug.log("[EXECUTOR] handle result ...");
             });
 
             // redirect using associated strategy
@@ -174,7 +178,7 @@ implements FormExecutor {
                 .handleResults(getCommonContext(), resultResponse);
 
             _Debug.onCondition(XrayUi.isXrayEnabled(), ()->{
-                _Debug.log("... return");
+                _Debug.log("[EXECUTOR] ... return");
             });
 
             return FormExecutionOutcome.SUCCESS_AND_REDIRECED_TO_RESULT_PAGE; // success (valid args), allow redirect