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
>

Reply via email to