I forgot one part I had before: to put back in currentMap
Though I wonder why it's not put back in this.stackList
I guess this comment explains it:
// all write operations are local: only remove from the Map on the top of the
stack
So would be complete with
Index: framework/base/src/org/ofbiz/base/util/collections/MapContext.java
===================================================================
--- framework/base/src/org/ofbiz/base/util/collections/MapContext.java
(revision 1480164)
+++ framework/base/src/org/ofbiz/base/util/collections/MapContext.java (working
copy)
@@ -20,6 +20,7 @@
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -251,8 +252,15 @@
*/
public V remove(Object key) {
// all write operations are local: only remove from the Map on the top
of the stack
- Map<K, V> currentMap = this.stackList.get(0);
- return currentMap.remove(key);
+ Map<K, V> currentMap = this.stackList.get(0);
+ try {
+ return currentMap.remove(key);
+ } catch (UnsupportedOperationException e) {
+ Map<K, V> tempMap = new HashMap<K, V>(currentMap);
+ V value = tempMap.remove(key);
+ currentMap = Collections.unmodifiableMap(tempMap);
+ return value;
+ }
}
Jacques
----- Original Message -----
From: "Jacques Le Roux" <[email protected]>
To: <[email protected]>
Sent: Wednesday, May 08, 2013 4:10 PM
Subject: Re: Issue with resetBshInterpreter
You mean that, right? It works but I'm unsure of the remove method call yuo
talked about.
Index: framework/base/src/org/ofbiz/base/util/collections/MapContext.java
===================================================================
--- framework/base/src/org/ofbiz/base/util/collections/MapContext.java
(revision 1480164)
+++ framework/base/src/org/ofbiz/base/util/collections/MapContext.java (working
copy)
@@ -20,6 +20,7 @@
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -251,8 +252,13 @@
*/
public V remove(Object key) {
// all write operations are local: only remove from the Map on the top
of the stack
- Map<K, V> currentMap = this.stackList.get(0);
- return currentMap.remove(key);
+ Map<K, V> currentMap = this.stackList.get(0);
+ try {
+ return currentMap.remove(key);
+ } catch (UnsupportedOperationException e) {
+ Map<K, V> tempMap = new HashMap<K, V>(currentMap);
+ return (V) tempMap.remove(key);
+ }
}
Jacques
From: "Adrian Crum" <[email protected]>
> Just use a try-catch block around the remove method call.
>
> -Adrian
>
> On 5/8/2013 12:30 PM, Jacques Le Roux wrote:
>> A solution could be to rather do the same at MapContext.remove() level
>> But then we need to know if the collection is immutable or not to reset it
>> after change.
>> And it seems there are no reliable ways to know if a Java collection is
>> immutable or not, even using reflection.
>> We could introduce Guava com.google.common.collect.ImmutableCollection or
>> rely on UnsupportedOperationException exceptions
>>
>> What do you think?
>>
>> Jacques
>>
>> From: "Adrian Crum" <[email protected]>
>>> The GenericEntity instance was made immutable for a reason. Please do
>>> not change its behavior.
>>>
>>> -Adrian
>>>
>>> On 5/8/2013 11:04 AM, Jacques Le Roux wrote:
>>>> I just noticed at
>>>> https://demo-trunk.ofbiz.apache.org/catalog/control/FindProductStore
>>>>
>>>> that we have an issue with this line
>>>> this.resetBshInterpreter(localContext);
>>>>
>>>> That we find twice in ModelForm.java.
>>>>
>>>> From stack trace it could related to immutable being introduced in the
>>>> entity engine recently
>>>>
>>>> ---- cause
>>>> ---------------------------------------------------------------------
>>>> Exception: java.lang.UnsupportedOperationException
>>>> Message: null
>>>> ---- stack trace
>>>> ---------------------------------------------------------------
>>>> java.lang.UnsupportedOperationException
>>>> java.util.Collections$UnmodifiableMap.remove(Collections.java:1288)
>>>> org.ofbiz.entity.GenericEntity.remove(GenericEntity.java:1451)
>>>> org.ofbiz.base.util.collections.MapContext.remove(MapContext.java:255)
>>>> org.ofbiz.widget.form.ModelForm.resetBshInterpreter(ModelForm.java:2139)
>>>>
>>>>
>>>> I used this
>>>>
>>>> Index: framework/entity/src/org/ofbiz/entity/GenericEntity.java
>>>> ===================================================================
>>>> --- framework/entity/src/org/ofbiz/entity/GenericEntity.java (revision
>>>> 1480164)
>>>> +++ framework/entity/src/org/ofbiz/entity/GenericEntity.java (working copy)
>>>> @@ -314,6 +314,13 @@
>>>> }
>>>> }
>>>>
>>>> + public void setMutable() {
>>>> + if (!this.mutable) {
>>>> + this.mutable = true;
>>>> + this.fields = new HashMap<String, Object>(this.fields);
>>>> + }
>>>> + }
>>>> +
>>>> /**
>>>> * @return Returns the isFromEntitySync.
>>>> */
>>>> @@ -1448,7 +1455,10 @@
>>>> // ---- Methods added to implement the Map interface: ----
>>>>
>>>> public Object remove(Object key) {
>>>> - return this.fields.remove(key);
>>>> + setMutable();
>>>> + this.fields.remove(key);
>>>> + setImmutable();
>>>> + return this.fields;
>>>> }
>>>>
>>>> public boolean containsKey(Object key) {
>>>>
>>>>
>>>> I can commit if you are ok with it
>>>>
>>>> Jacques
>
