#824 Add system property to configure default enableProtocol value

devdoc/jdp/jdp-2023-04-disable-unsupported-protocols.md

@@ -3,8 +3,9 @@
 ## Status
 
 - Published: 2023-02-25
-- Updated: 2024-01-10
+- Updated: 2024-11-05
 - Implemented in: Jaybird 6
+- Updated by: [jdp-2024-07](https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2024-07-add-system-property-to-configure-default-enable-protocol.adoc)
 
 ## Type
 

devdoc/jdp/jdp-2024-07-add-system-property-to-configure-default-enable-protocol.adoc

@@ -0,0 +1,26 @@
+= jdp-2024-07: Add System Property to Configure Default of `enableProtocol`
+
+== Status
+
+* Published: 2024-11-05
+* Implemented in: Jaybird 6
+* Updates: https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2023-04-disable-unsupported-protocols.md[jdp-2023-04]
+
+== Type
+
+* Feature-Specification
+
+== Context
+
+In https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2023-04-disable-unsupported-protocols.md[jdp-2023-04], we disabled unsupported protocols by default (that is, wire protocol versions 10 - 12 for pure Java connections), and added a connection property `enableProtocol` which can selectively enable unsupported protocols with a comma-separated list of versions, or all unsupported protocols with '```*```'.
+
+In some cases, it might be easier for users to configure a system property than update connection properties, so having a way to globally control this default will be handy to have as a fallback measure.
+
+== Decision
+
+Jaybird will add a connection property `org.firebirdsql.jdbc.defaultEnableProtocol` with the same syntax as the `enableProtocol` connection property.
+This system property will establish the default value for `enableProtocol`, and will be evaluated for each connection, so it can also be changed on the fly during run time.
+
+== Consequences
+
+A default value for `enableProtocol` can be established without having to specify the connection property explicitly, by setting a system property on startup, or during run time.

src/docs/asciidoc/release_notes.adoc

@@ -487,6 +487,11 @@ However, we recommend using the unmasked version (e.g. `"12"` for protocol versi
 * `"*"` -- enable all available protocol versions
 * `null` or empty string (`++""++`) -- default behaviour, only use supported protocols
 
+A different default value of `enableProtocol` can be set using the system property `org.firebirdsql.jdbc.defaultEnableProtocol`.
+This system property is checked each time a connection configuration is created, so it can be changed at runtime.
+If you use a Jaybird `DataSource` implementation, it uses the value at the time the `DataSource` is created;
+if you use `DriverManager` -- this can include third-party data sources, it uses the value at the time the connection is created.
+
 [WARNING]
 ====
 Given these protocol versions and their Firebird version are not supported, there is no guarantee that the driver will function correctly when an unsupported protocol is enabled.

src/main/org/firebirdsql/gds/JaybirdSystemProperties.java

@@ -39,6 +39,7 @@ public final class JaybirdSystemProperties {
     public static final String PROCESS_NAME_PROP = JDBC_PREFIX + "processName";
     public static final String DEFAULT_CONNECTION_ENCODING_PROPERTY = JDBC_PREFIX + "defaultConnectionEncoding";
     public static final String REQUIRE_CONNECTION_ENCODING_PROPERTY = JDBC_PREFIX + "requireConnectionEncoding";
+    public static final String DEFAULT_ENABLE_PROTOCOL = JDBC_PREFIX + "defaultEnableProtocol";
     public static final String DATATYPE_CODER_CACHE_SIZE = COMMON_PREFIX + "datatypeCoderCacheSize";
     public static final String NATIVE_LIBRARY_SHUTDOWN_DISABLED = COMMON_PREFIX + "nativeResourceShutdownDisabled";
     public static final String WIRE_DEFLATE_BUFFER_SIZE = WIRE_PREFIX + "deflateBufferSize";
@@ -99,6 +100,10 @@ public static int getWireOutputBufferSize(int defaultValue) {
         return getWithDefault(WIRE_OUTPUT_BUFFER_SIZE, defaultValue);
     }
 
+    public static String getDefaultEnableProtocol() {
+        return getSystemPropertyPrivileged(DEFAULT_ENABLE_PROTOCOL);
+    }
+
     private static int getWithDefault(String propertyName, int defaultValue) {
         Integer value = getIntegerSystemPropertyPrivileged(propertyName);
         return value != null ? value : defaultValue;

src/main/org/firebirdsql/gds/ng/AbstractAttachProperties.java

@@ -18,6 +18,7 @@
  */
 package org.firebirdsql.gds.ng;
 
+import org.firebirdsql.gds.JaybirdSystemProperties;
 import org.firebirdsql.jaybird.props.PropertyNames;
 import org.firebirdsql.jaybird.props.def.ConnectionProperty;
 import org.firebirdsql.jaybird.props.internal.ConnectionPropertyRegistry;
@@ -65,17 +66,16 @@ public void afterUpdate(ConnectionProperty connectionProperty, Object newValue)
      *         Source to copy from
      */
     protected AbstractAttachProperties(IAttachProperties<T> src) {
-        this();
-        if (src != null) {
-            propertyValues.putAll(src.connectionPropertyValues());
-        }
+        // Do not call this() as that also sets defaults, which should not be done when copying
+        propertyValues = src != null ? new HashMap<>(src.connectionPropertyValues()) : new HashMap<>();
     }
 
     /**
      * Default constructor for AbstractAttachProperties
      */
     protected AbstractAttachProperties() {
         propertyValues = new HashMap<>();
+        setEnableProtocol(JaybirdSystemProperties.getDefaultEnableProtocol());
     }
 
     // For internal use, to provide serialization support in FbConnectionProperties

src/test/org/firebirdsql/common/SystemPropertyHelper.java

@@ -0,0 +1,89 @@
+/*
+ * Firebird Open Source JDBC Driver
+ *
+ * Distributable under LGPL license.
+ * You may obtain a copy of the License at http://www.gnu.org/copyleft/lgpl.html
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * LGPL License for more details.
+ *
+ * This file was created by members of the firebird development team.
+ * All individual contributions remain the Copyright (C) of those
+ * individuals.  Contributors to this file are either listed here or
+ * can be obtained from a source control history command.
+ *
+ * All rights reserved.
+ */
+package org.firebirdsql.common;
+
+import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
+
+/**
+ * Helper methods for managing system properties during a test.
+ *
+ * @author Mark Rotteveel
+ */
+@NullMarked
+public final class SystemPropertyHelper {
+
+    private SystemPropertyHelper() {
+        // no instances
+    }
+
+    /**
+     * Temporarily sets the system property {@code name} to {@code value}, restoring the original value when
+     * {@link AutoCloseable#close()} is called.
+     * <p>
+     * The recommended use for this method is in a try-with-resources statement.
+     * </p>
+     *
+     * @param name
+     *         name of the system property
+     * @param value
+     *         value of the system property ({@code null} will clear/remove the system property if it exists)
+     * @return auto-closeable which will restore the original value of the system property
+     */
+    public static AutoCloseable withTemporarySystemProperty(String name, @Nullable String value) {
+        record TemporarySystemProperty(String name, @Nullable String value, @Nullable String originalValue)
+                implements AutoCloseable {
+
+            TemporarySystemProperty {
+                if (name == null) {
+                    throw new NullPointerException("name");
+                }
+                setSystemProperty(name, value);
+            }
+
+            TemporarySystemProperty(String name, @Nullable String value) {
+                this(name, value, System.getProperty(name));
+            }
+
+            @Override
+            public void close() {
+                setSystemProperty(name, originalValue);
+            }
+        }
+
+        return new TemporarySystemProperty(name, value);
+    }
+
+    /**
+     * Sets the system property {@code name} to {@code value}, or clears the property if {@code value} is {@code null}.
+     *
+     * @param name
+     *         system property name
+     * @param value
+     *         new value
+     */
+    public static void setSystemProperty(String name, @Nullable String value) {
+        if (value == null) {
+            System.clearProperty(name);
+        } else {
+            System.setProperty(name, value);
+        }
+    }
+
+}

src/test/org/firebirdsql/common/SystemPropertyHelperTest.java

@@ -0,0 +1,63 @@
+/*
+ * Firebird Open Source JDBC Driver
+ *
+ * Distributable under LGPL license.
+ * You may obtain a copy of the License at http://www.gnu.org/copyleft/lgpl.html
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * LGPL License for more details.
+ *
+ * This file was created by members of the firebird development team.
+ * All individual contributions remain the Copyright (C) of those
+ * individuals.  Contributors to this file are either listed here or
+ * can be obtained from a source control history command.
+ *
+ * All rights reserved.
+ */
+package org.firebirdsql.common;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+/**
+ * @author Mark Rotteveel
+ */
+class SystemPropertyHelperTest {
+
+    private static final String TEST_PROPERTY_NAME = "org.firebirdsql.common.SystemPropertyHelperTest";
+    private static final String INITIAL_VALUE = "Initial value";
+
+    @BeforeEach
+    void removeTestProperty() {
+        System.clearProperty(TEST_PROPERTY_NAME);
+    }
+
+    @ParameterizedTest
+    @NullAndEmptySource
+    @ValueSource(strings = { "Test value", INITIAL_VALUE })
+    void withTemporarySystemProperty_propertyDidNotExist(String testValue) throws Exception {
+        try (var ignored = SystemPropertyHelper.withTemporarySystemProperty(TEST_PROPERTY_NAME, testValue)) {
+            assertEquals(testValue, System.getProperty(TEST_PROPERTY_NAME), "Unexpected value in try");
+        }
+        assertNull(System.getProperty(TEST_PROPERTY_NAME), "Unexpected value after try");
+    }
+
+    @ParameterizedTest
+    @NullAndEmptySource
+    @ValueSource(strings = { "Test value", INITIAL_VALUE })
+    void withTemporarySystemProperty_propertyExisted(String testValue) throws Exception{
+        System.setProperty(TEST_PROPERTY_NAME, INITIAL_VALUE);
+        try (var ignored = SystemPropertyHelper.withTemporarySystemProperty(TEST_PROPERTY_NAME, testValue)) {
+            assertEquals(testValue, System.getProperty(TEST_PROPERTY_NAME), "Unexpected value in try");
+        }
+        assertEquals(INITIAL_VALUE, System.getProperty(TEST_PROPERTY_NAME), "Unexpected value after try");
+    }
+
+}

src/test/org/firebirdsql/gds/ng/FbConnectionPropertiesTest.java

@@ -18,10 +18,14 @@
  */
 package org.firebirdsql.gds.ng;
 
+import org.firebirdsql.gds.JaybirdSystemProperties;
 import org.firebirdsql.jaybird.props.PropertyConstants;
 import org.firebirdsql.jaybird.props.def.ConnectionProperty;
 import org.firebirdsql.jaybird.props.internal.ConnectionPropertyRegistry;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.util.Arrays;
 import java.util.HashMap;
@@ -30,6 +34,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.firebirdsql.common.SystemPropertyHelper.withTemporarySystemProperty;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
 
@@ -261,4 +266,14 @@ void testSessionTimeZoneNormalizationOfOffsets() {
         assertEquals("+05:00", info.getSessionTimeZone());
     }
 
+    @ParameterizedTest
+    @NullAndEmptySource
+    @ValueSource(strings = { "*", "11" })
+    void enableProtocolDefaultDerivedFromSystemProperty(String defaultValue) throws Exception {
+        try (var ignored = withTemporarySystemProperty(JaybirdSystemProperties.DEFAULT_ENABLE_PROTOCOL, defaultValue)) {
+            assertEquals(defaultValue, new FbConnectionProperties().getEnableProtocol(),
+                    "Unexpected enableProtocol value");
+        }
+    }
+
 }

src/test/org/firebirdsql/gds/ng/FbServicePropertiesTest.java

@@ -0,0 +1,44 @@
+/*
+ * Firebird Open Source JDBC Driver
+ *
+ * Distributable under LGPL license.
+ * You may obtain a copy of the License at http://www.gnu.org/copyleft/lgpl.html
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * LGPL License for more details.
+ *
+ * This file was created by members of the firebird development team.
+ * All individual contributions remain the Copyright (C) of those
+ * individuals.  Contributors to this file are either listed here or
+ * can be obtained from a source control history command.
+ *
+ * All rights reserved.
+ */
+package org.firebirdsql.gds.ng;
+
+import org.firebirdsql.gds.JaybirdSystemProperties;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.firebirdsql.common.SystemPropertyHelper.withTemporarySystemProperty;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Tests for {@link FbServiceProperties}.
+ */
+class FbServicePropertiesTest {
+
+    @ParameterizedTest
+    @NullAndEmptySource
+    @ValueSource(strings = { "*", "11" })
+    void enableProtocolDefaultDerivedFromSystemProperty(String defaultValue) throws Exception {
+        try (var ignored = withTemporarySystemProperty(JaybirdSystemProperties.DEFAULT_ENABLE_PROTOCOL, defaultValue)) {
+            assertEquals(defaultValue, new FbServiceProperties().getEnableProtocol(),
+                    "Unexpected enableProtocol value");
+        }
+    }
+
+}