From e9498be580c6b22de9f4fc7a26f23c62c94c376c Mon Sep 17 00:00:00 2001 From: Jay Shaughnessy Date: Fri, 25 Jul 2014 18:22:44 -0400 Subject: [PATCH] In a recent commit for [1120417] (and the resulting oracle test failures) we added protection against Hibernate errors related to detached sets when orphanRemoval=true. This applies similar changes to entity classes outside of the BZ work. (cherry picked from commit 0e8b7ed217d5ec91ac1313e5fc87829c330b4ca9) Signed-off-by: Jay Shaughnessy --- .../domain/configuration/Configuration.java | 10 ++++-- .../domain/configuration/PropertyList.java | 6 ++-- .../domain/configuration/PropertyMap.java | 35 +++++++++++-------- .../definition/ConfigurationDefinition.java | 25 +++++++++---- .../rhq/core/domain/dashboard/Dashboard.java | 11 +++--- .../rhq/core/domain/resource/Resource.java | 16 +++++++-- 6 files changed, 68 insertions(+), 35 deletions(-) diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java index a7e9887a5ce..4abb6c5ffad 100644 --- a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java +++ b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/Configuration.java @@ -390,8 +390,8 @@ public Configuration build() { // use the prop name as the map key @MapKey(name = "name") // CascadeType.REMOVE has been omitted, the cascade delete has been moved to the data model for performance - @Cascade({ CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH, CascadeType.DELETE_ORPHAN }) - @OneToMany(mappedBy = "configuration", fetch = FetchType.EAGER) + @Cascade({ CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH }) + @OneToMany(mappedBy = "configuration", fetch = FetchType.EAGER, orphanRemoval = true) @XmlTransient private Map properties = new LinkedHashMap(1); @@ -771,6 +771,8 @@ public void setProperties(Collection properties) { return; } + // Don't replace the possible Hibernate proxy when orphanRemoval=true. It can cause + // "collection with cascade=all-delete-orphan was no longer referenced" exceptions. this.properties.clear(); for (Property p : properties) { this.put(p); @@ -866,6 +868,10 @@ public void cleanoutRawConfiguration() { } } + /** + * Warning: This should probably not be performed on an attached entity, it could replace a + * Hibernate proxy, which can lead to issues (especially when orphanRemoval=true) + */ public void resize() { Map tmp =new LinkedHashMap(this.properties.size()); tmp.putAll(this.properties); diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyList.java b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyList.java index 5b1f40a949e..e48fbec8ab4 100644 --- a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyList.java +++ b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyList.java @@ -59,9 +59,9 @@ public class PropertyList extends Property { private static final long serialVersionUID = 1L; - // CascadeType.REMOVE has been omitted, the cascade delete has been moved to the data model for performance - @Cascade( { CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH, CascadeType.DELETE_ORPHAN }) - @OneToMany(mappedBy = "parentList", targetEntity = Property.class, fetch = FetchType.EAGER) + // CascadeType.REMOVE has been omitted, the cascade delete has been moved to the data model for performance + @Cascade({ CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH }) + @OneToMany(mappedBy = "parentList", targetEntity = Property.class, fetch = FetchType.EAGER, orphanRemoval = true) // Order by primary key which will also put the list elements in chronological order. // Note, if we decide at some point to add support in the GUI for reordering list elements, we'll // need to add a new ORDER column and order by that. diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyMap.java b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyMap.java index bc4a6b9a979..245a6d36f99 100644 --- a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyMap.java +++ b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/PropertyMap.java @@ -60,9 +60,9 @@ public class PropertyMap extends Property implements AbstractPropertyMap { // use the prop name as the map key @MapKey(name = "name") - // CascadeType.REMOVE has been omitted, the cascade delete has been moved to the data model for performance - @Cascade({ CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH, CascadeType.DELETE_ORPHAN }) - @OneToMany(mappedBy = "parentMap", fetch = FetchType.EAGER) + // CascadeType.REMOVE has been omitted, the cascade delete has been moved to the data model for performance + @Cascade({ CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH }) + @OneToMany(mappedBy = "parentMap", fetch = FetchType.EAGER, orphanRemoval = true) private Map map = new LinkedHashMap(); /** @@ -80,7 +80,8 @@ protected PropertyMap(PropertyMap original, boolean keepId) { * * @param name the name of the map itself */ - public PropertyMap(@NotNull String name) { + public PropertyMap(@NotNull + String name) { setName(name); } @@ -92,7 +93,8 @@ public PropertyMap(@NotNull String name) { * @param name the name of the map itself * @param startingProperties a set of properties to be immediately added to this map */ - public PropertyMap(@NotNull String name, Property... startingProperties) { + public PropertyMap(@NotNull + String name, Property... startingProperties) { this(name); for (Property property : startingProperties) { put(property); @@ -118,17 +120,21 @@ public Map getMap() { } /** - * Sets the map of child properties directly to the given map reference. This means the actual - * map object is stored internally in this object. Changes made to map will be reflected back - * into this object. + * This clears the current internal map replaces it with all of the provided map entries. * - *

Warning: Caution should be used when setting this object's internal map. Please see - * {@link PropertyMap the javadoc for this class} for more information.

- * - * @param map the new map used internally by this object + * @param map the map providing the new mappings */ public void setMap(Map map) { - this.map = map; + if (this.map == map) { + return; + } + // Don't replace the possible Hibernate proxy when orphanRemoval=true. It can cause + // "collection with cascade=all-delete-orphan was no longer referenced" exceptions. + this.map = getMap(); + this.map.clear(); + if (null != map) { + this.map.putAll(map); + } } /** @@ -137,7 +143,8 @@ public void setMap(Map map) { * * @param property the property to add to this map. */ - public void put(@NotNull Property property) { + public void put(@NotNull + Property property) { getMap().put(property.getName(), property); property.setParentMap(this); } diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/definition/ConfigurationDefinition.java b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/definition/ConfigurationDefinition.java index 73756199c15..14d5ae04e86 100644 --- a/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/definition/ConfigurationDefinition.java +++ b/modules/core/domain/src/main/java/org/rhq/core/domain/configuration/definition/ConfigurationDefinition.java @@ -97,8 +97,7 @@ public class ConfigurationDefinition implements Serializable { // plugin descriptor, iterating on the map (LinkedHashMap) will give us the same ordering. So, unless // propDef.order is set and used for ordering by the accessing code, we'll default to the descriptor order. @OrderBy - @org.hibernate.annotations.Cascade(org.hibernate.annotations.CascadeType.DELETE_ORPHAN) - @OneToMany(mappedBy = "configurationDefinition", cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @OneToMany(mappedBy = "configurationDefinition", cascade = CascadeType.ALL, fetch = FetchType.EAGER, orphanRemoval = true) private Map propertyDefinitions = new LinkedHashMap(); // use the configTemplate name as the map key @@ -117,7 +116,8 @@ protected ConfigurationDefinition() { // JPA use only } - public ConfigurationDefinition(@NotNull String name, String description) { + public ConfigurationDefinition(@NotNull + String name, String description) { this.name = name.intern(); this.description = description; } @@ -135,9 +135,10 @@ public String getName() { return name; } - public void setName(@NotNull String name) { + public void setName(@NotNull + String name) { // Need to protect due to possible deserialization from Coregui. - if (name!=null) { + if (name != null) { this.name = name.intern(); } else { this.name = null; @@ -187,7 +188,16 @@ public Map getPropertyDefinitions() { } public void setPropertyDefinitions(Map propertyDefinitions) { - this.propertyDefinitions = propertyDefinitions; + if (this.propertyDefinitions == propertyDefinitions) { + return; + } + // Don't replace the possible Hibernate proxy when orphanRemoval=true. It can cause + // "collection with cascade=all-delete-orphan was no longer referenced" exceptions. + this.propertyDefinitions = getPropertyDefinitions(); + this.propertyDefinitions.clear(); + if (null != propertyDefinitions) { + this.propertyDefinitions.putAll(propertyDefinitions); + } } public void put(PropertyDefinition propertyDefinition) { @@ -332,7 +342,8 @@ public ConfigurationTemplate getDefaultTemplate() { * @return ConfigurationTemplate with the specified name; null if no template by that name * exists. */ - public ConfigurationTemplate getTemplate(@NotNull String name) { + public ConfigurationTemplate getTemplate(@NotNull + String name) { return getTemplates().get(name); } diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/dashboard/Dashboard.java b/modules/core/domain/src/main/java/org/rhq/core/domain/dashboard/Dashboard.java index 90e08c6e016..bd48e2d5a7a 100644 --- a/modules/core/domain/src/main/java/org/rhq/core/domain/dashboard/Dashboard.java +++ b/modules/core/domain/src/main/java/org/rhq/core/domain/dashboard/Dashboard.java @@ -104,9 +104,8 @@ public class Dashboard implements Serializable { @ManyToOne(fetch = FetchType.LAZY, optional = true) private ResourceGroup group; - @OneToMany(mappedBy = "dashboard", fetch = FetchType.EAGER) - @Cascade( { org.hibernate.annotations.CascadeType.PERSIST, org.hibernate.annotations.CascadeType.MERGE, - org.hibernate.annotations.CascadeType.DELETE_ORPHAN }) + @OneToMany(mappedBy = "dashboard", fetch = FetchType.EAGER, orphanRemoval = true) + @Cascade({ org.hibernate.annotations.CascadeType.PERSIST, org.hibernate.annotations.CascadeType.MERGE }) private Set portlets = new HashSet(); public static final String CFG_COLUMNS = "columns"; @@ -251,10 +250,10 @@ public boolean removePortlet(DashboardPortlet storedPortlet) { return true; } - /** + /** * This can be used to safely add a portlet without knowing the current portlet positioning on the * Dashboard. It adds the portlet to the bottom of column with the least portlets. - * + * * @param storedPortlet MODIFIED with assigned column, index */ public void addPortlet(DashboardPortlet storedPortlet) { @@ -279,7 +278,7 @@ public void addPortlet(DashboardPortlet storedPortlet) { /** * Call this only if you are sure the column and index are valid, not already used and not leaving gaps. - * + * * @param storedPortlet MODIFIED with assigned column, index * @param column * @param index diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java b/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java index 7dd5bbfa496..3c11f66800b 100644 --- a/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java +++ b/modules/core/domain/src/main/java/org/rhq/core/domain/resource/Resource.java @@ -1170,8 +1170,7 @@ public class Resource implements Comparable, Serializable { @OneToMany(mappedBy = "resource", fetch = FetchType.LAZY, cascade = CascadeType.REMOVE) private Set dashboards = null; - @OneToMany(mappedBy = "resource", fetch = FetchType.LAZY, cascade = { CascadeType.ALL }) - @org.hibernate.annotations.Cascade(org.hibernate.annotations.CascadeType.DELETE_ORPHAN) + @OneToMany(mappedBy = "resource", fetch = FetchType.LAZY, cascade = { CascadeType.ALL }, orphanRemoval = true) private Set driftDefinitions = null; @Transient @@ -1961,7 +1960,18 @@ public Set getDriftDefinitions() { } public void setDriftDefinitions(Set driftDefinitions) { - this.driftDefinitions = driftDefinitions; + if (this.driftDefinitions == driftDefinitions) { + return; + } + // Don't replace the possible Hibernate proxy when orphanRemoval=true. It can cause + // "collection with cascade=all-delete-orphan was no longer referenced" exceptions. + this.driftDefinitions = getDriftDefinitions(); + this.driftDefinitions.clear(); + if (null != driftDefinitions) { + for (DriftDefinition dd : driftDefinitions) { + addDriftDefinition(dd); + } + } } public void addDriftDefinition(DriftDefinition driftDefinition) {