From b35d2ae8724b4639398b87e89034f1781bf08832 Mon Sep 17 00:00:00 2001
From: Marco Collovati <mcollovati@gmail.com>
Date: Thu, 28 Dec 2017 09:04:50 +0100
Subject: Make Binder.setReadonly ignore effectively readonly bindings (#10368)

Binder.setReadonly will ignore bindings with null setter
or property without an accessible setter when changing
field read only flag.

Fixes #10252
---
 server/src/main/java/com/vaadin/data/Binder.java   | 18 +++++++++++-----
 .../test/java/com/vaadin/data/BeanBinderTest.java  | 18 ++++++++++++++++
 .../src/test/java/com/vaadin/data/BinderTest.java  | 25 ++++++++++++++++++++++
 3 files changed, 56 insertions(+), 5 deletions(-)

(limited to 'server/src')

diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java
index a443421a0f..26354138ce 100644
--- a/server/src/main/java/com/vaadin/data/Binder.java
+++ b/server/src/main/java/com/vaadin/data/Binder.java
@@ -33,6 +33,7 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.BiFunction;
+import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -762,10 +763,10 @@ public class Binder<BEAN> implements Serializable {
                                     + " from " + getBinder().propertySet));
 
             ValueProvider<BEAN, ?> getter = definition.getGetter();
-            Setter<BEAN, ?> setter = definition.getSetter()
-                    .orElse((bean, value) -> {
-                        // Setter ignores value
-                    });
+            Setter<BEAN, ?> setter = definition.getSetter().orElse(null);
+            if (setter == null) {
+                getLogger().fine(() -> propertyName + " does not have an accessible setter");
+            }
 
             BindingBuilder<BEAN, ?> finalBinding = withConverter(
                     createConverter(definition.getType()), false);
@@ -2287,7 +2288,9 @@ public class Binder<BEAN> implements Serializable {
      *            write
      */
     public void setReadOnly(boolean fieldsReadOnly) {
-        getBindings().stream().map(BindingImpl::getField)
+        getBindings().stream()
+            .filter(binding -> Objects.nonNull(binding.setter))
+            .map(BindingImpl::getField)
                 .forEach(field -> field.setReadOnly(fieldsReadOnly));
     }
 
@@ -2740,4 +2743,9 @@ public class Binder<BEAN> implements Serializable {
         Optional.ofNullable(boundProperties.get(propertyName))
                 .ifPresent(Binding::unbind);
     }
+
+    private static final Logger getLogger() {
+        return Logger.getLogger(Binder.class.getName());
+    }
+
 }
diff --git a/server/src/test/java/com/vaadin/data/BeanBinderTest.java b/server/src/test/java/com/vaadin/data/BeanBinderTest.java
index 45610754d5..567bdcc3e8 100644
--- a/server/src/test/java/com/vaadin/data/BeanBinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BeanBinderTest.java
@@ -310,6 +310,24 @@ public class BeanBinderTest
         assertEquals(propertyValue, item.getReadOnlyProperty());
     }
 
+    @Test
+    public void setReadonlyShouldIgnoreBindingsForReadOnlyProperties() {
+        binder.bind(nameField, "readOnlyProperty");
+
+        binder.setReadOnly(true);
+        assertFalse("Name field should be ignored and not be readonly", nameField.isReadOnly());
+
+        binder.setReadOnly(false);
+        assertFalse("Name field should be ignored and not be readonly", nameField.isReadOnly());
+
+        nameField.setReadOnly(true);
+        binder.setReadOnly(true);
+        assertTrue("Name field should be ignored and be readonly", nameField.isReadOnly());
+
+        binder.setReadOnly(false);
+        assertTrue("Name field should be ignored and be readonly", nameField.isReadOnly());
+    }
+
     @Test
     public void beanBound_setInvalidFieldValue_validationError() {
         binder.setBean(item);
diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java
index 36301afcfa..5c6c9657a5 100644
--- a/server/src/test/java/com/vaadin/data/BinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderTest.java
@@ -608,6 +608,31 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
         assertFalse(ageField.isReadOnly());
     }
 
+    @Test
+    public void setReadonlyShouldIgnoreBindingsWithNullSetter() {
+        binder.bind(nameField, Person::getFirstName, null);
+        binder.forField(ageField)
+            .withConverter(new StringToIntegerConverter(""))
+            .bind(Person::getAge, Person::setAge);
+
+        binder.setReadOnly(true);
+        assertFalse("Name field should be ignored and not be readonly", nameField.isReadOnly());
+        assertTrue("Age field should be readonly", ageField.isReadOnly());
+
+        binder.setReadOnly(false);
+        assertFalse("Name field should be ignored and remain not readonly", nameField.isReadOnly());
+        assertFalse("Age field should not be readonly", ageField.isReadOnly());
+
+        nameField.setReadOnly(false);
+        binder.setReadOnly(false);
+        assertFalse("Name field should be ignored and remain not readonly", nameField.isReadOnly());
+        assertFalse("Age field should not be readonly", ageField.isReadOnly());
+
+        binder.setReadOnly(true);
+        assertFalse("Name field should be ignored and remain not readonly", nameField.isReadOnly());
+        assertTrue("Age field should be readonly", ageField.isReadOnly());
+    }
+
     @Test
     public void isValidTest_bound_binder() {
         binder.forField(nameField)
-- 
cgit v1.2.3