Fix the one spot where we were accidentally relying on a cached record,
via an update event, and ensure that can't happen again.
Review at http://gwt-code-reviews.appspot.com/751802
Review by: amitmanjhi@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8569 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java b/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
index 99ebb72..d13b57f 100644
--- a/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
+++ b/user/src/com/google/gwt/app/place/AbstractRecordListActivity.java
@@ -163,8 +163,7 @@
}
};
- createRangeRequest(range).forProperties(getView().getProperties()).fire(
- callback);
+ fireRangeRequest(range, callback);
}
public void onStop() {
@@ -233,6 +232,12 @@
placeController.goTo(new ProxyPlace(record, Operation.DETAILS));
}
+ private void fireRangeRequest(final Range range,
+ final Receiver<List<R>> callback) {
+ createRangeRequest(range).forProperties(getView().getProperties()).fire(
+ callback);
+ }
+
private void getLastPage() {
fireCountRequest(new Receiver<Long>() {
public void onSuccess(Long response, Set<SyncResult> syncResults) {
@@ -266,8 +271,16 @@
}
private void update(R record) {
- Integer row = recordToRow.get(record.getId());
- getView().asHasData().setRowData(row, Collections.singletonList(record));
+ final Integer row = recordToRow.get(record.getId());
+ if (row == null) {
+ return;
+ }
+ fireRangeRequest(new Range(row, 1), new Receiver<List<R>>() {
+ public void onSuccess(List<R> response, Set<SyncResult> syncResults) {
+ getView().asHasData().setRowData(row,
+ Collections.singletonList(response.get(0)));
+ }
+ });
}
private void updateSelection(Place newPlace) {
diff --git a/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java b/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
index 150aca5..f7cc91c 100644
--- a/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
+++ b/user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java
@@ -184,8 +184,7 @@
master.records.put(key, value);
masterRecord = value;
toRemove.add(new RecordKey(datastoreId, futureKey.schema, RequestFactoryJsonImpl.IS_FUTURE));
- master.eventBus.fireEvent(masterRecord.getSchema().createChangeEvent(
- masterRecord, WriteOperation.CREATE));
+ requestFactory.postChangeEvent(masterRecord, WriteOperation.CREATE);
syncResults.add(makeSyncResult(masterRecord, null, futureKey.id));
} else {
// do not change the masterRecord or fire event
@@ -213,8 +212,7 @@
if (masterRecord != null) {
if (violations == NULL_VIOLATIONS) {
master.records.remove(key);
- master.eventBus.fireEvent(masterRecord.getSchema().createChangeEvent(
- masterRecord, WriteOperation.DELETE));
+ requestFactory.postChangeEvent(masterRecord, WriteOperation.DELETE);
syncResults.add(makeSyncResult(masterRecord, null, null));
} else {
// do not change the masterRecord or fire event
@@ -237,8 +235,7 @@
assert masterRecord != null;
masterRecord.merge(entry.getValue());
toRemove.add(key);
- master.eventBus.fireEvent(masterRecord.getSchema().createChangeEvent(
- masterRecord, WriteOperation.UPDATE));
+ requestFactory.postChangeEvent(masterRecord, WriteOperation.UPDATE);
syncResults.add(makeSyncResult(masterRecord, null, null));
} else {
// do not change the masterRecord or fire event
@@ -466,7 +463,7 @@
}
private RecordJsoImpl newChangeRecord(RecordImpl fromRecord) {
- return RecordJsoImpl.emptyCopy(fromRecord);
+ return RecordJsoImpl.emptyCopy(fromRecord.asJso());
}
private void processToRemove(Set<RecordKey> toRemove,
diff --git a/user/src/com/google/gwt/requestfactory/client/impl/RecordJsoImpl.java b/user/src/com/google/gwt/requestfactory/client/impl/RecordJsoImpl.java
index de31d43..4cf57e4 100644
--- a/user/src/com/google/gwt/requestfactory/client/impl/RecordJsoImpl.java
+++ b/user/src/com/google/gwt/requestfactory/client/impl/RecordJsoImpl.java
@@ -73,10 +73,10 @@
return copy;
}
- public static RecordJsoImpl emptyCopy(RecordImpl from) {
- Long id = from.get(Record.id);
- Integer version = from.get(Record.version);
- final RecordSchema<?> schema = from.getSchema();
+ public static RecordJsoImpl emptyCopy(RecordJsoImpl jso) {
+ Long id = jso.get(Record.id);
+ Integer version = jso.get(Record.version);
+ final RecordSchema<?> schema = jso.getSchema();
return create(id, version, schema);
}
diff --git a/user/src/com/google/gwt/requestfactory/client/impl/RecordSchema.java b/user/src/com/google/gwt/requestfactory/client/impl/RecordSchema.java
index 1991941..fd2b05e 100644
--- a/user/src/com/google/gwt/requestfactory/client/impl/RecordSchema.java
+++ b/user/src/com/google/gwt/requestfactory/client/impl/RecordSchema.java
@@ -60,12 +60,6 @@
public abstract RecordChangedEvent<?, ?> createChangeEvent(Record record,
WriteOperation writeOperation);
- public RecordChangedEvent<?, ?> createChangeEvent(RecordJsoImpl jsoRecord,
- WriteOperation writeOperation) {
- R record = create(jsoRecord);
- return createChangeEvent(record, writeOperation);
- }
-
// TODO(rjrjr) rename getProxyClass
public abstract Class<? extends Record> getToken();
}
diff --git a/user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java b/user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java
index 65ba21f..2a4b169 100644
--- a/user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java
+++ b/user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java
@@ -29,6 +29,7 @@
import com.google.gwt.requestfactory.shared.RequestEvent.State;
import com.google.gwt.user.client.Window.Location;
import com.google.gwt.valuestore.shared.Record;
+import com.google.gwt.valuestore.shared.WriteOperation;
import java.util.HashMap;
import java.util.Map;
@@ -65,7 +66,7 @@
private ValueStoreJsonImpl valueStore;
- private EventBus handlerManager;
+ private EventBus eventBus;
public com.google.gwt.valuestore.shared.Record create(
Class<? extends Record> token, RecordToTypeMap recordToTypeMap) {
@@ -84,7 +85,7 @@
builder.setHeader("Content-Type", RequestFactory.JSON_CONTENT_TYPE_UTF8);
builder.setHeader("pageurl", Location.getHref());
builder.setRequestData(ClientRequestHelper.getRequestString(requestObject.getRequestData().getRequestMap(
- ((AbstractRequest<?,?>) requestObject).deltaValueStore.toJson())));
+ ((AbstractRequest<?, ?>) requestObject).deltaValueStore.toJson())));
builder.setCallback(new RequestCallback() {
public void onError(Request request, Throwable exception) {
@@ -120,7 +121,7 @@
e);
}
}
-
+
public Class<? extends Record> getClass(Record proxy) {
return ((RecordImpl) proxy).getSchema().getToken();
}
@@ -138,11 +139,11 @@
}
/**
- * @param handlerManager
+ * @param eventBus
*/
- public void init(EventBus handlerManager) {
- this.valueStore = new ValueStoreJsonImpl(handlerManager);
- this.handlerManager = handlerManager;
+ public void init(EventBus eventBus) {
+ this.valueStore = new ValueStoreJsonImpl();
+ this.eventBus = eventBus;
Logger.getLogger("").addHandler(
new RequestFactoryLogHandler(this, Level.WARNING, wireLogger.getName()));
logger.fine("Successfully initialized RequestFactory");
@@ -192,8 +193,18 @@
return valueStore;
}
- private Record createFuture(
- RecordSchema<? extends Record> schema) {
+ void postChangeEvent(RecordJsoImpl newJsoRecord, WriteOperation op) {
+ /*
+ * Ensure event receivers aren't accidentally using cached info by making an
+ * unpopulated copy of the record.
+ */
+ newJsoRecord = RecordJsoImpl.emptyCopy(newJsoRecord);
+ Record javaRecord = newJsoRecord.getSchema().create(newJsoRecord);
+ eventBus.fireEvent(newJsoRecord.getSchema().createChangeEvent(javaRecord,
+ op));
+ }
+
+ private Record createFuture(RecordSchema<? extends Record> schema) {
Long futureId = ++currentFutureId;
RecordJsoImpl newRecord = RecordJsoImpl.create(futureId, INITIAL_VERSION,
schema);
@@ -203,6 +214,6 @@
}
private void postRequestEvent(State received, Response response) {
- handlerManager.fireEvent(new RequestEvent(received, response));
+ eventBus.fireEvent(new RequestEvent(received, response));
}
}
diff --git a/user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java b/user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java
index 765f296..4555970 100644
--- a/user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java
+++ b/user/src/com/google/gwt/requestfactory/client/impl/ValueStoreJsonImpl.java
@@ -16,7 +16,6 @@
package com.google.gwt.requestfactory.client.impl;
import com.google.gwt.core.client.JsArray;
-import com.google.gwt.event.shared.EventBus;
import com.google.gwt.valuestore.shared.Record;
import com.google.gwt.valuestore.shared.WriteOperation;
@@ -33,14 +32,8 @@
public class ValueStoreJsonImpl {
// package protected fields for use by DeltaValueStoreJsonImpl
- final EventBus eventBus;
-
final Map<RecordKey, RecordJsoImpl> records = new HashMap<RecordKey, RecordJsoImpl>();
- ValueStoreJsonImpl(EventBus eventBus) {
- this.eventBus = eventBus;
- }
-
public Record getRecordBySchemaAndId(RecordSchema<?> schema,
Long id) {
if (id == null) {
@@ -64,34 +57,27 @@
}
}
- /**
- * @param newRecord
- * @param i
- * @param array
- * @param requestFactory
- */
- private void setRecordInList(RecordJsoImpl newRecord, int i,
+ private void setRecordInList(RecordJsoImpl newJsoRecord, int i,
JsArray<RecordJsoImpl> array, RequestFactoryJsonImpl requestFactory) {
- RecordKey recordKey = new RecordKey(newRecord, RequestFactoryJsonImpl.NOT_FUTURE);
- newRecord.setValueStore(this);
- newRecord.setRequestFactory(requestFactory);
+ RecordKey recordKey = new RecordKey(newJsoRecord, RequestFactoryJsonImpl.NOT_FUTURE);
+ newJsoRecord.setValueStore(this);
+ newJsoRecord.setRequestFactory(requestFactory);
RecordJsoImpl oldRecord = records.get(recordKey);
if (oldRecord == null) {
- records.put(recordKey, newRecord);
+ records.put(recordKey, newJsoRecord);
// TODO: need to fire a create event.
} else {
// TODO: Merging is not the correct thing to do but it works as long as we
// don't have filtering by properties. Need to revisit this once response
// only has a subset of all properties.
- boolean changed = oldRecord.merge(newRecord);
- newRecord = oldRecord.cast();
+ boolean changed = oldRecord.merge(newJsoRecord);
+ newJsoRecord = oldRecord.cast();
if (array != null) {
- array.set(i, newRecord);
+ array.set(i, newJsoRecord);
}
if (changed) {
- eventBus.fireEvent(newRecord.getSchema().createChangeEvent(newRecord,
- WriteOperation.UPDATE));
+ requestFactory.postChangeEvent(newJsoRecord, WriteOperation.UPDATE);
}
}
}
diff --git a/user/src/com/google/gwt/valuestore/shared/RecordChangedEvent.java b/user/src/com/google/gwt/valuestore/shared/RecordChangedEvent.java
index a71f1ef..7f58120 100644
--- a/user/src/com/google/gwt/valuestore/shared/RecordChangedEvent.java
+++ b/user/src/com/google/gwt/valuestore/shared/RecordChangedEvent.java
@@ -25,6 +25,13 @@
* </span>
* </p>
* Abstract base class for an event announcing changes to a {@link Record}.
+ * <p>
+ * Note that this event includes an unpopulated copy of the changed record
+ * — all properties are undefined except it's id. That is, the event
+ * includes only enough information for receivers to issue requests to get
+ * themselves fresh copies of the record.
+ * <p>
+ * TODO: rather than an empty record, consider using a string token
*
* @param <R> the type of the record
* @param <H> the type of event handler
@@ -41,6 +48,10 @@
this.writeOperation = writeOperation;
}
+ /**
+ * @return an unpopulated copy of the changed record — all properties
+ * are undefined except it's id
+ */
public R getRecord() {
return record;
}
diff --git a/user/test/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImplTest.java b/user/test/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImplTest.java
index 287b474..7753b41 100644
--- a/user/test/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImplTest.java
+++ b/user/test/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImplTest.java
@@ -72,7 +72,7 @@
@Override
public void gwtSetUp() {
- valueStore = new ValueStoreJsonImpl(null);
+ valueStore = new ValueStoreJsonImpl();
requestFactory = new RequestFactoryJsonImpl() {
public Record create(Class token) {
diff --git a/user/test/com/google/gwt/requestfactory/client/impl/RecordJsoImplTest.java b/user/test/com/google/gwt/requestfactory/client/impl/RecordJsoImplTest.java
index 413899c..6b5f637 100644
--- a/user/test/com/google/gwt/requestfactory/client/impl/RecordJsoImplTest.java
+++ b/user/test/com/google/gwt/requestfactory/client/impl/RecordJsoImplTest.java
@@ -40,8 +40,7 @@
}
public void testEmptyCopy() {
- RecordJsoImpl emptyCopy = RecordJsoImpl.emptyCopy(new RecordImpl(
- getPopulatedJso(), false));
+ RecordJsoImpl emptyCopy = RecordJsoImpl.emptyCopy(getPopulatedJso());
testMinimalJso(emptyCopy, SCHEMA_PRESENT);
}
