RE1-T122 Trying to fix sql server update issue with worker #419
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // // Copyright (c) smiggleworth. All rights reserved. | ||
| // // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using Quidjibo.Configurations; | ||
| using Quidjibo.Constants; | ||
|
|
||
| namespace Quidjibo.SqlServer.Configurations | ||
| { | ||
| public class SqlServerQuidjiboConfiguration : IQuidjiboConfiguration | ||
| { | ||
| public int PollingInterval { get; set; } = 10; | ||
| public string ConnectionString { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// The number of days to keep completed/faulted work items. | ||
| /// </summary> | ||
| public int DaysToKeep { get; set; } = 3; | ||
|
|
||
| public int BatchSize { get; set; } = 5; | ||
|
|
||
| /// <inheritdoc /> | ||
| public int? WorkPollingInterval { get; set; } | ||
|
|
||
| /// <inheritdoc /> | ||
| public bool EnableScheduler { get; set; } = true; | ||
|
|
||
| /// <inheritdoc /> | ||
| public int? SchedulePollingInterval { get; set; } | ||
|
|
||
| /// <inheritdoc /> | ||
| public bool EnableWorker { get; set; } = true; | ||
|
|
||
| /// <inheritdoc /> | ||
| public bool SingleLoop { get; set; } = true; | ||
|
|
||
| /// <inheritdoc /> | ||
| public int LockInterval { get; set; } = 30; | ||
|
|
||
| /// <inheritdoc /> | ||
| public int MaxAttempts { get; set; } = 5; | ||
|
|
||
| /// <inheritdoc /> | ||
| public int Throttle { get; set; } = 10; | ||
|
|
||
| /// <inheritdoc /> | ||
| public string[] Queues { get; set; } = Default.Queues; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,95 @@ | ||||||||||||||||||||
| using System; | ||||||||||||||||||||
| using Quidjibo.Constants; | ||||||||||||||||||||
| using Quidjibo.SqlServer.Configurations; | ||||||||||||||||||||
| using Quidjibo.SqlServer.Factories; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| namespace Quidjibo.SqlServer.Extensions | ||||||||||||||||||||
| { | ||||||||||||||||||||
| public static class QuidjiboBuilderExtensions | ||||||||||||||||||||
| { | ||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Use Sql Server for Work, Progress, an Scheduled Jobs | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| /// <param name="builder"></param> | ||||||||||||||||||||
| /// <param name="sqlServerQuidjiboConfiguration"></param> | ||||||||||||||||||||
| /// <returns></returns> | ||||||||||||||||||||
| public static QuidjiboBuilder UseSqlServer(this QuidjiboBuilder builder, Action<SqlServerQuidjiboConfiguration> sqlServerQuidjiboConfiguration) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| var config = new SqlServerQuidjiboConfiguration(); | ||||||||||||||||||||
| sqlServerQuidjiboConfiguration(config); | ||||||||||||||||||||
| return builder.UseSqlServer(config); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Use Sql Server for Work, Progress, and Scheduled Jobs | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| /// <param name="builder"></param> | ||||||||||||||||||||
| /// <param name="sqlServerQuidjiboConfiguration"></param> | ||||||||||||||||||||
| /// <returns></returns> | ||||||||||||||||||||
| public static QuidjiboBuilder UseSqlServer(this QuidjiboBuilder builder, SqlServerQuidjiboConfiguration sqlServerQuidjiboConfiguration) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| return builder.Configure(sqlServerQuidjiboConfiguration) | ||||||||||||||||||||
| .ConfigureWorkProviderFactory(new SqlWorkProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration)) | ||||||||||||||||||||
| .ConfigureProgressProviderFactory(new SqlProgressProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration.ConnectionString)) | ||||||||||||||||||||
| .ConfigureScheduleProviderFactory(new SqlScheduleProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration.ConnectionString)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Use Sql Server for Work, Progress, and Scheduled Jobs | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| /// <param name="builder"></param> | ||||||||||||||||||||
| /// <param name="connectionString"></param> | ||||||||||||||||||||
| /// <param name="queues"></param> | ||||||||||||||||||||
| /// <returns></returns> | ||||||||||||||||||||
| public static QuidjiboBuilder UseSqlServer(this QuidjiboBuilder builder, string connectionString, params string[] queues) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| if (queues == null || queues.Length == 0) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| queues = Default.Queues; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| var config = new SqlServerQuidjiboConfiguration | ||||||||||||||||||||
| { | ||||||||||||||||||||
| ConnectionString = connectionString, | ||||||||||||||||||||
| Queues = queues | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| return builder.Configure(config) | ||||||||||||||||||||
| .ConfigureWorkProviderFactory(new SqlWorkProviderFactory(builder.LoggerFactory, config)) | ||||||||||||||||||||
| .ConfigureProgressProviderFactory(new SqlProgressProviderFactory(builder.LoggerFactory, connectionString)) | ||||||||||||||||||||
| .ConfigureScheduleProviderFactory(new SqlScheduleProviderFactory(builder.LoggerFactory, connectionString)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Use Sql Server For Work | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| /// <param name="builder"></param> | ||||||||||||||||||||
| /// <param name="sqlServerQuidjiboConfiguration"></param> | ||||||||||||||||||||
| /// <returns></returns> | ||||||||||||||||||||
| public static QuidjiboBuilder UseSqlServerForWork(this QuidjiboBuilder builder, SqlServerQuidjiboConfiguration sqlServerQuidjiboConfiguration) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| return builder.ConfigureWorkProviderFactory(new SqlWorkProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+68
to
+71
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unlike the full Proposed fix public static QuidjiboBuilder UseSqlServerForWork(this QuidjiboBuilder builder, SqlServerQuidjiboConfiguration sqlServerQuidjiboConfiguration)
{
- return builder.ConfigureWorkProviderFactory(new SqlWorkProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration));
+ return builder.Configure(sqlServerQuidjiboConfiguration)
+ .ConfigureWorkProviderFactory(new SqlWorkProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration));
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Use Sql Server For Progress | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| /// <param name="builder"></param> | ||||||||||||||||||||
| /// <param name="connectionString"></param> | ||||||||||||||||||||
| /// <returns></returns> | ||||||||||||||||||||
| public static QuidjiboBuilder UseSqlServerForProgress(this QuidjiboBuilder builder, string connectionString) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| return builder.ConfigureProgressProviderFactory(new SqlProgressProviderFactory(builder.LoggerFactory, connectionString)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||
| /// Use Sql Server For Scheduled Jobs | ||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||
| /// <param name="builder"></param> | ||||||||||||||||||||
| /// <param name="connectionString"></param> | ||||||||||||||||||||
| /// <returns></returns> | ||||||||||||||||||||
| public static QuidjiboBuilder UseSqlServerForSchedule(this QuidjiboBuilder builder, string connectionString) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| return builder.ConfigureScheduleProviderFactory(new SqlScheduleProviderFactory(builder.LoggerFactory, connectionString)); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // Copyright (c) smiggleworth. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System; | ||
| using Microsoft.Data.SqlClient; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Quidjibo.Models; | ||
| using Quidjibo.SqlServer.Providers; | ||
| using Quidjibo.SqlServer.Utils; | ||
|
|
||
| namespace Quidjibo.SqlServer.Extensions | ||
| { | ||
| public static class SqlCommandExtensions | ||
| { | ||
| public static async Task PrepareForSendAsync(this SqlCommand cmd, WorkItem item, int delay, CancellationToken cancellationToken) | ||
| { | ||
| var createdOn = DateTime.UtcNow; | ||
| var visibleOn = createdOn.AddSeconds(delay); | ||
| var expireOn = item.ExpireOn == default(DateTime) ? visibleOn.AddDays(SqlWorkProvider.DEFAULT_EXPIRE_DAYS) : item.ExpireOn; | ||
|
|
||
| #pragma warning disable CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.CommandText = await SqlLoader.GetScript("Work.Send"); | ||
| #pragma warning restore CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.AddParameter("@Id", item.Id); | ||
| cmd.AddParameter("@ScheduleId", item.ScheduleId); | ||
| cmd.AddParameter("@CorrelationId", item.CorrelationId); | ||
| cmd.AddParameter("@Name", item.Name); | ||
| cmd.AddParameter("@Worker", item.Worker); | ||
| cmd.AddParameter("@Queue", item.Queue); | ||
| cmd.AddParameter("@Attempts", item.Attempts); | ||
| cmd.AddParameter("@CreatedOn", createdOn); | ||
| cmd.AddParameter("@ExpireOn", expireOn); | ||
| cmd.AddParameter("@VisibleOn", visibleOn); | ||
| cmd.AddParameter("@Status", SqlWorkProvider.StatusFlags.New); | ||
| cmd.AddParameter("@Payload", item.Payload); | ||
| } | ||
|
|
||
| public static async Task PrepareForRenewAsync(this SqlCommand cmd, WorkItem item, DateTime lockExpireOn, CancellationToken cancellationToken) | ||
| { | ||
| #pragma warning disable CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.CommandText = await SqlLoader.GetScript("Work.Renew"); | ||
| #pragma warning restore CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.AddParameter("@Id", item.Id); | ||
| cmd.AddParameter("@VisibleOn", lockExpireOn); | ||
| } | ||
|
|
||
| public static async Task PrepareForCompleteAsync(this SqlCommand cmd, WorkItem item, CancellationToken cancellationToken) | ||
| { | ||
| #pragma warning disable CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.CommandText = await SqlLoader.GetScript("Work.Complete"); | ||
| #pragma warning restore CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.AddParameter("@Id", item.Id); | ||
| cmd.AddParameter("@Complete", SqlWorkProvider.StatusFlags.Complete); | ||
| } | ||
|
|
||
| public static async Task PrepareForFaultAsync(this SqlCommand cmd, WorkItem item, int visibilityTimeout, CancellationToken cancellationToken) | ||
| { | ||
| var faultedOn = DateTime.UtcNow; | ||
|
|
||
| #pragma warning disable CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.CommandText = await SqlLoader.GetScript("Work.Fault"); | ||
| #pragma warning restore CA2100 // Review SQL queries for security vulnerabilities | ||
| cmd.AddParameter("@Id", item.Id); | ||
| cmd.AddParameter("@VisibleOn", faultedOn.AddSeconds(Math.Max(visibilityTimeout, 30))); | ||
| cmd.AddParameter("@Faulted", SqlWorkProvider.StatusFlags.Faulted); | ||
| } | ||
|
Comment on lines
+39
to
+67
Contributor
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Include a lease owner/token in work state transitions. Renew, complete, and fault currently prepare updates using only 🤖 Prompt for AI Agents |
||
|
|
||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| using System; | ||
| using Microsoft.Data.SqlClient; | ||
|
|
||
| namespace Quidjibo.SqlServer.Extensions | ||
| { | ||
| public static class SqlParameterCollectionExtensions | ||
| { | ||
| public static SqlParameter AddParameter(this SqlCommand cmd, string name, object value) | ||
| { | ||
| return cmd.Parameters.AddWithValue(name, value ?? DBNull.Value); | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail fast on null configuration and blank connection strings.
These entry points pass configuration straight into the factories, so a missing delegate/config/connection string is only discovered later when the provider first opens SQL connections. Validate the public inputs here so startup fails deterministically.
Proposed fix
public static QuidjiboBuilder UseSqlServer(this QuidjiboBuilder builder, Action<SqlServerQuidjiboConfiguration> sqlServerQuidjiboConfiguration) { + ArgumentNullException.ThrowIfNull(builder); + ArgumentNullException.ThrowIfNull(sqlServerQuidjiboConfiguration); + var config = new SqlServerQuidjiboConfiguration(); sqlServerQuidjiboConfiguration(config); return builder.UseSqlServer(config); } @@ public static QuidjiboBuilder UseSqlServer(this QuidjiboBuilder builder, SqlServerQuidjiboConfiguration sqlServerQuidjiboConfiguration) { + ArgumentNullException.ThrowIfNull(builder); + ArgumentNullException.ThrowIfNull(sqlServerQuidjiboConfiguration); + if (string.IsNullOrWhiteSpace(sqlServerQuidjiboConfiguration.ConnectionString)) + throw new ArgumentException("ConnectionString is required.", nameof(sqlServerQuidjiboConfiguration)); + return builder.Configure(sqlServerQuidjiboConfiguration) .ConfigureWorkProviderFactory(new SqlWorkProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration)) .ConfigureProgressProviderFactory(new SqlProgressProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration.ConnectionString)) .ConfigureScheduleProviderFactory(new SqlScheduleProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration.ConnectionString)); } @@ public static QuidjiboBuilder UseSqlServer(this QuidjiboBuilder builder, string connectionString, params string[] queues) { + ArgumentNullException.ThrowIfNull(builder); + if (string.IsNullOrWhiteSpace(connectionString)) + throw new ArgumentException("Connection string is required.", nameof(connectionString)); + if (queues == null || queues.Length == 0) { queues = Default.Queues; } @@ public static QuidjiboBuilder UseSqlServerForWork(this QuidjiboBuilder builder, SqlServerQuidjiboConfiguration sqlServerQuidjiboConfiguration) { + ArgumentNullException.ThrowIfNull(builder); + ArgumentNullException.ThrowIfNull(sqlServerQuidjiboConfiguration); + if (string.IsNullOrWhiteSpace(sqlServerQuidjiboConfiguration.ConnectionString)) + throw new ArgumentException("ConnectionString is required.", nameof(sqlServerQuidjiboConfiguration)); + return builder.ConfigureWorkProviderFactory(new SqlWorkProviderFactory(builder.LoggerFactory, sqlServerQuidjiboConfiguration)); } @@ public static QuidjiboBuilder UseSqlServerForProgress(this QuidjiboBuilder builder, string connectionString) { + ArgumentNullException.ThrowIfNull(builder); + if (string.IsNullOrWhiteSpace(connectionString)) + throw new ArgumentException("Connection string is required.", nameof(connectionString)); + return builder.ConfigureProgressProviderFactory(new SqlProgressProviderFactory(builder.LoggerFactory, connectionString)); } @@ public static QuidjiboBuilder UseSqlServerForSchedule(this QuidjiboBuilder builder, string connectionString) { + ArgumentNullException.ThrowIfNull(builder); + if (string.IsNullOrWhiteSpace(connectionString)) + throw new ArgumentException("Connection string is required.", nameof(connectionString)); + return builder.ConfigureScheduleProviderFactory(new SqlScheduleProviderFactory(builder.LoggerFactory, connectionString)); }Also applies to: 29-35, 44-60, 68-71, 79-93
🤖 Prompt for AI Agents