Note that this would not remove the bshInterpreter key/value pair which is what resetBshInterpreter tries to do
Anyway, we agree even resetBshInterpreter is weird... Jacques 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. > > 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 >
