Merged 6.8 branch

3 files changed, 146 insertions, 19 deletions

diff --git a/server/src/com/vaadin/data/util/sqlcontainer/ColumnProperty.java b/server/src/com/vaadin/data/util/sqlcontainer/ColumnProperty.java
index 3e55633574..0146c92b5c 100644
--- a/server/src/com/vaadin/data/util/sqlcontainer/ColumnProperty.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/ColumnProperty.java
@@ -20,6 +20,7 @@ import java.sql.Time;
 import java.sql.Timestamp;
 
 import com.vaadin.data.Property;
+import com.vaadin.data.util.converter.Converter.ConversionException;
 
 /**
  * ColumnProperty represents the value of one column in a RowItem. In addition
@@ -47,6 +48,7 @@ final public class ColumnProperty implements Property {
     private boolean modified;
 
     private boolean versionColumn;
+    private boolean primaryKey = false;
 
     /**
      * Prevent instantiation without required parameters.
@@ -55,9 +57,50 @@ final public class ColumnProperty implements Property {
     private ColumnProperty() {
     }
 
+    /**
+     * Deprecated constructor for ColumnProperty. If this is used the primary
+     * keys are not identified correctly in some cases for some databases (i.e.
+     * Oracle). See http://dev.vaadin.com/ticket/9145.
+     * 
+     * @param propertyId
+     * @param readOnly
+     * @param allowReadOnlyChange
+     * @param nullable
+     * @param value
+     * @param type
+     * 
+     * @deprecated
+     */
+    @Deprecated
     public ColumnProperty(String propertyId, boolean readOnly,
             boolean allowReadOnlyChange, boolean nullable, Object value,
             Class<?> type) {
+        this(propertyId, readOnly, allowReadOnlyChange, nullable, false, value,
+                type);
+    }
+
+    /**
+     * Creates a new ColumnProperty instance.
+     * 
+     * @param propertyId
+     *            The ID of this property.
+     * @param readOnly
+     *            Whether this property is read-only.
+     * @param allowReadOnlyChange
+     *            Whether the read-only status of this property can be changed.
+     * @param nullable
+     *            Whether this property accepts null values.
+     * @param primaryKey
+     *            Whether this property corresponds to a database primary key.
+     * @param value
+     *            The value of this property.
+     * @param type
+     *            The type of this property.
+     */
+    public ColumnProperty(String propertyId, boolean readOnly,
+            boolean allowReadOnlyChange, boolean nullable, boolean primaryKey,
+            Object value, Class<?> type) {
+
         if (propertyId == null) {
             throw new IllegalArgumentException("Properties must be named.");
         }
@@ -71,8 +114,15 @@ final public class ColumnProperty implements Property {
         this.allowReadOnlyChange = allowReadOnlyChange;
         this.nullable = nullable;
         this.readOnly = readOnly;
+        this.primaryKey = primaryKey;
     }
 
+    /**
+     * Returns the current value for this property. To get the previous value
+     * (if one exists) for a modified property use {@link #getOldValue()}.
+     * 
+     * @return
+     */
     @Override
     public Object getValue() {
         if (isModified()) {
@@ -81,8 +131,20 @@ final public class ColumnProperty implements Property {
         return value;
     }
 
+    /**
+     * Returns the original non-modified value of this property if it has been
+     * modified.
+     * 
+     * @return The original value if <code>isModified()</code> is true,
+     *         <code>getValue()</code> otherwise.
+     */
+    public Object getOldValue() {
+        return value;
+    }
+
     @Override
-    public void setValue(Object newValue) throws ReadOnlyException {
+    public void setValue(Object newValue) throws ReadOnlyException,
+    ConversionException {
         if (newValue == null && !nullable) {
             throw new NotNullableException(
                     "Null values are not allowed for this property.");
@@ -158,6 +220,17 @@ final public class ColumnProperty implements Property {
         return readOnly;
     }
 
+    /**
+     * Returns whether the read-only status of this property can be changed
+     * using {@link #setReadOnly(boolean)}.
+     * <p>
+     * Used to prevent setting to read/write mode a property that is not allowed
+     * to be written by the underlying database. Also used for values like
+     * VERSION and AUTO_INCREMENT fields that might be set to read-only by the
+     * container but the database still allows writes.
+     * 
+     * @return true if the read-only status can be changed, false otherwise.
+     */
     public boolean isReadOnlyChangeAllowed() {
         return allowReadOnlyChange;
     }
@@ -169,6 +242,10 @@ final public class ColumnProperty implements Property {
         }
     }
 
+    public boolean isPrimaryKey() {
+        return primaryKey;
+    }
+
     public String getPropertyId() {
         return propertyId;
     }
@@ -214,6 +291,32 @@ final public class ColumnProperty implements Property {
     }
 
     /**
+     * Return whether the value of this property should be persisted to the
+     * database.
+     * 
+     * @return true if the value should be written to the database, false
+     *         otherwise.
+     */
+    public boolean isPersistent() {
+        if (isVersionColumn()) {
+            return false;
+        } else if (isReadOnlyChangeAllowed() && !isReadOnly()) {
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    /**
+     * Returns whether or not this property is used as a row identifier.
+     * 
+     * @return true if the property is a row identifier, false otherwise.
+     */
+    public boolean isRowIdentifier() {
+        return isPrimaryKey() || isVersionColumn();
+    }
+
+    /**
      * An exception that signals that a <code>null</code> value was passed to
      * the <code>setValue</code> method, but the value of this property can not
      * be set to <code>null</code>.
diff --git a/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java b/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java
index 78700caee9..f772e2701c 100644
--- a/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/SQLContainer.java
@@ -68,7 +68,9 @@ public class SQLContainer implements Container, Container.Filterable,
     private final List<String> propertyIds = new ArrayList<String>();
     private final Map<String, Class<?>> propertyTypes = new HashMap<String, Class<?>>();
     private final Map<String, Boolean> propertyReadOnly = new HashMap<String, Boolean>();
+    private final Map<String, Boolean> propertyPersistable = new HashMap<String, Boolean>();
     private final Map<String, Boolean> propertyNullable = new HashMap<String, Boolean>();
+    private final Map<String, Boolean> propertyPrimaryKey = new HashMap<String, Boolean>();
 
     /** Filters (WHERE) and sorters (ORDER BY) */
     private final List<Filter> filters = new ArrayList<Filter>();
@@ -150,11 +152,14 @@ public class SQLContainer implements Container, Container.Filterable,
         List<ColumnProperty> itemProperties = new ArrayList<ColumnProperty>();
         for (String propertyId : propertyIds) {
             /* Default settings for new item properties. */
-            itemProperties
-                    .add(new ColumnProperty(propertyId, propertyReadOnly
-                            .get(propertyId),
-                            !propertyReadOnly.get(propertyId), propertyNullable
-                                    .get(propertyId), null, getType(propertyId)));
+            ColumnProperty cp = new ColumnProperty(propertyId,
+                    propertyReadOnly.get(propertyId),
+                    propertyPersistable.get(propertyId),
+                    propertyNullable.get(propertyId),
+                    propertyPrimaryKey.get(propertyId), null,
+                    getType(propertyId));
+
+            itemProperties.add(cp);
         }
         RowItem newRowItem = new RowItem(this, itemId, itemProperties);
 
@@ -546,7 +551,9 @@ public class SQLContainer implements Container, Container.Filterable,
     @Override
     public void removeContainerFilter(Filter filter) {
         filters.remove(filter);
-        refresh();
+        // TODO this cannot be added before ComboBox is fixed
+        // (Select.requestRepaint() must not affect filter string)
+        // refresh();
     }
 
     /**
@@ -1141,14 +1148,22 @@ public class SQLContainer implements Container, Container.Filterable,
                  */
                 boolean readOnly = rsmd.isAutoIncrement(i)
                         || rsmd.isReadOnly(i);
-                if (delegate instanceof TableQuery
-                        && rsmd.getColumnLabel(i).equals(
-                                ((TableQuery) delegate).getVersionColumn())) {
-                    readOnly = true;
+
+                boolean persistable = !rsmd.isReadOnly(i);
+
+                if (delegate instanceof TableQuery) {
+                    if (rsmd.getColumnLabel(i).equals(
+                            ((TableQuery) delegate).getVersionColumn())) {
+                        readOnly = true;
+                    }
                 }
+
                 propertyReadOnly.put(colName, readOnly);
+                propertyPersistable.put(colName, persistable);
                 propertyNullable.put(colName,
                         rsmd.isNullable(i) == ResultSetMetaData.columnNullable);
+                propertyPrimaryKey.put(colName, delegate.getPrimaryKeyColumns()
+                        .contains(rsmd.getColumnLabel(i)));
                 propertyTypes.put(colName, type);
             }
             rs.getStatement().close();
@@ -1248,10 +1263,13 @@ public class SQLContainer implements Container, Container.Filterable,
                          * column.
                          */
                         if (propertiesToAdd.contains(colName)) {
+
                             cp = new ColumnProperty(colName,
                                     propertyReadOnly.get(colName),
-                                    !propertyReadOnly.get(colName),
-                                    propertyNullable.get(colName), value, type);
+                                    propertyPersistable.get(colName),
+                                    propertyNullable.get(colName),
+                                    propertyPrimaryKey.get(colName), value,
+                                    type);
                             itemProperties.add(cp);
                             propertiesToAdd.remove(colName);
                         }
diff --git a/server/src/com/vaadin/data/util/sqlcontainer/query/generator/DefaultSQLGenerator.java b/server/src/com/vaadin/data/util/sqlcontainer/query/generator/DefaultSQLGenerator.java
index c4b640e274..6ebefcd85c 100644
--- a/server/src/com/vaadin/data/util/sqlcontainer/query/generator/DefaultSQLGenerator.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/query/generator/DefaultSQLGenerator.java
@@ -327,10 +327,8 @@ public class DefaultSQLGenerator implements SQLGenerator {
                     && cp.getPropertyId().equalsIgnoreCase("rownum")) {
                 continue;
             }
-            Object value = cp.getValue() == null ? null : cp.getValue();
-            /* Only include properties whose read-only status can be altered */
-            if (cp.isReadOnlyChangeAllowed() && !cp.isVersionColumn()) {
-                columnToValueMap.put(cp.getPropertyId(), value);
+            if (cp.isPersistent()) {
+                columnToValueMap.put(cp.getPropertyId(), cp.getValue());
             }
         }
         return columnToValueMap;
@@ -345,8 +343,16 @@ public class DefaultSQLGenerator implements SQLGenerator {
                     && cp.getPropertyId().equalsIgnoreCase("rownum")) {
                 continue;
             }
-            Object value = cp.getValue() == null ? null : cp.getValue();
-            if (!cp.isReadOnlyChangeAllowed() || cp.isVersionColumn()) {
+
+            if (cp.isRowIdentifier()) {
+                Object value;
+                if (cp.isPrimaryKey()) {
+                    // If the value of a primary key has changed, its old value
+                    // should be used to identify the row (#9145)
+                    value = cp.getOldValue();
+                } else {
+                    value = cp.getValue();
+                }
                 rowIdentifiers.put(cp.getPropertyId(), value);
             }
         }