From: "Adrian Crum" <[email protected]>
> return currentMap.remove(key);
>
> is the remove method call.
>
> There is no point removing an element in a copy of the Map:
>
> Map<K, V> currentMap = this.stackList.get(0);
> try {
> return currentMap.remove(key);
> } catch (UnsupportedOperationException e) {
> return (V) currentMap.get(key);
> -OR-
> return null;
> }
>
> But that simply hides the problem. The code calling this method should do the
> try-catch.
Yes I was thinking that too. But the problem is more general. As the stack I
posted shows it (here again). It's in ModelForm, and not dependend of a
specific screen widget
---- 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)
The screen and form are quite standard. It's rather related to this
resetBshInterpreter call which we could maybe get rid off. But before
considering anything like that I preferred a non intrusive way
Jacques
> You said the problem is in one of the screen widgets - so that is where it
> should be fixed. Why is a widget removing elements from a Map? So, there is a
> design flaw in there somewhere.
>
> Just let me know where in the screen widget code you encountered the problem.
> I caused it, so I should fix it.
>
> -Adrian
>
> On 5/8/2013 3:10 PM, Jacques Le Roux wrote:
>> 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
>
