From 416dfd2f5b4f2341dfaa7ed4b81fca2125970028 Mon Sep 17 00:00:00 2001 From: Ameya Rele Date: Thu, 3 Nov 2022 16:23:19 +0530 Subject: [PATCH 1/3] Throw exception for scaling configuration invalid value --- src/TriggerBinding/SqlTriggerListener.cs | 17 +++++------------ .../TriggerBinding/SqlTriggerListenerTests.cs | 16 ++++------------ 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/TriggerBinding/SqlTriggerListener.cs b/src/TriggerBinding/SqlTriggerListener.cs index 187a00383..b12e46515 100644 --- a/src/TriggerBinding/SqlTriggerListener.cs +++ b/src/TriggerBinding/SqlTriggerListener.cs @@ -69,24 +69,17 @@ public SqlTriggerListener(string connectionString, string tableName, string user this._executor = executor ?? throw new ArgumentNullException(nameof(executor)); this._logger = logger ?? throw new ArgumentNullException(nameof(logger)); this._configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); - int configuredMaxChangesPerWorker; + int? configuredMaxChangesPerWorker; // Do not convert the scale-monitor ID to lower-case string since SQL table names can be case-sensitive // depending on the collation of the current database. this._scaleMonitorDescriptor = new ScaleMonitorDescriptor($"{userFunctionId}-SqlTrigger-{tableName}"); - // In case converting from string to int is not possible from the user input. - try + configuredMaxChangesPerWorker = configuration.GetValue(SqlTriggerConstants.ConfigKey_SqlTrigger_MaxChangesPerWorker); + this._maxChangesPerWorker = configuredMaxChangesPerWorker ?? DefaultMaxChangesPerWorker; + if (this._maxChangesPerWorker <= 0) { - configuredMaxChangesPerWorker = configuration.GetValue(SqlTriggerConstants.ConfigKey_SqlTrigger_MaxChangesPerWorker); - } - catch (Exception ex) - { - this._logger.LogError($"Failed to resolve integer value from user configured setting '{SqlTriggerConstants.ConfigKey_SqlTrigger_MaxChangesPerWorker}' due to exception: {ex.GetType()}. Exception message: {ex.Message}"); - TelemetryInstance.TrackException(TelemetryErrorName.InvalidConfigurationValue, ex, this._telemetryProps); - - configuredMaxChangesPerWorker = DefaultMaxChangesPerWorker; + throw new ArgumentException($"Invalid value for configuration setting '{SqlTriggerConstants.ConfigKey_SqlTrigger_MaxChangesPerWorker}'. Ensure that the value is a positive integer."); } - this._maxChangesPerWorker = configuredMaxChangesPerWorker > 0 ? configuredMaxChangesPerWorker : DefaultMaxChangesPerWorker; } public void Cancel() diff --git a/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs b/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs index 8995bc18d..d39cb1939 100644 --- a/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs +++ b/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs @@ -232,20 +232,12 @@ public void ScaleMonitorGetScaleStatus_UserConfiguredMaxChangesPerWorker_Respect [InlineData("-1")] [InlineData("0")] [InlineData("10000000000")] - public void ScaleMonitorGetScaleStatus_InvalidUserConfiguredMaxChangesPerWorker_UsesDefaultValue(string maxChangesPerWorker) + public void InvalidUserConfiguredMaxChangesPerWorker(string maxChangesPerWorker) { - (IScaleMonitor monitor, _) = GetScaleMonitor(maxChangesPerWorker); - - ScaleStatusContext context; - ScaleStatus scaleStatus; - - context = GetScaleStatusContext(new int[] { 0, 0, 0, 0, 10000 }, 10); - scaleStatus = monitor.GetScaleStatus(context); - Assert.Equal(ScaleVote.None, scaleStatus.Vote); + (Mock mockLogger, List logMessages) = CreateMockLogger(); + Mock mockConfiguration = CreateMockConfiguration(maxChangesPerWorker); - context = GetScaleStatusContext(new int[] { 0, 0, 0, 0, 10001 }, 10); - scaleStatus = monitor.GetScaleStatus(context); - Assert.Equal(ScaleVote.ScaleOut, scaleStatus.Vote); + Assert.Throws(() => new SqlTriggerListener("testConnectionString", "testTableName", "testUserFunctionId", Mock.Of(), mockLogger.Object, mockConfiguration.Object)); } private static IScaleMonitor GetScaleMonitor(string tableName, string userFunctionId) From ea9f911cf0618a412912686c0908bfc9dc6dd9d6 Mon Sep 17 00:00:00 2001 From: Ameya Rele Date: Mon, 7 Nov 2022 14:23:16 +0530 Subject: [PATCH 2/3] Change Exception type --- src/TriggerBinding/SqlTriggerListener.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/TriggerBinding/SqlTriggerListener.cs b/src/TriggerBinding/SqlTriggerListener.cs index 5d190d27d..0887f5047 100644 --- a/src/TriggerBinding/SqlTriggerListener.cs +++ b/src/TriggerBinding/SqlTriggerListener.cs @@ -79,7 +79,7 @@ public SqlTriggerListener(string connectionString, string tableName, string user this._maxChangesPerWorker = configuredMaxChangesPerWorker ?? DefaultMaxChangesPerWorker; if (this._maxChangesPerWorker <= 0) { - throw new ArgumentException($"Invalid value for configuration setting '{ConfigKey_SqlTrigger_MaxChangesPerWorker}'. Ensure that the value is a positive integer."); + throw new InvalidOperationException($"Invalid value for configuration setting '{ConfigKey_SqlTrigger_MaxChangesPerWorker}'. Ensure that the value is a positive integer."); } } From d6c6941bb3e92a62900f7f5ea03a657a65f22c04 Mon Sep 17 00:00:00 2001 From: Ameya Rele Date: Mon, 7 Nov 2022 14:23:16 +0530 Subject: [PATCH 3/3] Change Exception type --- src/TriggerBinding/SqlTriggerListener.cs | 2 +- test/Unit/TriggerBinding/SqlTriggerListenerTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/TriggerBinding/SqlTriggerListener.cs b/src/TriggerBinding/SqlTriggerListener.cs index 5d190d27d..0887f5047 100644 --- a/src/TriggerBinding/SqlTriggerListener.cs +++ b/src/TriggerBinding/SqlTriggerListener.cs @@ -79,7 +79,7 @@ public SqlTriggerListener(string connectionString, string tableName, string user this._maxChangesPerWorker = configuredMaxChangesPerWorker ?? DefaultMaxChangesPerWorker; if (this._maxChangesPerWorker <= 0) { - throw new ArgumentException($"Invalid value for configuration setting '{ConfigKey_SqlTrigger_MaxChangesPerWorker}'. Ensure that the value is a positive integer."); + throw new InvalidOperationException($"Invalid value for configuration setting '{ConfigKey_SqlTrigger_MaxChangesPerWorker}'. Ensure that the value is a positive integer."); } } diff --git a/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs b/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs index d39cb1939..6f50e099f 100644 --- a/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs +++ b/test/Unit/TriggerBinding/SqlTriggerListenerTests.cs @@ -237,7 +237,7 @@ public void InvalidUserConfiguredMaxChangesPerWorker(string maxChangesPerWorker) (Mock mockLogger, List logMessages) = CreateMockLogger(); Mock mockConfiguration = CreateMockConfiguration(maxChangesPerWorker); - Assert.Throws(() => new SqlTriggerListener("testConnectionString", "testTableName", "testUserFunctionId", Mock.Of(), mockLogger.Object, mockConfiguration.Object)); + Assert.Throws(() => new SqlTriggerListener("testConnectionString", "testTableName", "testUserFunctionId", Mock.Of(), mockLogger.Object, mockConfiguration.Object)); } private static IScaleMonitor GetScaleMonitor(string tableName, string userFunctionId)