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 2020/01/22 13:10:37 UTC
[isis] branch master updated (f05bb7c -> 5d3388d)
This is an automated email from the ASF dual-hosted git repository.
ahuber pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git.
from f05bb7c ISIS-2158: supposed thread-safety fix for EntityPage serialization
new ad1cc65 ISIS-2158: don't NPE on collectionLayoutData not found
new 5d3388d ISIS-2158: supposed thread-safety fix for EntityPage serialization (2)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../layout/component/ActionLayoutDataOwner.java | 4 +-
.../layout/component/CollectionLayoutData.java | 22 ++++-----
.../isis/applib/layout/component/FieldSet.java | 31 ++++++------
.../layout/component/PropertyLayoutData.java | 14 ++++--
.../isis/applib/layout/grid/bootstrap3/BS3Col.java | 15 +++---
.../_ConcurrentListWrapper.java} | 57 +++++++++++++---------
.../commons/internal/exceptions/_Exceptions.java | 35 +++++++++++++
.../services/grid/GridLoaderServiceDefault.java | 16 +++---
.../grid/bootstrap3/GridSystemServiceBS3.java | 20 ++++----
9 files changed, 139 insertions(+), 75 deletions(-)
copy core/commons/src/main/java/org/apache/isis/core/commons/internal/{base/_Oneshot.java => concurrent/_ConcurrentListWrapper.java} (53%)
[isis] 02/02: ISIS-2158: supposed thread-safety fix for EntityPage
serialization (2)
Posted by ah...@apache.org.
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
commit 5d3388d85f043616bffe630e2ee12da66e1f941b
Author: Andi Huber <ah...@apache.org>
AuthorDate: Wed Jan 22 14:10:26 2020 +0100
ISIS-2158: supposed thread-safety fix for EntityPage serialization (2)
---
.../layout/component/ActionLayoutDataOwner.java | 4 +-
.../layout/component/CollectionLayoutData.java | 22 +++----
.../isis/applib/layout/component/FieldSet.java | 31 +++++----
.../layout/component/PropertyLayoutData.java | 14 ++--
.../isis/applib/layout/grid/bootstrap3/BS3Col.java | 15 +++--
.../concurrent/_ConcurrentListWrapper.java | 77 ++++++++++++++++++++++
.../commons/internal/exceptions/_Exceptions.java | 35 ++++++++++
.../services/grid/GridLoaderServiceDefault.java | 16 +++--
.../grid/bootstrap3/GridSystemServiceBS3.java | 10 +--
9 files changed, 173 insertions(+), 51 deletions(-)
diff --git a/api/applib/src/main/java/org/apache/isis/applib/layout/component/ActionLayoutDataOwner.java b/api/applib/src/main/java/org/apache/isis/applib/layout/component/ActionLayoutDataOwner.java
index c4b07bb..c4153d0 100644
--- a/api/applib/src/main/java/org/apache/isis/applib/layout/component/ActionLayoutDataOwner.java
+++ b/api/applib/src/main/java/org/apache/isis/applib/layout/component/ActionLayoutDataOwner.java
@@ -21,6 +21,8 @@ package org.apache.isis.applib.layout.component;
import java.util.List;
public interface ActionLayoutDataOwner extends Owner {
+
List<ActionLayoutData> getActions();
- void setActions(List<ActionLayoutData> actions);
+ void addAction(ActionLayoutData actionLayoutData);
+
}
diff --git a/api/applib/src/main/java/org/apache/isis/applib/layout/component/CollectionLayoutData.java b/api/applib/src/main/java/org/apache/isis/applib/layout/component/CollectionLayoutData.java
index 90a4e34..6991c75 100644
--- a/api/applib/src/main/java/org/apache/isis/applib/layout/component/CollectionLayoutData.java
+++ b/api/applib/src/main/java/org/apache/isis/applib/layout/component/CollectionLayoutData.java
@@ -23,13 +23,14 @@ import java.util.List;
import javax.xml.bind.annotation.XmlAttribute;
import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlElementRef;
import javax.xml.bind.annotation.XmlRootElement;
import javax.xml.bind.annotation.XmlTransient;
import javax.xml.bind.annotation.XmlType;
import org.apache.isis.applib.annotation.Where;
import org.apache.isis.applib.layout.links.Link;
-import org.apache.isis.core.commons.collections.Can;
+import org.apache.isis.core.commons.internal.concurrent._ConcurrentListWrapper;
/**
* Describes the layout of a single collection, broadly corresponds to the {@link org.apache.isis.applib.annotation.CollectionLayout} annotation.
@@ -196,23 +197,22 @@ HasCssClass, HasDescribedAs, HasHidden, HasNamed {
this.sortedBy = sortedBy;
}
- private final Object $actionsLock = new Object[0]; // serializable
- private Can<ActionLayoutData> actions = Can.empty(); // serializable
+ private _ConcurrentListWrapper<ActionLayoutData> actions = new _ConcurrentListWrapper<>();
// no wrapper
@Override
- @XmlElement(name = "action", required = false)
+ @XmlElementRef(type = ActionLayoutData.class, name = "action", required = false)
public List<ActionLayoutData> getActions() {
- synchronized ($actionsLock) {
- return actions.toList();
- }
+ return actions.snapshot();
}
- @Override
public void setActions(List<ActionLayoutData> actionLayoutDatas) {
- synchronized ($actionsLock) {
- this.actions = Can.ofCollection(actionLayoutDatas);
- }
+ this.actions.replace(actionLayoutDatas);
+ }
+
+ @Override
+ public void addAction(ActionLayoutData actionLayoutData) {
+ this.actions.add(actionLayoutData);
}
private CollectionLayoutDataOwner owner;
diff --git a/api/applib/src/main/java/org/apache/isis/applib/layout/component/FieldSet.java b/api/applib/src/main/java/org/apache/isis/applib/layout/component/FieldSet.java
index 335d65a..c5e071b 100644
--- a/api/applib/src/main/java/org/apache/isis/applib/layout/component/FieldSet.java
+++ b/api/applib/src/main/java/org/apache/isis/applib/layout/component/FieldSet.java
@@ -19,7 +19,6 @@
package org.apache.isis.applib.layout.component;
import java.io.Serializable;
-import java.util.ArrayList;
import java.util.List;
import javax.xml.bind.annotation.XmlAttribute;
@@ -30,6 +29,7 @@ import javax.xml.bind.annotation.XmlType;
import org.apache.isis.applib.annotation.MemberOrder;
import org.apache.isis.applib.layout.grid.bootstrap3.BS3Col;
+import org.apache.isis.core.commons.internal.concurrent._ConcurrentListWrapper;
/**
* A {@link MemberRegion region} of the page containing a set of
@@ -133,36 +133,39 @@ Serializable {
this.name = name;
}
-
-
- private List<ActionLayoutData> actions = new ArrayList<>();
+ private _ConcurrentListWrapper<ActionLayoutData> actions = new _ConcurrentListWrapper<>();
// no wrapper
@Override
@XmlElement(name = "action", required = false)
public List<ActionLayoutData> getActions() {
- return actions;
+ return actions.snapshot();
}
- @Override
public void setActions(List<ActionLayoutData> actionLayoutDatas) {
- this.actions = actionLayoutDatas;
+ this.actions.replace(actionLayoutDatas);
}
-
-
-
- private List<PropertyLayoutData> properties = new ArrayList<>();
+
+ @Override
+ public void addAction(ActionLayoutData actionLayoutData) {
+ this.actions.add(actionLayoutData);
+ }
+
+ private _ConcurrentListWrapper<PropertyLayoutData> properties = new _ConcurrentListWrapper<>();
// no wrapper; required=false because may be auto-generated
@XmlElement(name = "property", required = false)
public List<PropertyLayoutData> getProperties() {
- return properties;
+ return properties.snapshot();
}
public void setProperties(List<PropertyLayoutData> properties) {
- this.properties = properties;
+ this.properties.replace(properties);
+ }
+
+ public void addProperty(PropertyLayoutData actionLayoutData) {
+ this.properties.add(actionLayoutData);
}
-
private FieldSetOwner owner;
/**
diff --git a/api/applib/src/main/java/org/apache/isis/applib/layout/component/PropertyLayoutData.java b/api/applib/src/main/java/org/apache/isis/applib/layout/component/PropertyLayoutData.java
index 817a6aa..a782ff4 100644
--- a/api/applib/src/main/java/org/apache/isis/applib/layout/component/PropertyLayoutData.java
+++ b/api/applib/src/main/java/org/apache/isis/applib/layout/component/PropertyLayoutData.java
@@ -19,7 +19,6 @@
package org.apache.isis.applib.layout.component;
import java.io.Serializable;
-import java.util.ArrayList;
import java.util.List;
import javax.xml.bind.annotation.XmlAttribute;
@@ -34,6 +33,7 @@ import org.apache.isis.applib.annotation.RenderDay;
import org.apache.isis.applib.annotation.Repainting;
import org.apache.isis.applib.annotation.Where;
import org.apache.isis.applib.layout.links.Link;
+import org.apache.isis.core.commons.internal.concurrent._ConcurrentListWrapper;
/**
* Describes the layout of a single property, broadly corresponds to the {@link org.apache.isis.applib.annotation.PropertyLayout} annotation.
@@ -234,18 +234,22 @@ HasCssClass, HasDescribedAs, HasHidden, HasNamed {
this.repainting = repainting;
}
- private List<ActionLayoutData> actions = new ArrayList<>();
+ private _ConcurrentListWrapper<ActionLayoutData> actions = new _ConcurrentListWrapper<>();
// no wrapper
@Override
@XmlElement(name = "action", required = false)
public List<ActionLayoutData> getActions() {
- return actions;
+ return actions.snapshot();
}
- @Override
public void setActions(List<ActionLayoutData> actionLayoutDatas) {
- this.actions = actionLayoutDatas;
+ this.actions.replace(actionLayoutDatas);
+ }
+
+ @Override
+ public void addAction(ActionLayoutData actionLayoutData) {
+ this.actions.add(actionLayoutData);
}
diff --git a/api/applib/src/main/java/org/apache/isis/applib/layout/grid/bootstrap3/BS3Col.java b/api/applib/src/main/java/org/apache/isis/applib/layout/grid/bootstrap3/BS3Col.java
index 7f44432..439b9b1 100644
--- a/api/applib/src/main/java/org/apache/isis/applib/layout/grid/bootstrap3/BS3Col.java
+++ b/api/applib/src/main/java/org/apache/isis/applib/layout/grid/bootstrap3/BS3Col.java
@@ -35,6 +35,7 @@ import org.apache.isis.applib.layout.component.DomainObjectLayoutData;
import org.apache.isis.applib.layout.component.DomainObjectLayoutDataOwner;
import org.apache.isis.applib.layout.component.FieldSet;
import org.apache.isis.applib.layout.component.FieldSetOwner;
+import org.apache.isis.core.commons.internal.concurrent._ConcurrentListWrapper;
/**
* A column within a row which, depending on its {@link #getSpan()}, could be as narrow as 1/12th of the page's width, all the way up to spanning the entire page.
@@ -171,23 +172,25 @@ CollectionLayoutDataOwner, DomainObjectLayoutDataOwner {
}
-
- private List<ActionLayoutData> actions = new ArrayList<>();
+ private _ConcurrentListWrapper<ActionLayoutData> actions = new _ConcurrentListWrapper<>();
// no wrapper
@Override
@XmlElementRef(type = ActionLayoutData.class, name = "action", required = false)
public List<ActionLayoutData> getActions() {
- return actions;
+ return actions.snapshot();
}
+ public void setActions(List<ActionLayoutData> actionLayoutDatas) {
+ this.actions.replace(actionLayoutDatas);
+ }
+
@Override
- public void setActions(final List<ActionLayoutData> actions) {
- this.actions = actions;
+ public void addAction(ActionLayoutData actionLayoutData) {
+ this.actions.add(actionLayoutData);
}
-
private List<BS3Row> rows = new ArrayList<>();
// no wrapper
diff --git a/core/commons/src/main/java/org/apache/isis/core/commons/internal/concurrent/_ConcurrentListWrapper.java b/core/commons/src/main/java/org/apache/isis/core/commons/internal/concurrent/_ConcurrentListWrapper.java
new file mode 100644
index 0000000..edc1d4e
--- /dev/null
+++ b/core/commons/src/main/java/org/apache/isis/core/commons/internal/concurrent/_ConcurrentListWrapper.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.isis.core.commons.internal.concurrent;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.annotation.Nullable;
+
+/**
+ * <h1>- internal use only -</h1>
+ *
+ * Provides limited thread-safe access to a list object, allowing add/replace/clear/snapshot.
+ * <p>
+ * Serializable
+ *
+ * <p>
+ * <b>WARNING</b>: Do <b>NOT</b> use any of the classes provided by this package! <br/>
+ * These may be changed or removed without notice!
+ * </p>
+ *
+ * @since 2.0
+ */
+public class _ConcurrentListWrapper<T> implements Serializable {
+
+ private static final long serialVersionUID = 1L;
+
+ private final Object $lock = new Object[0]; // serializable
+ private List<T> list = new ArrayList<>(); // serializable
+
+ public List<T> snapshot() {
+ synchronized ($lock) {
+ return new ArrayList<>(list);
+ }
+ }
+
+ public void replace(@Nullable List<T> list) {
+ synchronized ($lock) {
+ this.list = list!=null
+ ? new ArrayList<>(list)
+ : new ArrayList<>();
+ }
+ }
+
+ public void clear() {
+ synchronized ($lock) {
+ this.list = new ArrayList<>(); // allow garbage collection of the old one
+ }
+ }
+
+ public void add(@Nullable T element) {
+ if(element==null) {
+ return; // noop
+ }
+ synchronized ($lock) {
+ this.list.add(element);
+ }
+ }
+
+}
diff --git a/core/commons/src/main/java/org/apache/isis/core/commons/internal/exceptions/_Exceptions.java b/core/commons/src/main/java/org/apache/isis/core/commons/internal/exceptions/_Exceptions.java
index fd1f314..eb2ba12 100644
--- a/core/commons/src/main/java/org/apache/isis/core/commons/internal/exceptions/_Exceptions.java
+++ b/core/commons/src/main/java/org/apache/isis/core/commons/internal/exceptions/_Exceptions.java
@@ -31,6 +31,7 @@ import java.util.stream.Stream;
import javax.annotation.Nullable;
+import org.apache.isis.core.commons.collections.Can;
import org.apache.isis.core.commons.exceptions.IsisException;
import org.apache.isis.core.commons.internal.base._NullSafe;
import org.apache.isis.core.commons.internal.base._Strings;
@@ -132,6 +133,38 @@ public final class _Exceptions {
return new UnsupportedOperationException("unrecoverable error: method call not allowed/supported");
}
+ // -- MESSAGE
+
+ public static String getMessage(Exception ex) {
+ if(ex==null) {
+ return "no exception present";
+ }
+ if(_Strings.isNotEmpty(ex.getMessage())) {
+ return ex.getMessage();
+ }
+ val sb = new StringBuilder();
+ val nestedMsg = streamCausalChain(ex)
+ .peek(throwable->{
+ sb.append(throwable.getClass().getSimpleName()).append("/");
+ })
+ .map(Throwable::getMessage)
+ .filter(_NullSafe::isPresent)
+ .findFirst();
+
+ if(nestedMsg.isPresent()) {
+ sb.append(nestedMsg.get());
+ } else {
+
+ Can.ofArray(ex.getStackTrace())
+ .stream()
+ .limit(20)
+ .forEach(trace->sb.append("\n").append(trace));
+ }
+
+ return sb.toString();
+ }
+
+ // -- THROWING
/**
* Used to hide from the compiler the fact, that this call always throws.
@@ -405,6 +438,8 @@ public final class _Exceptions {
}
}
+
+
}
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/GridLoaderServiceDefault.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/GridLoaderServiceDefault.java
index 16c1ecb..1819b9e 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/GridLoaderServiceDefault.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/GridLoaderServiceDefault.java
@@ -39,8 +39,10 @@ import org.apache.isis.applib.services.message.MessageService;
import org.apache.isis.core.commons.internal.collections._Lists;
import org.apache.isis.core.commons.internal.collections._Maps;
import org.apache.isis.core.commons.internal.environment.IsisSystemEnvironment;
+import org.apache.isis.core.commons.internal.exceptions._Exceptions;
import org.apache.isis.core.commons.internal.resources._Resources;
+import lombok.val;
import lombok.extern.log4j.Log4j2;
@Service
@@ -51,7 +53,10 @@ import lombok.extern.log4j.Log4j2;
@Log4j2
public class GridLoaderServiceDefault implements GridLoaderService {
-
+ @Inject private MessageService messageService;
+ @Inject private GridReaderUsingJaxb gridReader;
+ @Inject private IsisSystemEnvironment isisSystemEnvironment;
+
static class DomainClassAndLayout {
private final Class<?> domainClass;
private final String layoutIfAny;
@@ -183,6 +188,8 @@ public class GridLoaderServiceDefault implements GridLoaderService {
}
return grid;
} catch(Exception ex) {
+
+ val exceptionMessage = _Exceptions.getMessage(ex);
if(supportsReloading()) {
// save fact that this was bad XML, so that we don't log again if called next time
@@ -192,7 +199,7 @@ public class GridLoaderServiceDefault implements GridLoaderService {
// note that we don't blacklist if the file exists but couldn't be parsed;
// the developer might fix so we will want to retry.
final String resourceName = resourceNameFor(dcal);
- final String message = "Failed to parse " + resourceName + " file (" + ex.getMessage() + ")";
+ final String message = "Failed to parse " + resourceName + " file (" + exceptionMessage + ")";
if(supportsReloading()) {
messageService.warnUser(message);
}
@@ -248,10 +255,5 @@ public class GridLoaderServiceDefault implements GridLoaderService {
return null;
}
- // -- DEPENDENCIES
-
- @Inject MessageService messageService;
- @Inject GridReaderUsingJaxb gridReader;
- @Inject IsisSystemEnvironment isisSystemEnvironment;
}
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java
index 0c9d0d0..0e3fafa 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java
@@ -521,7 +521,7 @@ public class GridSystemServiceBS3 extends GridSystemServiceAbstract<BS3Grid> {
// action to the property
val collectionLayoutData = collectionLayoutDataById.get(id);
if(collectionLayoutData==null) {
- log.warn("failed to lookup CollectionLayoutData by id '{}'", id);
+ log.warn("Failed to lookup CollectionLayoutData by id '{}'", id);
} else {
val actionLayoutData = new ActionLayoutData(actionId);
addActionTo(collectionLayoutData, actionLayoutData);
@@ -582,7 +582,7 @@ public class GridSystemServiceBS3 extends GridSystemServiceAbstract<BS3Grid> {
for (final String propertyId : propertyIds) {
if(!existingIds.contains(propertyId)) {
final PropertyLayoutData propertyLayoutData = new PropertyLayoutData(propertyId);
- fieldSet.getProperties().add(propertyLayoutData);
+ fieldSet.addProperty(propertyLayoutData);
propertyLayoutData.setOwner(fieldSet);
propertyLayoutDataById.put(propertyId, propertyLayoutData);
}
@@ -652,11 +652,7 @@ public class GridSystemServiceBS3 extends GridSystemServiceAbstract<BS3Grid> {
private void addActionTo(
final ActionLayoutDataOwner owner,
final ActionLayoutData actionLayoutData) {
- List<ActionLayoutData> actions = owner.getActions();
- if(actions == null) {
- owner.setActions(actions = _Lists.newArrayList());
- }
- actions.add(actionLayoutData);
+ owner.addAction(actionLayoutData);
actionLayoutData.setOwner(owner);
}
[isis] 01/02: ISIS-2158: don't NPE on collectionLayoutData not found
Posted by ah...@apache.org.
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
commit ad1cc65b8e382437a0fb2b58e815c5df1d5f39bc
Author: Andi Huber <ah...@apache.org>
AuthorDate: Wed Jan 22 13:25:22 2020 +0100
ISIS-2158: don't NPE on collectionLayoutData not found
---
.../services/grid/bootstrap3/GridSystemServiceBS3.java | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java
index cbe8f23..0c9d0d0 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/grid/bootstrap3/GridSystemServiceBS3.java
@@ -72,12 +72,14 @@ import org.apache.isis.core.metamodel.spec.feature.OneToOneAssociation;
import static org.apache.isis.core.commons.internal.base._NullSafe.stream;
import lombok.val;
+import lombok.extern.log4j.Log4j2;
@Service
@Named("isisMetaModel.GridSystemServiceBS3")
@Order(OrderPrecedence.MIDPOINT)
@Primary
@Qualifier("BS3")
+@Log4j2
public class GridSystemServiceBS3 extends GridSystemServiceAbstract<BS3Grid> {
public static final String TNS = "http://isis.apache.org/applib/layout/grid/bootstrap3";
@@ -517,9 +519,13 @@ public class GridSystemServiceBS3 extends GridSystemServiceAbstract<BS3Grid> {
if(!(memberOrderFacet instanceof MemberOrderFacetAnnotation)) {
// if binding not via annotation, then explicitly bind this
// action to the property
- final CollectionLayoutData collectionLayoutData = collectionLayoutDataById.get(id);
- final ActionLayoutData actionLayoutData = new ActionLayoutData(actionId);
- addActionTo(collectionLayoutData, actionLayoutData);
+ val collectionLayoutData = collectionLayoutDataById.get(id);
+ if(collectionLayoutData==null) {
+ log.warn("failed to lookup CollectionLayoutData by id '{}'", id);
+ } else {
+ val actionLayoutData = new ActionLayoutData(actionId);
+ addActionTo(collectionLayoutData, actionLayoutData);
+ }
}
continue;
}