Clarify EditorDriver behavior with combinations of Editor mix-in interfaces through brute-force testing. Patch by: bobv Review by: rjrjr Review at http://gwt-code-reviews.appspot.com/840801 git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8710 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java b/user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java index 17f98ed..b6704b7 100644 --- a/user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java +++ b/user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java
@@ -17,6 +17,7 @@ import com.google.gwt.editor.client.Editor; import com.google.gwt.editor.client.EditorDelegate; +import com.google.gwt.editor.client.LeafValueEditor; import com.google.gwt.editor.client.ValueAwareEditor; import com.google.gwt.event.shared.EventBus; import com.google.gwt.event.shared.HandlerRegistration; @@ -43,7 +44,7 @@ } protected EventBus eventBus; - + protected LeafValueEditor<T> leafValueEditor; /** * This field avoids needing to repeatedly cast {@link #editor}. */ @@ -60,6 +61,13 @@ if (valueAwareEditor != null) { valueAwareEditor.flush(); } + + // See comment in initialize about LeafValueEditors + if (leafValueEditor != null) { + setObject(leafValueEditor.getValue()); + return; + } + if (getObject() == null) { return; } @@ -68,6 +76,8 @@ flushValues(); } + public abstract T getObject(); + public String getPath() { return path; } @@ -86,17 +96,33 @@ protected abstract void flushValues(); - protected abstract T getObject(); - protected void initialize(EventBus eventBus, String pathSoFar, T object, E editor) { this.eventBus = eventBus; this.path = pathSoFar; setEditor(editor); setObject(object); + + // Set up pre-casted fields to access the editor + if (editor instanceof LeafValueEditor<?>) { + leafValueEditor = (LeafValueEditor<T>) editor; + } if (editor instanceof ValueAwareEditor<?>) { valueAwareEditor = (ValueAwareEditor<T>) editor; valueAwareEditor.setDelegate(this); + } + + /* + * Unusual case: The user may have installed an editor subtype that adds the + * LeafValueEditor interface into a plain Editor field. If this has + * happened, only set the value and don't descend into any sub-Editors. + */ + if (leafValueEditor != null) { + leafValueEditor.setValue(object); + return; + } + + if (valueAwareEditor != null) { valueAwareEditor.setValue(object); } if (object != null) {
diff --git a/user/src/com/google/gwt/editor/rebind/AbstractEditorDriverGenerator.java b/user/src/com/google/gwt/editor/rebind/AbstractEditorDriverGenerator.java index 8d34376..615ea6e 100644 --- a/user/src/com/google/gwt/editor/rebind/AbstractEditorDriverGenerator.java +++ b/user/src/com/google/gwt/editor/rebind/AbstractEditorDriverGenerator.java
@@ -108,7 +108,7 @@ sw.println("protected void setEditor(%s editor) {this.editor=editor;}", editor.getQualifiedSourceName()); sw.println("private %s object;", proxy.getQualifiedSourceName()); - sw.println("protected %s getObject() {return object;}", + sw.println("public %s getObject() {return object;}", proxy.getQualifiedSourceName()); sw.println("protected void setObject(%s object) {this.object=object;}", proxy.getQualifiedSourceName()); @@ -127,7 +127,8 @@ sw.println("protected void attachSubEditors() {"); sw.indent(); for (EditorData d : data) { - if (d.isBeanEditor()) { + if (d.isBeanEditor() && !d.isLeafValueEditor() + || d.isValueAwareEditor()) { String subDelegateType = getEditorDelegate(d.getEditedType(), d.getEditorType()); sw.println("if (editor.%s != null) {", d.getSimpleExpression()); @@ -147,8 +148,15 @@ sw.indent(); for (EditorData d : data) { if (d.isBeanEditor()) { - sw.println("if (%1$sDelegate != null) %1$sDelegate.flush();", + sw.println("if (%1$sDelegate != null) { %1$sDelegate.flush();", d.getPropertyName()); + if (d.getSetterName() != null) { + String mutableObjectExpression = mutableObjectExpression(String.format( + "getObject()%s", d.getBeanOwnerExpression())); + sw.println("%s.%s(%sDelegate.getObject());", + mutableObjectExpression, d.getSetterName(), d.getPropertyName()); + } + sw.println("}"); } } sw.outdent(); @@ -158,7 +166,8 @@ sw.println("protected void flushValues() {"); sw.indent(); for (EditorData d : data) { - if (d.isLeafValueEditor() && d.getSetterName() != null) { + if ((d.isLeafValueEditor() && !d.isValueAwareEditor()) + && d.getSetterName() != null) { String mutableObjectExpression = mutableObjectExpression(String.format( "getObject()%s", d.getBeanOwnerExpression())); sw.println("if (editor.%1$s != null)" @@ -196,7 +205,7 @@ sw.println("protected void pushValues() {"); sw.indent(); for (EditorData d : data) { - if (d.isLeafValueEditor()) { + if (d.isLeafValueEditor() && !d.isValueAwareEditor()) { sw.println("if (editor.%1$s != null)" + " editor.%1$s.setValue(getObject()%2$s.%3$s());", d.getSimpleExpression(), d.getBeanOwnerExpression(),
diff --git a/user/src/com/google/gwt/editor/rebind/model/EditorModel.java b/user/src/com/google/gwt/editor/rebind/model/EditorModel.java index f5243da..c23a0cb 100644 --- a/user/src/com/google/gwt/editor/rebind/model/EditorModel.java +++ b/user/src/com/google/gwt/editor/rebind/model/EditorModel.java
@@ -38,6 +38,8 @@ * Analyzes an Editor driver declaration. */ public class EditorModel { + private static final EditorData[] EMPTY_EDITOR_DATA = new EditorData[0]; + /** * Given type assignable to <code>Editor<Foo></code>, return * <code>Foo</code>. It is an error to call this method with a type not @@ -193,10 +195,13 @@ return editorData; } + /** + * Guaranteed to never return null. + */ public EditorData[] getEditorData(JClassType editor) { List<EditorData> toReturn = typeData.get(editor); if (toReturn == null) { - return null; + return EMPTY_EDITOR_DATA; } return toReturn.toArray(new EditorData[toReturn.size()]); }
diff --git a/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java b/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java index 0ab4ca8..9e76465 100644 --- a/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java +++ b/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java
@@ -19,30 +19,15 @@ import com.google.gwt.editor.client.adapters.StringEditor; import com.google.gwt.junit.client.GWTTestCase; +import java.util.Set; + +import javax.validation.ConstraintViolation; + /** * Uses the SimpleBeanEditorTest to test core Editor behaviors as generated by * AbstractEditorDriverGenerator. */ public class SimpleBeanEditorTest extends GWTTestCase { - @Override - protected void gwtSetUp() throws Exception { - personAddress = new Address(); - personAddress.city = "City"; - personAddress.street = "Street"; - - manager = new Person(); - manager.name = "Bill"; - - person = new Person(); - person.address = personAddress; - person.name = "Alice"; - person.manager = manager; - } - - Person person; - Address personAddress; - Person manager; - class Address { String city; String street; @@ -64,13 +49,32 @@ } } - static final String UNINITIALIZED = "uninitialized"; - class AddressEditor implements Editor<Address> { StringEditor city = StringEditor.of(UNINITIALIZED); StringEditor street = StringEditor.of(UNINITIALIZED); } + class LeafAddressEditor extends AddressEditor implements + LeafValueEditor<Address> { + /* + * These two fields are used to ensure that getValue() and setValue() aren't + * called excessively. + */ + int getValueCalled; + int setValueCalled; + Address value; + + public Address getValue() { + getValueCalled++; + return value; + } + + public void setValue(Address value) { + setValueCalled++; + value = new Address(); + } + } + class Person { String name; Address address; @@ -112,6 +116,96 @@ SimpleBeanEditorDriver<Person, PersonEditor> { } + class PersonEditorWithLeafAddressEditor implements Editor<Person> { + LeafAddressEditor addressEditor = new LeafAddressEditor(); + StringEditor name = StringEditor.of(UNINITIALIZED); + @Path("manager.name") + StringEditor managerName = StringEditor.of(UNINITIALIZED); + } + + interface PersonEditorWithLeafAddressEditorDriver extends + SimpleBeanEditorDriver<Person, PersonEditorWithLeafAddressEditor> { + } + + class PersonEditorWithValueAwareAddressEditor implements Editor<Person> { + ValueAwareAddressEditor addressEditor = new ValueAwareAddressEditor(); + StringEditor name = StringEditor.of(UNINITIALIZED); + @Path("manager.name") + StringEditor managerName = StringEditor.of(UNINITIALIZED); + } + + interface PersonEditorWithValueAwareAddressEditorDriver extends + SimpleBeanEditorDriver<Person, PersonEditorWithValueAwareAddressEditor> { + } + + class PersonEditorWithValueAwareLeafAddressEditor implements Editor<Person> { + ValueAwareLeafAddressEditor addressEditor = new ValueAwareLeafAddressEditor(); + StringEditor name = StringEditor.of(UNINITIALIZED); + @Path("manager.name") + StringEditor managerName = StringEditor.of(UNINITIALIZED); + } + + interface PersonEditorWithValueAwareLeafAddressEditorDriver + extends + SimpleBeanEditorDriver<Person, PersonEditorWithValueAwareLeafAddressEditor> { + } + + class ValueAwareAddressEditor extends AddressEditor implements + ValueAwareEditor<Address> { + int flushCalled; + int setDelegateCalled; + int setValueCalled; + Address value; + + public void flush() { + flushCalled++; + } + + public void onPropertyChange(String... paths) { + } + + public void setDelegate(EditorDelegate<Address> delegate) { + setDelegateCalled++; + } + + public void setValue(Address value) { + setValueCalled++; + this.value = value; + } + + public void showErrors(Set<ConstraintViolation<Address>> violations) { + } + } + + /** + * All the mix-ins. Not that this seems like a good idea... + */ + class ValueAwareLeafAddressEditor extends LeafAddressEditor implements + ValueAwareEditor<Address> { + int flushCalled; + int setDelegateCalled; + + public void flush() { + flushCalled++; + } + + public void onPropertyChange(String... paths) { + } + + public void setDelegate(EditorDelegate<Address> delegate) { + setDelegateCalled++; + } + + public void showErrors(Set<ConstraintViolation<Address>> violations) { + } + } + + Person person; + Address personAddress; + Person manager; + + static final String UNINITIALIZED = "uninitialized"; + @Override public String getModuleName() { return "com.google.gwt.editor.Editor"; @@ -139,6 +233,36 @@ assertEquals("David", person.manager.name); } + /** + * We want to verify that the sub-editors of a LeafValueEditor are not + * initialized. Additonally, we want to ensure that the instance returned from + * the LVE is assigned into the owner type. + */ + public void testLeafValueEditorDeclaredInSlot() { + PersonEditorWithLeafAddressEditor personEditor = new PersonEditorWithLeafAddressEditor(); + PersonEditorWithLeafAddressEditorDriver driver = GWT.create(PersonEditorWithLeafAddressEditorDriver.class); + LeafAddressEditor addressEditor = personEditor.addressEditor; + + testLeafAddressEditor(driver, personEditor, addressEditor); + } + + /** + * We want to verify that the sub-editors of a LeafValueEditor are not + * initialized. Additionally, we want to ensure that the instance returned + * from the LVE is assigned into the owner type. + */ + public void testLeafValueEditorInPlainEditorSlot() { + PersonEditorDriver driver = GWT.create(PersonEditorDriver.class); + + PersonEditor personEditor = new PersonEditor(); + LeafAddressEditor addressEditor = new LeafAddressEditor(); + + // Runtime assignment of unexpected LeafValueEditor + personEditor.addressEditor = addressEditor; + + testLeafAddressEditor(driver, personEditor, addressEditor); + } + public void testLifecycle() { PersonEditorDriver driver = GWT.create(PersonEditorDriver.class); try { @@ -155,4 +279,112 @@ driver.edit(person); driver.flush(); } + + public void testValueAwareEditorInPlainSlot() { + PersonEditorDriver driver = GWT.create(PersonEditorDriver.class); + + PersonEditor personEditor = new PersonEditor(); + ValueAwareAddressEditor addressEditor = new ValueAwareAddressEditor(); + + // Runtime assignment of unexpected LeafValueEditor + personEditor.addressEditor = addressEditor; + + testValueAwareAddressEditor(driver, personEditor, addressEditor); + } + + public void testValueAwareEditorInDeclaredSlot() { + PersonEditorWithValueAwareAddressEditorDriver driver = GWT.create(PersonEditorWithValueAwareAddressEditorDriver.class); + PersonEditorWithValueAwareAddressEditor personEditor = new PersonEditorWithValueAwareAddressEditor(); + ValueAwareAddressEditor addressEditor = personEditor.addressEditor; + + testValueAwareAddressEditor(driver, personEditor, addressEditor); + } + + public void testValueAwareLeafValueEditorInDeclaredSlot() { + PersonEditorWithValueAwareLeafAddressEditor personEditor = new PersonEditorWithValueAwareLeafAddressEditor(); + PersonEditorWithValueAwareLeafAddressEditorDriver driver = GWT.create(PersonEditorWithValueAwareLeafAddressEditorDriver.class); + ValueAwareLeafAddressEditor addressEditor = personEditor.addressEditor; + + testLeafAddressEditor(driver, personEditor, addressEditor); + assertEquals(1, addressEditor.flushCalled); + assertEquals(1, addressEditor.setDelegateCalled); + } + + public void testValueAwareLeafValueEditorInPlainEditorSlot() { + PersonEditorDriver driver = GWT.create(PersonEditorDriver.class); + + PersonEditor personEditor = new PersonEditor(); + ValueAwareLeafAddressEditor addressEditor = new ValueAwareLeafAddressEditor(); + + // Runtime assignment of unexpected LeafValueEditor + personEditor.addressEditor = addressEditor; + + testLeafAddressEditor(driver, personEditor, addressEditor); + assertEquals(1, addressEditor.flushCalled); + assertEquals(1, addressEditor.setDelegateCalled); + } + + @Override + protected void gwtSetUp() throws Exception { + personAddress = new Address(); + personAddress.city = "City"; + personAddress.street = "Street"; + + manager = new Person(); + manager.name = "Bill"; + + person = new Person(); + person.address = personAddress; + person.name = "Alice"; + person.manager = manager; + } + + private <T extends Editor<Person>> void testValueAwareAddressEditor( + SimpleBeanEditorDriver<Person, T> driver, T personEditor, + ValueAwareAddressEditor addressEditor) { + Address oldAddress = person.address; + // Initialize + driver.initialize(personEditor); + assertEquals(0, addressEditor.setValueCalled); + assertEquals(0, addressEditor.setDelegateCalled); + assertEquals(0, addressEditor.flushCalled); + + // Edit + driver.edit(person); + assertEquals(1, addressEditor.setValueCalled); + assertEquals(1, addressEditor.setDelegateCalled); + assertEquals(0, addressEditor.flushCalled); + assertEquals("City", addressEditor.city.getValue()); + + // Flush + driver.flush(); + assertEquals(1, addressEditor.setValueCalled); + assertEquals(1, addressEditor.setDelegateCalled); + assertEquals(1, addressEditor.flushCalled); + assertSame(oldAddress, person.address); + assertSame(person.address, addressEditor.value); + } + + private <T extends Editor<Person>> void testLeafAddressEditor( + SimpleBeanEditorDriver<Person, T> driver, T personEditor, + LeafAddressEditor addressEditor) { + Address oldAddress = person.address; + // Initialize + driver.initialize(personEditor); + assertEquals(0, addressEditor.setValueCalled); + assertEquals(0, addressEditor.getValueCalled); + + // Edit + driver.edit(person); + assertEquals(1, addressEditor.setValueCalled); + assertEquals(0, addressEditor.getValueCalled); + assertEquals(UNINITIALIZED, addressEditor.city.getValue()); + + // Flush + driver.flush(); + assertEquals(1, addressEditor.setValueCalled); + assertEquals(1, addressEditor.getValueCalled); + assertNotSame(oldAddress, person.address); + assertSame(person.address, addressEditor.value); + } }
diff --git a/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java b/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java index 39780f4..3156b1a 100644 --- a/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java +++ b/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java
@@ -218,7 +218,8 @@ EditorModel m = new EditorModel(logger, types.findType("t.CompositeEditorDriver"), rfedType); - assertNull(m.getEditorData(types.getJavaLangObject())); + assertNotNull(m.getEditorData(types.getJavaLangObject())); + assertEquals(0, m.getEditorData(types.getJavaLangObject()).length); EditorData[] composite = m.getEditorData(types.findType("t.CompositeEditor")); assertEquals(2, composite.length);