Revision: 8710
Author: b...@google.com
Date: Thu Sep  2 16:22:41 2010
Log: 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

http://code.google.com/p/google-web-toolkit/source/detail?r=8710

Modified:
/trunk/user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java /trunk/user/src/com/google/gwt/editor/rebind/AbstractEditorDriverGenerator.java
 /trunk/user/src/com/google/gwt/editor/rebind/model/EditorModel.java
 /trunk/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java
 /trunk/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java

=======================================
--- /trunk/user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java Thu Sep 2 10:32:07 2010 +++ /trunk/user/src/com/google/gwt/editor/client/impl/AbstractEditorDelegate.java Thu Sep 2 16:22:41 2010
@@ -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;
     }
@@ -67,6 +75,8 @@
     setObject(ensureMutable(getObject()));
     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) {
=======================================
--- /trunk/user/src/com/google/gwt/editor/rebind/AbstractEditorDriverGenerator.java Thu Sep 2 10:32:07 2010 +++ /trunk/user/src/com/google/gwt/editor/rebind/AbstractEditorDriverGenerator.java Thu Sep 2 16:22:41 2010
@@ -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(),
=======================================
--- /trunk/user/src/com/google/gwt/editor/rebind/model/EditorModel.java Thu Sep 2 10:32:07 2010 +++ /trunk/user/src/com/google/gwt/editor/rebind/model/EditorModel.java Thu Sep 2 16:22:41 2010
@@ -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&lt;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()]);
   }
=======================================
--- /trunk/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java Thu Sep 2 10:32:07 2010 +++ /trunk/user/test/com/google/gwt/editor/client/SimpleBeanEditorTest.java Thu Sep 2 16:22:41 2010
@@ -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;
@@ -63,13 +48,32 @@
       this.street = street;
     }
   }
-
-  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;
@@ -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";
@@ -138,6 +232,36 @@
     assertEquals("12345", person.address.street);
     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);
@@ -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);
+  }
+}
=======================================
--- /trunk/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java Thu Sep 2 10:32:07 2010 +++ /trunk/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java Thu Sep 2 16:22:41 2010
@@ -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);

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to