LGTM As we discussed, putting the new JSO in the master value store feels skeevey but should do no harm for this patch. Landing this lets us proceed.
We have uncovered a pretty serious problem with the api, the fact that requests are not responsible for their own creates. I'll log a follow up issue on that. http://gwt-code-reviews.appspot.com/921801/diff/8001/9002 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/events/EditPersonEvent.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9002#newcode41 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/events/EditPersonEvent.java:41: public EditPersonEvent(PersonProxy person) { A bit squirrely that this event is carrying a proxy rather than a proxyid. Yes, yes, more async code, more server hits. We will get caching sooner or later, and we'd like this app to serve as an example of thorough decoupling. http://gwt-code-reviews.appspot.com/921801/diff/8001/9006 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9006#newcode185 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java:185: PersonProxy person = (PersonProxy) response; @Override, and don't need the cast. Did eclipse not warn? http://gwt-code-reviews.appspot.com/921801/diff/8001/9006#newcode216 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java:216: // XXX add back filter NC-17 http://gwt-code-reviews.appspot.com/921801/diff/8001/9007 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.ui.xml (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9007#newcode6 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.ui.xml:6: display: inline; tab http://gwt-code-reviews.appspot.com/921801/diff/8001/9007#newcode10 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.ui.xml:10: width: 30% tab http://gwt-code-reviews.appspot.com/921801/diff/8001/9009 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9009#newcode71 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java:71: true, true, true, true, true, true, true}; Storing this per user is pretty funny. Not pushing back — it's a demo, and we need to exercise the feature. Merely sharing my amusement. http://gwt-code-reviews.appspot.com/921801/diff/8001/9009#newcode151 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java:151: this.address.copyFrom(address); // TODO: odd to copy an entity this, but we're waiting for value object support. Convert when they're ready http://gwt-code-reviews.appspot.com/921801/diff/8001/9014 File samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/SchoolCalendarService.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9014#newcode50 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/SchoolCalendarService.java:50: * XXX Remove this once primitive lists work. XXX is even worse than TODO? Thought they work now? http://gwt-code-reviews.appspot.com/921801/diff/8001/9014#newcode75 samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/server/SchoolCalendarService.java:75: * XXX private due to inability to add method overloads. Is there a bug on that? I can't see any excuse for it. http://gwt-code-reviews.appspot.com/921801/diff/8001/9018 File user/src/com/google/gwt/editor/client/ValueAwareEditor.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9018#newcode41 user/src/com/google/gwt/editor/client/ValueAwareEditor.java:41: * ValueAwareEditors should preferentially use sub-editors to alter the They don't have a choice any more, due they? http://gwt-code-reviews.appspot.com/921801/diff/8001/9021 File user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9021#newcode202 user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java:202: protected <Q> Q ensureMutable(Q object) { javadoc http://gwt-code-reviews.appspot.com/921801/diff/8001/9026 File user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9026#newcode114 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:114: private final Map<EntityProxyId<?>, WriteOperation> operations = new HashMap<EntityProxyId<?>, WriteOperation>(); Thank you. I couldn't get this to work. http://gwt-code-reviews.appspot.com/921801/diff/8001/9026#newcode218 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:218: ProxyJsoImpl proxy = creates.get(record.stableId()); nit: I've been trying to use the proxyJso name in any var that points to one. Newcomers are very confused by the ProxyImpl-wraps-ProxyJsoImpl thing. http://gwt-code-reviews.appspot.com/921801/diff/8001/9026#newcode237 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:237: ProxyJsoImpl proxy = creates.get(record.stableId()); proxyJso http://gwt-code-reviews.appspot.com/921801/diff/8001/9026#newcode329 user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java:329: if (!proxy.unpersisted()) { I would not be offended if this were reversed and named persisted. http://gwt-code-reviews.appspot.com/921801/diff/8001/9030 File user/src/com/google/gwt/requestfactory/client/impl/ProxyImpl.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9030#newcode76 user/src/com/google/gwt/requestfactory/client/impl/ProxyImpl.java:76: * and they are from the same Request object. Should document this on EntityProxy.java http://gwt-code-reviews.appspot.com/921801/diff/8001/9030#newcode79 user/src/com/google/gwt/requestfactory/client/impl/ProxyImpl.java:79: public boolean equals(Object o) { need null check on o. Does instanceof take care of that? http://gwt-code-reviews.appspot.com/921801/diff/8001/9030#newcode197 user/src/com/google/gwt/requestfactory/client/impl/ProxyImpl.java:197: this.request = request; Why have pointers to both the request and the dvs? They're 1:1, no? http://gwt-code-reviews.appspot.com/921801/diff/8001/9031 File user/src/com/google/gwt/requestfactory/client/impl/ProxyJsoImpl.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9031#newcode251 user/src/com/google/gwt/requestfactory/client/impl/ProxyJsoImpl.java:251: return (V) toReturn; it's a shame you couldn't fit in a few more calls to split http://gwt-code-reviews.appspot.com/921801/diff/8001/9033 File user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java (right): http://gwt-code-reviews.appspot.com/921801/diff/8001/9033#newcode120 user/src/com/google/gwt/requestfactory/client/impl/RequestFactoryJsonImpl.java:120: for (Object proxy : requestData.getProxyParameters()) { Did you delete the dummy object support code in JsonRequestProcessor? If you can't do it now, lmk and I'll file a task. http://gwt-code-reviews.appspot.com/921801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
