GROOVY-8065: Map created as org.springframework.http.HttpHeaders is e…

[paulk-asert]

…mpty in 2.4.8, works fine in 2.4.7

[blackdrag]

The problem is that the following test now fails:

class X {
public String foo
void set(String name, String value) { }
}
def x = new X()
x.foo = "2"
assert x.@foo == "2"

[paulk-asert]

Do we have the priority documented anywhere or just in the code? We don't have test coverage for your example.

[blackdrag]

I think no documentation and no test. I extracted the example from the code you changed actually to show you what your change will affect. In my opinion the get/set path needs to be removed, but that is another topic. The trouble is that get/set in the MOP is in general very badly documented. So it might or it might not be, that your change is a breaking one. Well no, it is a breaking change in terms of semantics, but if real world examples will be badly affected by this? no idea. But I have another example:

class A extends HashMap {
public foo
}

def a = new A()
a.x = 1
assert a.x == 1
a.foo = 2
assert a.@foo == 2
assert a.foo == null
assert a == [x:1]

the last three asserts will all fail.

[jwagenleitner]

I think restoring the code to the way it was pre- 2c22683 commit and then partially duplicating the map check prior to throwing the ReadOnlyPropertyException exception would allow all tests to pass.

.....
            if (Modifier.isFinal(field.getModifiers())) {
                // GROOVY-5985
                if (!isStatic && this.isMap) {
                    ((Map) object).put(name, newValue);
                    return;
                }
                throw new ReadOnlyPropertyException(name, theClass);
            }
.....
.....

[paulk-asert]

@blackdrag These three asserts fail in 2.4.8
@jwagenleitner That does the trick - I modified the PR

[jwagenleitner]

+1

[blackdrag]

@paulk-asert I might have used a pre 2.4.8 program to test my two scripts. But I can see, that your new version makes both pass again. So +1