Reviewers: rdayal,
Description:
DefaultProxyStore violated ProxyStore#nextId contract.
The javadoc for nextId states that the return values must be unique, but
DefaultProxyStore returned the same value twice in a row unless an
object is added to the store. This situation happens when serializing a
list of ValueProxy objects (containing at least two distinct objects),
which unfortunately wasn't tested.
The sequence is now monotonically increasing. It's initial value is the
size of the internal map: in case where the initial map is only made of
ValueProxies, they would all have synthetic IDs starting at 0 up to the
size of the map minus one, so the size of the map can safely be used as
the next unique ID (this only works because no object can ever be
removed from the store).
Fixes issue 6961.
Please review this at http://gwt-code-reviews.appspot.com/1622803/
Affected files:
M
user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
M
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
Index:
user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
diff --git
a/user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
b/user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
index
10a588ae228942b5fad30e3ccf5a015ae1105e0f..f816d2fdd69e31a88d94c37b5e123b3e8ef2c8e3
100644
---
a/user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
+++
b/user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java
@@ -36,6 +36,7 @@ public class DefaultProxyStore implements ProxyStore {
private static final String EXPECTED_VERSION = "211";
private final AutoBean<OperationMessage> messageBean;
private final Map<String, Splittable> map;
+ private int nextId;
/**
* Construct an empty DefaultProxyStore.
@@ -66,6 +67,8 @@ public class DefaultProxyStore implements ProxyStore {
+ message.getVersion());
}
map = message.getPropertyMap();
+ // Note: this only works because the store is append-only!
+ nextId = map.size();
}
/**
@@ -80,7 +83,7 @@ public class DefaultProxyStore implements ProxyStore {
}
public int nextId() {
- return map.size();
+ return nextId++;
}
public void put(String key, Splittable value) {
Index:
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
diff --git
a/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
index
01da1c3d1b560b1d43dfd12c378a2b2f0e6d099e..4f71e3b859976507208da58336717bcacebd2930
100644
---
a/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
+++
b/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
@@ -2600,14 +2600,18 @@ public class RequestFactoryTest extends
RequestFactoryTestBase {
SimpleFooRequest req = simpleFooRequest();
// Create
- final SimpleValueProxy created =
req.create(SimpleValueProxy.class);
- created.setNumber(42);
- created.setString("Hello world!");
- created.setSimpleFoo(response);
+ final SimpleValueProxy created1 =
req.create(SimpleValueProxy.class);
+ created1.setNumber(42);
+ created1.setString("Able");
+ created1.setSimpleFoo(response);
+ final SimpleValueProxy created2 =
req.create(SimpleValueProxy.class);
+ created2.setNumber(43);
+ created2.setString("Baker");
+ created2.setSimpleFoo(response);
// Set
response = req.edit(response);
- response.setSimpleValues(Arrays.asList(created));
+ response.setSimpleValues(Arrays.asList(created1, created2));
// Retrieve
req.persistAndReturnSelf().using(response).with("simpleValues").to(
@@ -2615,12 +2619,19 @@ public class RequestFactoryTest extends
RequestFactoryTestBase {
@Override
public void onSuccess(SimpleFooProxy response) {
response = checkSerialization(response);
- SimpleValueProxy value = response.getSimpleValues().get(0);
- assertEquals(42, value.getNumber());
+ assertEquals(2, response.getSimpleValues().size());
+ SimpleValueProxy value1 =
response.getSimpleValues().get(0);
+ assertEquals(42, value1.getNumber());
+ assertEquals("Able", value1.getString());
+ assertSame(response, value1.getSimpleFoo());
+ SimpleValueProxy value2 =
response.getSimpleValues().get(1);
+ assertEquals(43, value2.getNumber());
+ assertEquals("Baker", value2.getString());
+ assertSame(response, value2.getSimpleFoo());
try {
// Require owning object to be editable
- response.getSimpleValues().get(0).setNumber(43);
+ response.getSimpleValues().get(0).setNumber(44);
fail("Should have thrown exception");
} catch (IllegalStateException expected) {
}
@@ -2628,13 +2639,13 @@ public class RequestFactoryTest extends
RequestFactoryTestBase {
// Update
SimpleFooRequest req = simpleFooRequest();
response = req.edit(response);
- response.getSimpleValues().get(0).setNumber(43);
+ response.getSimpleValues().get(0).setNumber(44);
req.persistAndReturnSelf().using(response).with("simpleValues").to(
new Receiver<SimpleFooProxy>() {
@Override
public void onSuccess(SimpleFooProxy response) {
response = checkSerialization(response);
- assertEquals(43,
response.getSimpleValues().get(0).getNumber());
+ assertEquals(44,
response.getSimpleValues().get(0).getNumber());
finishTestAndReset();
}
}).fire();
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
