Skip to content

Commit a52f9f2

Browse files
committed
RE1-T117 PR#403 fixes
1 parent af50a35 commit a52f9f2

6 files changed

Lines changed: 168 additions & 29 deletions

File tree

Core/Resgrid.Model/Repositories/IUtf8MaintenanceRepository.cs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,17 @@ public interface IUtf8MaintenanceRepository
1515
{
1616
/// <summary>
1717
/// Returns every base-table text column grouped by table, restricted to tables that have a
18-
/// single-column primary key (required for stable keyset pagination).
18+
/// single-column primary key of a pageable type — text/citext, an integer type, or uuid
19+
/// (required for stable keyset pagination). Tables whose PK is some other type are skipped.
1920
/// </summary>
2021
Task<List<Utf8TextColumnTarget>> GetTextColumnTargetsAsync(CancellationToken cancellationToken);
2122

2223
/// <summary>
2324
/// Reads the next batch of rows for <paramref name="target"/> ordered by primary key, starting
24-
/// after <paramref name="lastKey"/> (null/empty for the first page).
25+
/// after <paramref name="lastKey"/> (null/empty for the first page). The cursor is the previous
26+
/// page's last key rendered as an invariant string; the implementation re-binds it to the PK's
27+
/// native type (see <see cref="Utf8TextColumnTarget.PrimaryKeyType"/>) so the keyset comparison
28+
/// works for text, integer and uuid keys alike.
2529
/// </summary>
2630
Task<Utf8RowBatch> GetRowBatchAsync(Utf8TextColumnTarget target, string lastKey, int batchSize, CancellationToken cancellationToken);
2731

@@ -38,12 +42,33 @@ public interface IUtf8MaintenanceRepository
3842
Task SaveProgressAsync(Utf8CleanupProgress progress, CancellationToken cancellationToken);
3943
}
4044

45+
/// <summary>
46+
/// The pageable category of a table's primary key. Keyset pagination renders the cursor as an
47+
/// invariant string and re-binds it as this type for the <c>&gt;</c> / <c>=</c> predicates, so only
48+
/// these PK types are swept; tables with any other PK type are skipped.
49+
/// </summary>
50+
public enum Utf8PrimaryKeyType
51+
{
52+
/// <summary>char/varchar/nchar/nvarchar/text/ntext and PostgreSQL citext — bound as a string.</summary>
53+
Text = 0,
54+
55+
/// <summary>tinyint/smallint/int/bigint (and PostgreSQL integer types) — bound as Int64.</summary>
56+
Integer = 1,
57+
58+
/// <summary>SQL Server uniqueidentifier / PostgreSQL uuid — bound as a Guid.</summary>
59+
Guid = 2
60+
}
61+
4162
/// <summary>A table plus its single-column primary key and the free-form text columns to scan.</summary>
4263
public class Utf8TextColumnTarget
4364
{
4465
public string Schema { get; set; }
4566
public string TableName { get; set; }
4667
public string PrimaryKeyColumn { get; set; }
68+
69+
/// <summary>The PK's value category, so keyset cursors are bound as the PK's native type.</summary>
70+
public Utf8PrimaryKeyType PrimaryKeyType { get; set; }
71+
4772
public List<string> TextColumns { get; set; } = new List<string>();
4873

4974
/// <summary>Stable identifier used as the watermark key (schema-qualified table name).</summary>
@@ -64,6 +89,7 @@ public class Utf8RowBatch
6489
/// </summary>
6590
public class Utf8TextRow
6691
{
92+
/// <summary>The row's primary key rendered as an invariant string (re-bound to its native type for queries).</summary>
6793
public string Key { get; set; }
6894
public Dictionary<string, string> Columns { get; set; } = new Dictionary<string, string>();
6995
}

Core/Resgrid.Services/UsersService.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ public IdentityUser GetUserByEmail(string emailAddress)
9898
public async Task<bool> DoesUserHaveAnyActiveDepartments(string userName)
9999
{
100100
var user = await GetUserByNameAsync(userName);
101+
102+
// Defensive: the user has already authenticated by this point, but if the lookup returns
103+
// null (e.g. a stale/edge data state) treat it as no active departments rather than NRE.
104+
if (user == null)
105+
return false;
106+
101107
var memberships = await _departmentMembersRepository.GetAllByUserIdAsync(user.UserId);
102108

103109
return memberships.Any(x => x.IsDeleted == false && (x.IsDisabled == null || x.IsDisabled == false));

Repositories/Resgrid.Repositories.DataRepository/IdentityRepository.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ public async Task<IdentityUser> GetUserByUserNameAsync(string userName)
6363
{
6464
using (IDbConnection db = new NpgsqlConnection(DataConfig.CoreConnectionString))
6565
{
66-
var result = await db.QueryAsync<IdentityUser>($"SELECT * FROM aspnetusers WHERE username = @userName", new { userName = userName });
66+
// PostgreSQL text comparison is case-sensitive (unlike SQL Server's default collation),
67+
// so match on normalizedusername the same way ASP.NET Identity sign-in does. Otherwise a
68+
// user who authenticated with different casing than the stored username is not found here.
69+
var result = await db.QueryAsync<IdentityUser>($"SELECT * FROM aspnetusers WHERE normalizedusername = @normalizedUserName", new { normalizedUserName = userName?.ToUpperInvariant() });
6770

6871
return result.FirstOrDefault();
6972
}
@@ -87,7 +90,9 @@ public IdentityUser GetUserByEmail(string email)
8790
{
8891
using (IDbConnection db = new NpgsqlConnection(DataConfig.CoreConnectionString))
8992
{
90-
return db.Query<IdentityUser>($"SELECT * FROM aspnetusers WHERE email = @email", new { email = email }).FirstOrDefault();
93+
// PostgreSQL text comparison is case-sensitive; LOWER() both sides so a different-cased email
94+
// still matches (normalizedemail is not maintained on the PG update path, so it can't be relied on).
95+
return db.Query<IdentityUser>($"SELECT * FROM aspnetusers WHERE LOWER(email) = LOWER(@email)", new { email = email }).FirstOrDefault();
9196
}
9297
}
9398
else
@@ -107,7 +112,8 @@ public void UpdateUsername(string oldUsername, string newUsername)
107112
{
108113
using (IDbConnection db = new NpgsqlConnection(DataConfig.CoreConnectionString))
109114
{
110-
db.Execute($"UPDATE public.aspnetusers SET username = @newUsername, normalizedusername = @newUsernameUpper WHERE username = @oldUsername", new { newUsername = newUsername, newUsernameUpper = newUsername.ToUpper(), oldUsername = oldUsername });
115+
// Case-insensitive match on the old username (PostgreSQL text comparison is case-sensitive).
116+
db.Execute($"UPDATE public.aspnetusers SET username = @newUsername, normalizedusername = @newUsernameUpper WHERE LOWER(username) = LOWER(@oldUsername)", new { newUsername = newUsername, newUsernameUpper = newUsername.ToUpper(), oldUsername = oldUsername });
111117
}
112118
}
113119
else
@@ -125,7 +131,9 @@ public void UpdateEmail(string userId, string newEmail)
125131
{
126132
using (IDbConnection db = new NpgsqlConnection(DataConfig.CoreConnectionString))
127133
{
128-
db.Execute($"UPDATE public.aspnetusers SET email = @newEmail WHERE id = @userId", new { userId = userId, newEmail = newEmail });
134+
// Keep normalizedemail in sync (ASP.NET Identity's FindByEmailAsync looks up by it); the
135+
// SQL Server branch already does this. Without it, email lookups go stale after a change.
136+
db.Execute($"UPDATE public.aspnetusers SET email = @newEmail, normalizedemail = @newEmailUpper WHERE id = @userId", new { userId = userId, newEmail = newEmail, newEmailUpper = newEmail?.ToUpper() });
129137
}
130138
}
131139
else

Repositories/Resgrid.Repositories.DataRepository/Utf8MaintenanceRepository.cs

Lines changed: 120 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,17 @@ INNER JOIN information_schema.tables t
7070
AND c.table_schema NOT IN ('pg_catalog', 'information_schema')";
7171

7272
pkSql = @"
73-
SELECT tc.table_schema AS TableSchema, tc.table_name AS TableName, kcu.column_name AS ColumnName
73+
SELECT tc.table_schema AS TableSchema, tc.table_name AS TableName, kcu.column_name AS ColumnName,
74+
col.data_type AS DataType, col.udt_name AS UdtName
7475
FROM information_schema.table_constraints tc
7576
INNER JOIN information_schema.key_column_usage kcu
7677
ON tc.constraint_name = kcu.constraint_name
7778
AND tc.table_schema = kcu.table_schema
7879
AND tc.table_name = kcu.table_name
80+
INNER JOIN information_schema.columns col
81+
ON col.table_schema = kcu.table_schema
82+
AND col.table_name = kcu.table_name
83+
AND col.column_name = kcu.column_name
7984
WHERE tc.constraint_type = 'PRIMARY KEY'
8085
AND tc.table_schema NOT IN ('pg_catalog', 'information_schema')";
8186
}
@@ -89,12 +94,17 @@ INNER JOIN INFORMATION_SCHEMA.TABLES t
8994
WHERE c.DATA_TYPE IN ('char', 'varchar', 'nchar', 'nvarchar', 'text', 'ntext')";
9095

9196
pkSql = @"
92-
SELECT tc.TABLE_SCHEMA AS TableSchema, tc.TABLE_NAME AS TableName, kcu.COLUMN_NAME AS ColumnName
97+
SELECT tc.TABLE_SCHEMA AS TableSchema, tc.TABLE_NAME AS TableName, kcu.COLUMN_NAME AS ColumnName,
98+
col.DATA_TYPE AS DataType
9399
FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS tc
94100
INNER JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE kcu
95101
ON tc.CONSTRAINT_NAME = kcu.CONSTRAINT_NAME
96102
AND tc.TABLE_SCHEMA = kcu.TABLE_SCHEMA
97103
AND tc.TABLE_NAME = kcu.TABLE_NAME
104+
INNER JOIN INFORMATION_SCHEMA.COLUMNS col
105+
ON col.TABLE_SCHEMA = kcu.TABLE_SCHEMA
106+
AND col.TABLE_NAME = kcu.TABLE_NAME
107+
AND col.COLUMN_NAME = kcu.COLUMN_NAME
98108
WHERE tc.CONSTRAINT_TYPE = 'PRIMARY KEY'";
99109
}
100110

@@ -109,7 +119,7 @@ INNER JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE kcu
109119
var singlePk = pkRows
110120
.GroupBy(p => p.TableSchema + "." + p.TableName)
111121
.Where(g => g.Count() == 1)
112-
.ToDictionary(g => g.Key, g => g.First().ColumnName, StringComparer.OrdinalIgnoreCase);
122+
.ToDictionary(g => g.Key, g => g.First(), StringComparer.OrdinalIgnoreCase);
113123

114124
var targets = new List<Utf8TextColumnTarget>();
115125

@@ -120,13 +130,20 @@ INNER JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE kcu
120130
if (ExcludedTables.Contains(first.TableName))
121131
continue;
122132

123-
if (!singlePk.TryGetValue(group.Key, out var pkColumn))
133+
if (!singlePk.TryGetValue(group.Key, out var pk))
124134
continue; // no single-column PK -> cannot page safely
125135

136+
// The keyset cursor is a string re-bound to the PK's native type; only text,
137+
// integer and uuid PKs are supported. Skip anything else (e.g. date/decimal PKs)
138+
// so we never emit a query that compares an incompatible type against the cursor.
139+
var keyType = ClassifyKeyType(pk);
140+
if (keyType == null)
141+
continue;
142+
126143
// Never clean the primary-key column itself.
127144
var textColumns = group
128145
.Select(c => c.ColumnName)
129-
.Where(name => !string.Equals(name, pkColumn, StringComparison.OrdinalIgnoreCase))
146+
.Where(name => !string.Equals(name, pk.ColumnName, StringComparison.OrdinalIgnoreCase))
130147
.ToList();
131148

132149
if (textColumns.Count == 0)
@@ -136,7 +153,8 @@ INNER JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE kcu
136153
{
137154
Schema = first.TableSchema,
138155
TableName = first.TableName,
139-
PrimaryKeyColumn = pkColumn,
156+
PrimaryKeyColumn = pk.ColumnName,
157+
PrimaryKeyType = keyType.Value,
140158
TextColumns = textColumns
141159
});
142160
}
@@ -168,7 +186,7 @@ public async Task<Utf8RowBatch> GetRowBatchAsync(Utf8TextColumnTarget target, st
168186
var dynamicParameters = new DynamicParameters();
169187
dynamicParameters.Add("batchSize", batchSize);
170188
if (hasCursor)
171-
dynamicParameters.Add("lastKey", lastKey);
189+
dynamicParameters.Add("lastKey", ConvertKey(lastKey, target.PrimaryKeyType));
172190

173191
using (DbConnection conn = _connectionProvider.Create())
174192
{
@@ -238,7 +256,7 @@ public async Task<int> UpdateRowsAsync(Utf8TextColumnTarget target, IReadOnlyLis
238256
dynamicParameters.Add(paramName, (object)column.Value ?? DBNull.Value);
239257
}
240258

241-
dynamicParameters.Add("rg_key", row.Key);
259+
dynamicParameters.Add("rg_key", ConvertKey(row.Key, target.PrimaryKeyType));
242260

243261
var sql = $"UPDATE {table} SET {string.Join(", ", setClauses)} WHERE {pk} = @rg_key";
244262

@@ -289,24 +307,45 @@ public async Task SaveProgressAsync(Utf8CleanupProgress progress, CancellationTo
289307
{
290308
try
291309
{
292-
var updateSql = $@"
293-
UPDATE {ProgressTable}
294-
SET lastprocessedkey = @LastProcessedKey, lastcompletedutc = @LastCompletedUtc,
295-
rowsscanned = @RowsScanned, rowsfixed = @RowsFixed, updatedonutc = @UpdatedOnUtc
296-
WHERE tablename = @TableName";
310+
// Atomic upsert: overlapping cleanup runs (long sweeps or multiple workers) can save the
311+
// first watermark for the same table concurrently, which the old UPDATE-then-INSERT raced
312+
// on (duplicate-PK on the second INSERT). The tablename PK makes both forms below race-safe.
313+
string upsertSql;
297314

298-
var insertSql = $@"
299-
INSERT INTO {ProgressTable} (tablename, lastprocessedkey, lastcompletedutc, rowsscanned, rowsfixed, updatedonutc)
300-
VALUES (@TableName, @LastProcessedKey, @LastCompletedUtc, @RowsScanned, @RowsFixed, @UpdatedOnUtc)";
315+
if (IsPostgres)
316+
{
317+
upsertSql = $@"
318+
INSERT INTO {ProgressTable} (tablename, lastprocessedkey, lastcompletedutc, rowsscanned, rowsfixed, updatedonutc)
319+
VALUES (@TableName, @LastProcessedKey, @LastCompletedUtc, @RowsScanned, @RowsFixed, @UpdatedOnUtc)
320+
ON CONFLICT (tablename) DO UPDATE SET
321+
lastprocessedkey = EXCLUDED.lastprocessedkey,
322+
lastcompletedutc = EXCLUDED.lastcompletedutc,
323+
rowsscanned = EXCLUDED.rowsscanned,
324+
rowsfixed = EXCLUDED.rowsfixed,
325+
updatedonutc = EXCLUDED.updatedonutc";
326+
}
327+
else
328+
{
329+
// UPDLOCK + SERIALIZABLE takes a key-range lock so the row-absent case serializes:
330+
// one run inserts, the other blocks then updates. No MERGE (its upsert races are well known).
331+
upsertSql = $@"
332+
SET XACT_ABORT ON;
333+
BEGIN TRANSACTION;
334+
UPDATE {ProgressTable} WITH (UPDLOCK, SERIALIZABLE)
335+
SET lastprocessedkey = @LastProcessedKey, lastcompletedutc = @LastCompletedUtc,
336+
rowsscanned = @RowsScanned, rowsfixed = @RowsFixed, updatedonutc = @UpdatedOnUtc
337+
WHERE tablename = @TableName;
338+
IF @@ROWCOUNT = 0
339+
INSERT INTO {ProgressTable} (tablename, lastprocessedkey, lastcompletedutc, rowsscanned, rowsfixed, updatedonutc)
340+
VALUES (@TableName, @LastProcessedKey, @LastCompletedUtc, @RowsScanned, @RowsFixed, @UpdatedOnUtc);
341+
COMMIT TRANSACTION;";
342+
}
301343

302344
using (DbConnection conn = _connectionProvider.Create())
303345
{
304346
await conn.OpenAsync(cancellationToken);
305347

306-
var affected = await conn.ExecuteAsync(new CommandDefinition(updateSql, progress, cancellationToken: cancellationToken));
307-
308-
if (affected == 0)
309-
await conn.ExecuteAsync(new CommandDefinition(insertSql, progress, cancellationToken: cancellationToken));
348+
await conn.ExecuteAsync(new CommandDefinition(upsertSql, progress, cancellationToken: cancellationToken));
310349
}
311350
}
312351
catch (Exception ex)
@@ -316,11 +355,72 @@ public async Task SaveProgressAsync(Utf8CleanupProgress progress, CancellationTo
316355
}
317356
}
318357

358+
/// <summary>
359+
/// Maps a primary key column's declared type to the pageable category used for keyset cursors,
360+
/// or null when the type cannot be paged with a string cursor (e.g. date/decimal PKs).
361+
/// </summary>
362+
private static Utf8PrimaryKeyType? ClassifyKeyType(MetaColumn pk)
363+
{
364+
var udtName = (pk.UdtName ?? string.Empty).ToLowerInvariant();
365+
if (udtName == "citext")
366+
return Utf8PrimaryKeyType.Text;
367+
368+
var dataType = (pk.DataType ?? string.Empty).ToLowerInvariant();
369+
switch (dataType)
370+
{
371+
case "char":
372+
case "varchar":
373+
case "nchar":
374+
case "nvarchar":
375+
case "text":
376+
case "ntext":
377+
case "character":
378+
case "character varying":
379+
return Utf8PrimaryKeyType.Text;
380+
381+
case "tinyint":
382+
case "smallint":
383+
case "int":
384+
case "integer":
385+
case "bigint":
386+
return Utf8PrimaryKeyType.Integer;
387+
388+
case "uniqueidentifier":
389+
case "uuid":
390+
return Utf8PrimaryKeyType.Guid;
391+
392+
default:
393+
return null; // unsupported PK type -> table is skipped
394+
}
395+
}
396+
397+
/// <summary>
398+
/// Re-binds a string cursor to the primary key's native CLR type so the keyset <c>&gt;</c> / <c>=</c>
399+
/// predicates compare like types (PostgreSQL has no implicit text-to-integer/uuid coercion).
400+
/// </summary>
401+
private static object ConvertKey(string key, Utf8PrimaryKeyType keyType)
402+
{
403+
if (string.IsNullOrEmpty(key))
404+
return key;
405+
406+
switch (keyType)
407+
{
408+
case Utf8PrimaryKeyType.Integer:
409+
return long.Parse(key, CultureInfo.InvariantCulture);
410+
case Utf8PrimaryKeyType.Guid:
411+
return Guid.Parse(key);
412+
default:
413+
return key;
414+
}
415+
}
416+
319417
private class MetaColumn
320418
{
321419
public string TableSchema { get; set; }
322420
public string TableName { get; set; }
323421
public string ColumnName { get; set; }
422+
public string DataType { get; set; }
423+
public string UdtName { get; set; }
324424
}
325425
}
326426
}

Tools/Resgrid.Console/Commands/CleanUtf8Command.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ public async Task<ExitCode> ExecuteMainAsync(string[] args, CancellationToken ca
3838
}
3939
catch (Exception ex)
4040
{
41-
logger.LogError("There was an error running the UTF-8 data cleanup, see the error output below:");
42-
logger.LogError(ex.ToString());
41+
Resgrid.Framework.Logging.LogException(ex, "There was an error running the UTF-8 data cleanup");
4342
return ExitCode.Failed;
4443
}
4544

Workers/Resgrid.Workers.Console/Tasks/Utf8CleanupTask.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public class Utf8CleanupTask : IQuidjiboHandler<Utf8CleanupCommand>
1313
{
1414
public string Name => "UTF-8 Data Cleanup";
1515
public int Priority => 1;
16-
public ILogger _logger;
16+
private readonly ILogger _logger;
1717

1818
public Utf8CleanupTask(ILogger logger)
1919
{

0 commit comments

Comments
 (0)