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

ahuber pushed a commit to branch ISIS-3202_exec.not.persistent
in repository https://gitbox.apache.org/repos/asf/isis.git

commit 3bbf6777b132cfc901648b48769e6bc3f8ffffbe
Author: Andi Huber <[email protected]>
AuthorDate: Sat Sep 3 08:20:14 2022 +0200

    ISIS-3202: fail early if executor result adapter has no bookmark
---
 .../facets/object/callbacks/CallbackFacet.java     |  2 +-
 .../managed/nonscalar/DataTableModel.java          |  5 ++-
 .../isis/core/metamodel/object/ManagedObjects.java | 26 ++++++++++++++--
 .../executor/MemberExecutorServiceDefault.java     | 36 ++++++++++++----------
 .../factory/FactoryServiceDefault.java             |  3 +-
 .../publish/LifecycleCallbackNotifier.java         |  2 +-
 6 files changed, 48 insertions(+), 26 deletions(-)

diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/callbacks/CallbackFacet.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/callbacks/CallbackFacet.java
index 38ae297090..18575bf85a 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/callbacks/CallbackFacet.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/facets/object/callbacks/CallbackFacet.java
@@ -37,7 +37,7 @@ extends ImperativeFacet {
             final ManagedObject object,
             final Class<? extends CallbackFacet> callbackFacetType) {
 
-        ManagedObjects.whenSpecified(object)
+        ManagedObjects.asSpecified(object)
         .map(ManagedObject::getSpecification)
         .flatMap(spec->spec.lookupFacet(callbackFacetType))
         .ifPresent(callbackFacet->{
diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/nonscalar/DataTableModel.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/nonscalar/DataTableModel.java
index 8d49908696..decb8f7e50 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/nonscalar/DataTableModel.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/interactions/managed/nonscalar/DataTableModel.java
@@ -46,11 +46,11 @@ import 
org.apache.isis.core.metamodel.interactions.managed.ActionInteraction;
 import 
org.apache.isis.core.metamodel.interactions.managed.CollectionInteraction;
 import org.apache.isis.core.metamodel.interactions.managed.ManagedAction;
 import 
org.apache.isis.core.metamodel.interactions.managed.ManagedAction.MementoForArgs;
-import org.apache.isis.core.metamodel.object.ManagedObject;
-import org.apache.isis.core.metamodel.object.PackedManagedObject;
 import org.apache.isis.core.metamodel.interactions.managed.ManagedCollection;
 import org.apache.isis.core.metamodel.interactions.managed.ManagedMember;
 import org.apache.isis.core.metamodel.interactions.managed.MultiselectChoices;
+import org.apache.isis.core.metamodel.object.ManagedObject;
+import org.apache.isis.core.metamodel.object.PackedManagedObject;
 import org.apache.isis.core.metamodel.spec.ObjectSpecification;
 import org.apache.isis.core.metamodel.spec.feature.ObjectMember;
 
@@ -126,7 +126,6 @@ implements MultiselectChoices {
             dataElements.getValue().stream()
                 //XXX future extension: filter by searchArgument
                 .filter(this::ignoreHidden)
-                //FIXME[ISIS-3167] entities might be detached
                 .sorted(managedMember.getMetaModel().getElementComparator()
                         .orElseGet(()->(a, b)->0)) // else don't sort (no-op 
comparator for streams)
                 .map(domainObject->new DataRow(this, domainObject))
diff --git 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/object/ManagedObjects.java
 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/object/ManagedObjects.java
index 0964effb90..b74f2ea9f8 100644
--- 
a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/object/ManagedObjects.java
+++ 
b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/object/ManagedObjects.java
@@ -78,19 +78,41 @@ public final class ManagedObjects {
 
     /** whether has at least a spec */
     public static boolean isSpecified(final @Nullable ManagedObject adapter) {
-        return adapter!=null && adapter!=ManagedObject.unspecified();
+        return adapter!=null
+                && adapter!=ManagedObject.unspecified();
     }
 
     /**
      * Optionally given adapter, based on whether it is specified
      * (even if empty, that is, representing null.)
+     * @return SPECIFIED
      */
-    public static Optional<ManagedObject> whenSpecified(final ManagedObject 
adapter) {
+    public static Optional<ManagedObject> asSpecified(final @Nullable 
ManagedObject adapter) {
         return isSpecified(adapter)
                 ? Optional.of(adapter)
                 : Optional.empty();
     }
 
+    /**
+     * Optionally given adapter, based on whether it is specified and scalar 
(not packed)
+     * (even if empty, that is, representing null.)
+     * @return SCALAR or EMTPY
+     */
+    public static Optional<ManagedObject> asScalar(final @Nullable 
ManagedObject adapter) {
+        return asSpecified(adapter)
+                .filter(obj->!obj.getSpecialization().isPacked());
+    }
+
+    /**
+     * Optionally given adapter, based on whether it is specified
+     * (even if empty, that is, representing null.)
+     * @return SCALAR and NOT_EMTPY
+     */
+    public static Optional<ManagedObject> asScalarNonEmpty(final @Nullable 
ManagedObject adapter) {
+        return asScalar(adapter)
+                .filter(obj->!obj.getSpecialization().isEmpty());
+    }
+
     /**
      * whether the corresponding type can be mapped onto a REFERENCE (schema) 
or an Oid,
      * that is, the type is 'identifiable' (aka 'referencable' or 
'bookmarkable')
diff --git 
a/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/executor/MemberExecutorServiceDefault.java
 
b/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/executor/MemberExecutorServiceDefault.java
index 68692ac664..4a0b60dbe8 100644
--- 
a/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/executor/MemberExecutorServiceDefault.java
+++ 
b/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/executor/MemberExecutorServiceDefault.java
@@ -57,7 +57,6 @@ import 
org.apache.isis.core.metamodel.facets.properties.property.modify.Property
 import org.apache.isis.core.metamodel.interactions.InteractionHead;
 import org.apache.isis.core.metamodel.object.ManagedObject;
 import org.apache.isis.core.metamodel.object.ManagedObjects;
-import org.apache.isis.core.metamodel.object.MmEntityUtil;
 import org.apache.isis.core.metamodel.object.MmUnwrapUtil;
 import org.apache.isis.core.metamodel.object.MmVisibilityUtil;
 import org.apache.isis.core.metamodel.object.PackedManagedObject;
@@ -178,6 +177,14 @@ implements MemberExecutorService {
         val returnedAdapter = objectManager.adapt(
                 returnedPojo, owningAction::getElementType);
 
+        // assert has bookmark, unless non-scalar
+        ManagedObjects.asScalarNonEmpty(returnedAdapter)
+        .filter(scalarNonEmpty->!scalarNonEmpty.getSpecialization().isOther()) 
// don't care
+        .ifPresent(scalarNonEmpty->{
+            _Assert.assertTrue(scalarNonEmpty.getBookmark().isPresent(), 
()->String.format(
+                    "bookmark required for non-empty scalars %s", 
scalarNonEmpty.getSpecification()));
+        });
+
         // sync DTO with result
         interactionDtoFactory
         .updateResult(priorExecution.getDto(), owningAction, returnedAdapter);
@@ -276,27 +283,22 @@ implements MemberExecutorService {
         if(ManagedObjects.isNullOrUnspecifiedOrEmpty(resultAdapter)) {
             return;
         }
-
-        val entityState = MmEntityUtil.getEntityState(resultAdapter);
-        //FIXME what to do with new state
+        val entityState = resultAdapter.getEntityState();
+        if(!entityState.isPersistable()) {
+            return;
+        }
         if(entityState.isDetached())   {
             // ensure that any still-to-be-persisted adapters get persisted to 
DB.
             getTransactionService().flushTransaction();
         }
-        if(entityState.isAttached()) {
-            resultAdapter.getBookmark()
-            .ifPresent(bookmark->
-                command.updater().setResult(Try.success(bookmark))
-            );
-        } else {
-            if(entityState.isPersistable()) {
-                log.warn("was unable to get a bookmark for the command result, 
"
-                        + "which is an entity: {}", resultAdapter);
-            }
+        val entityState2 = resultAdapter.getEntityState();
+        if(!entityState2.isDetached()) {
+            val bookmark = ManagedObjects.bookmarkElseFail(resultAdapter);
+            command.updater().setResult(Try.success(bookmark));
+            return;
         }
-
-        // ignore all other sorts of objects
-
+        log.warn("was unable to get a bookmark for the command result, "
+                + "which is an entity: {}", resultAdapter);
     }
 
     private ManagedObject resultFilteredHonoringVisibility(
diff --git 
a/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/factory/FactoryServiceDefault.java
 
b/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/factory/FactoryServiceDefault.java
index 0e0fb5cb0b..0c028485a8 100644
--- 
a/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/factory/FactoryServiceDefault.java
+++ 
b/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/factory/FactoryServiceDefault.java
@@ -92,8 +92,7 @@ public class FactoryServiceDefault implements FactoryService {
         if(!spec.isEntity()) {
             throw _Exceptions.illegalArgument("Type '%s' is not recogniced as 
an entity type by the framework.",
                     entityClass);
-        }
-        
objectLifecyclePublisher().onPostCreate(ManagedObject.entityDetached(spec, 
entityPojo));
+        }        
objectLifecyclePublisher().onPostCreate(ManagedObject.entity(spec, entityPojo, 
Optional.empty()));
         return entityPojo;
     }
 
diff --git 
a/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/publish/LifecycleCallbackNotifier.java
 
b/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/publish/LifecycleCallbackNotifier.java
index 20826aad27..bfc9bdf331 100644
--- 
a/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/publish/LifecycleCallbackNotifier.java
+++ 
b/core/runtimeservices/src/main/java/org/apache/isis/core/runtimeservices/publish/LifecycleCallbackNotifier.java
@@ -123,7 +123,7 @@ public class LifecycleCallbackNotifier {
     private void postLifecycleEventIfRequired(
             final ManagedObject object,
             final Class<? extends LifecycleEventFacet> 
lifecycleEventFacetClass) {
-        ManagedObjects.whenSpecified(object)
+        ManagedObjects.asSpecified(object)
         .map(ManagedObject::getSpecification)
         .ifPresent(spec->{
 

Reply via email to