Repository: gchq/sleeper
Issue: Admin client doesn't display default value of table property when editing

To reproduce, in the admin client, set a value for a table property which was previously taking its default value from an instance property.

In the screen where it summarises the change, it does not show you the default value. It only tells you it was previously unset.

Changed files:
java/configuration/src/main/java/sleeper/configuration/properties/table/TablePropertyImpl.java
java/configuration/src/test/java/sleeper/configuration/properties/table/TablePropertiesTest.java
java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationTest.java

diff --git a/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationTest.java b/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationTest.java
index b4624caa5..d50cbdef2 100644
--- a/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationTest.java
+++ b/java/clients/src/test/java/sleeper/clients/admin/InstanceConfigurationTest.java
@@ -56,6 +56,7 @@ import static sleeper.configuration.properties.UserDefinedInstanceProperty.OPTIO
 import static sleeper.configuration.properties.UserDefinedInstanceProperty.VPC_ID;
 import static sleeper.configuration.properties.table.TableProperty.DATA_BUCKET;
 import static sleeper.configuration.properties.table.TableProperty.DYNAMODB_STRONGLY_CONSISTENT_READS;
+import static sleeper.configuration.properties.table.TableProperty.ITERATOR_CONFIG;
 import static sleeper.configuration.properties.table.TableProperty.ROW_GROUP_SIZE;
 import static sleeper.configuration.properties.table.TableProperty.TABLE_NAME;
 
@@ -530,7 +531,7 @@ class InstanceConfigurationTest extends AdminClientMockStoreBase {
             InstanceProperties properties = createValidInstanceProperties();
             TableProperties before = createValidTableProperties(properties);
             TableProperties after = createValidTableProperties(properties);
-            after.set(ROW_GROUP_SIZE, "123");
+            after.set(ITERATOR_CONFIG, "TestIterator");
 
             // When
             String output = editTableConfiguration(properties, before, after)
@@ -621,6 +622,30 @@ class InstanceConfigurationTest extends AdminClientMockStoreBase {
                     "\\n" +
                     PROPERTY_VALIDATION_SCREEN + DISPLAY_MAIN_SCREEN);
         }
+
+        @Test
+        void shouldEditAPropertyThatWasPreviouslyUnsetButHadADefaultProperty() throws Exception {
+            // Given
+            InstanceProperties properties = createValidInstanceProperties();
+            TableProperties before = createValidTableProperties(properties);
+            TableProperties after = createValidTableProperties(properties);
+            after.set(ROW_GROUP_SIZE, "123");
+
+            // When
+            String output = editTableConfiguration(properties, before, after)
+                    .enterPrompts(SaveChangesScreen.SAVE_CHANGES_OPTION, CONFIRM_PROMPT)
+                    .exitGetOutput();
+
+            // Then
+            assertThat(output).startsWith(DISPLAY_MAIN_SCREEN + CLEAR_CONSOLE + TABLE_SELECT_SCREEN)
+                    .contains("Found changes to properties:\\n" +
+                            "\\n" +
+                            "sleeper.table.rowgroup.size\\n" +
+                            "The size of the row group in the Parquet files - defaults to the value in the instance properties.\\n" +
+                            "Unset before, default value: 8388608\\n" +
+                            "After: 123\\n")
+                    .endsWith(PROPERTY_SAVE_CHANGES_SCREEN + PROMPT_SAVE_SUCCESSFUL_RETURN_TO_MAIN + DISPLAY_MAIN_SCREEN);
+        }
     }
 
     @DisplayName("Filter by group")
diff --git a/java/configuration/src/main/java/sleeper/configuration/properties/table/TablePropertyImpl.java b/java/configuration/src/main/java/sleeper/configuration/properties/table/TablePropertyImpl.java
index 25a96752f..42a20901d 100644
--- a/java/configuration/src/main/java/sleeper/configuration/properties/table/TablePropertyImpl.java
+++ b/java/configuration/src/main/java/sleeper/configuration/properties/table/TablePropertyImpl.java
@@ -139,6 +139,7 @@ class TablePropertyImpl implements TableProperty {
 
         public Builder defaultProperty(SleeperProperty defaultProperty) {
             this.defaultProperty = defaultProperty;
+            this.defaultValue = defaultProperty.getDefaultValue();
             return validationPredicate(defaultProperty.validationPredicate());
         }
 
diff --git a/java/configuration/src/test/java/sleeper/configuration/properties/table/TablePropertiesTest.java b/java/configuration/src/test/java/sleeper/configuration/properties/table/TablePropertiesTest.java
index 48c072536..2e4ef8671 100644
--- a/java/configuration/src/test/java/sleeper/configuration/properties/table/TablePropertiesTest.java
+++ b/java/configuration/src/test/java/sleeper/configuration/properties/table/TablePropertiesTest.java
@@ -49,6 +49,21 @@ class TablePropertiesTest {
         assertThat(tableProperties.get(PAGE_SIZE)).isEqualTo("20");
     }
 
+    @Test
+    void shouldGetDefaultValueFromDefaultProperty() {
+        // Given
+        InstanceProperties instanceProperties = new InstanceProperties();
+
+        // When
+        TableProperties tableProperties = new TableProperties(instanceProperties);
+
+        // Then
+        assertThat(tableProperties.get(PAGE_SIZE))
+                .isEqualTo("" + (128 * 1024));
+        assertThat(PAGE_SIZE.getDefaultValue())
+                .isEqualTo("" + (128 * 1024));
+    }
+
     @Test
     void shouldDefaultToAnotherTablePropertyIfConfigured() {
         // Given