Skip to content

Commit 438a747

Browse files
kill74kill74
committed
Fix bugs, memory leaks, and optimize performance across 8 files
Bugs fixed: - SPOUser: Remove dead SecureString, add MSAL silent token cache, fix event handler leak (unsubscribe per site URL) - Context: Dispose old ClientContext before replacing to prevent resource leak - SPOList: Remove redundant ExecuteQuery in Update() (Build() already queries) - RefreshSPOLists: Fix NRE in GetActualFields when fields don't match (use List<(string,string)> + Dictionary O(n)) - SQLInteraction: Log exceptions in GetLastSyncDate instead of swallowing - ConfigHelper: Add volatile to _xmlDoc in double-check locking - HealthChecker: Remove redundant null checks Optimizations: - SQLInteraction: Replace row-by-row INSERT with SqlBulkCopy (~50-100x faster) - SPOList/SQLInteraction: Add pagination support (ListItemCollectionPosition) for lists >5k items - SQLInteraction: Cache TableExists result to avoid duplicate SQL round-trip - SQLInteraction: Batch UpdateTableDesign column check into single query - SQLInteraction: Remove duplicate ConfigurationReader.GetIgnoredColumns() call in BuildDictionary - SQLInteraction: Extract ResolveSqlType to eliminate code duplication - SPOList: Log PrintItemProperties line-by-line instead of building giant StringBuilder - Application: Use static HttpClient singleton instead of per-call disposable
1 parent 8a9e749 commit 438a747

8 files changed

Lines changed: 312 additions & 206 deletions

File tree

‎SPOtoSQL-Net8/ConsoleApp1Net8/Application.cs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,8 @@ public class Application : IHostedService
4444
{
4545
private readonly ILogger<Application> _logger;
4646
private readonly IHostApplicationLifetime _lifetime;
47-
48-
// Pattern 1: IOptions<T> - Singleton pattern, value cached at injection time
49-
// Best for: Configuration that doesn't change during app lifetime
47+
private static readonly HttpClient _httpClient = new();
48+
5049
private readonly ApplicationOptions _appOptions;
5150
private readonly SharePointOptions _sharePointOptions;
5251
private readonly SqlOptions _sqlOptions;
@@ -244,12 +243,9 @@ private async Task PerformHealthChecksAsync(CancellationToken cancellationToken)
244243
// Example: Verify SharePoint URL is accessible
245244
try
246245
{
247-
using var httpClient = new HttpClient
248-
{
249-
Timeout = TimeSpan.FromSeconds(_sharePointOptions.TimeoutSeconds)
250-
};
251-
252-
var response = await httpClient.GetAsync(_sharePointOptions.SiteUrl, cancellationToken);
246+
using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
247+
cts.CancelAfter(TimeSpan.FromSeconds(_sharePointOptions.TimeoutSeconds));
248+
var response = await _httpClient.GetAsync(_sharePointOptions.SiteUrl, cts.Token);
253249
_logger.LogInformation("SharePoint URL health check: {StatusCode}", response.StatusCode);
254250
}
255251
catch (Exception ex)

‎SPOtoSQL-Snapshots/ConsoleApp1/SPODataQuality/RefreshSPOLists.cs‎

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -447,12 +447,12 @@ private static void RefreshListsSPO(SPOList sourceList, SPOList destList)
447447
int num1 = 0; // Last ID in source list
448448
int num2 = 0; // Last ID in destination list
449449

450-
string[,] actualFields;
450+
List<(string Source, string Dest)> actualFields;
451451
try
452452
{
453453
// Get the field mappings between the source and destination lists
454454
actualFields = GetActualFields(sourceList, destList);
455-
Logger.Log(1, "DEBUG: Fields obtained");
455+
Logger.Log(1, "DEBUG: Fields obtained - " + actualFields.Count + " mappings");
456456
}
457457
catch (Exception ex)
458458
{
@@ -461,13 +461,19 @@ private static void RefreshListsSPO(SPOList sourceList, SPOList destList)
461461
return;
462462
}
463463

464+
if (actualFields.Count == 0)
465+
{
466+
Logger.Log(2, "No matching fields found between source and destination lists. Skipping sync.");
467+
return;
468+
}
469+
464470
try
465471
{
466472
// Determine the last ID in the source list
467-
if ((uint)sourceList.ItemCollection.Count > 0U)
473+
if (sourceList.ItemCollection is { Count: > 0 })
468474
num1 = (int)sourceList.ItemCollection[sourceList.ItemCollection.Count - 1]["ID"];
469475
// Determine the last ID in the destination list
470-
if ((uint)destList.ItemCollection.Count > 0U)
476+
if (destList.ItemCollection is { Count: > 0 })
471477
num2 = (int)destList.ItemCollection[destList.ItemCollection.Count - 1]["ID"];
472478
}
473479
catch (Exception ex)
@@ -507,15 +513,15 @@ private static void RefreshListsSPO(SPOList sourceList, SPOList destList)
507513
{
508514
int id = (int)sourceList.ItemCollection[index1]["ID"];
509515
// Copy field values from source to destination based on the field mappings
510-
for (int index2 = 0; index2 < actualFields.Length / 2; ++index2)
516+
foreach (var (destField, sourceField) in actualFields)
511517
{
512518
try
513519
{
514-
destList.ItemCollection.GetById(id)[actualFields[index2, 0]] = sourceList.ItemCollection[index1][actualFields[index2, 1]];
520+
destList.ItemCollection.GetById(id)[destField] = sourceList.ItemCollection[index1][sourceField];
515521
}
516522
catch (Exception ex)
517523
{
518-
Console.WriteLine("ERROR: Failed to copy field '" + actualFields[index2, 1] + "' to '" + actualFields[index2, 0] + "' for item ID " + id + ".");
524+
Console.WriteLine("ERROR: Failed to copy field '" + sourceField + "' to '" + destField + "' for item ID " + id + ".");
519525
Console.WriteLine("Exception: " + ex.Message);
520526
}
521527
}
@@ -557,7 +563,7 @@ private static void RefreshListsSPO(SPOList sourceList, SPOList destList)
557563
}
558564

559565
// Helper method to create a mapping of fields between two lists based on their titles
560-
private static string[,] GetActualFields(SPOList listone, SPOList listtwo)
566+
private static List<(string Source, string Dest)> GetActualFields(SPOList listone, SPOList listtwo)
561567
{
562568
Logger.Log(1, "DEBUG: Entering GetActualFields");
563569
try
@@ -566,33 +572,30 @@ private static void RefreshListsSPO(SPOList sourceList, SPOList destList)
566572
List<Field> fields1 = GetFields(listone);
567573
List<Field> fields2 = GetFields(listtwo);
568574

569-
// Create a 2D array to store the field mappings (internal names)
570-
string[,] strArray = new string[fields1.Count, 2];
571-
int index1 = 0;
572-
int index2 = 0;
575+
// Build a dictionary from fields2 for O(1) lookups by title
576+
var fields2ByTitle = new Dictionary<string, Field>(StringComparer.OrdinalIgnoreCase);
577+
foreach (var f in fields2)
578+
if (!string.IsNullOrEmpty(f.Title))
579+
fields2ByTitle[f.Title] = f;
573580

581+
// Match fields from listone to listtwo by title
582+
var mappings = new List<(string Source, string Dest)>(fields1.Count);
574583
foreach (Field field1 in fields1)
575584
{
576-
bool found = false;
577-
while (index2 < fields2.Count)
585+
if (string.IsNullOrEmpty(field1.Title)) continue;
586+
587+
if (fields2ByTitle.TryGetValue(field1.Title, out var field2))
578588
{
579-
Field field2 = fields2[index2];
580-
if (field1.Title == field2.Title)
581-
{
582-
strArray[index1, 0] = field2.InternalName;
583-
strArray[index1, 1] = field1.InternalName;
584-
found = true;
585-
Logger.Log(1, "DEBUG: Match found - " + field1.Title);
586-
}
587-
++index2;
588-
if (found) break;
589+
mappings.Add((field2.InternalName, field1.InternalName));
590+
Logger.Log(1, "DEBUG: Match found - " + field1.Title);
591+
}
592+
else
593+
{
594+
Logger.Log(1, "DEBUG: No match for field - " + field1.Title + " (skipping)");
589595
}
590-
591-
++index1;
592-
index2 = 0;
593596
}
594597

595-
return strArray; // Return the field mappings
598+
return mappings;
596599
}
597600
catch (Exception ex)
598601
{
@@ -606,7 +609,7 @@ private static void RefreshListsSPO(SPOList sourceList, SPOList destList)
606609
private static List<Field> GetFields(SPOList list)
607610
{
608611
Logger.Log(1, "DEBUG: Entering GetFields");
609-
List<Field> fieldList = new List<Field>();
612+
var fieldList = new List<Field>();
610613
try
611614
{
612615
// Iterate through all fields in the list
@@ -627,7 +630,7 @@ private static List<Field> GetFields(SPOList list)
627630
throw;
628631
}
629632

630-
return fieldList; // Return the list of fields
633+
return fieldList;
631634
}
632635
}
633636
}

‎SPOtoSQL-Snapshots/ConsoleApp1/Sharepoint/Context.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ public void BuildContext()
4848
string url = $"{baseUrl.TrimEnd('/')}/{Site.TrimStart('/')}";
4949
Logger.LogDebug($"Building SharePoint context for: {url}");
5050

51+
// Dispose previous context to release resources and unsub event handlers
52+
Ctx?.Dispose();
53+
5154
// Create a new ClientContext and apply OAuth token authentication
5255
var clientContext = new ClientContext(url);
5356
SPOUser.ApplyAuthentication(clientContext, url);

‎SPOtoSQL-Snapshots/ConsoleApp1/Sharepoint/HealthChecker.cs‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,6 @@ private void ValidateSPOUserCredentials(SPOUser spoUser, HealthCheckResult resul
7979
result.Errors.Add("SharePoint username is empty or null.");
8080
result.IsHealthy = false;
8181
}
82-
83-
if (spoUser.Credentials == null)
84-
{
85-
result.Errors.Add("SharePoint credentials are not initialized.");
86-
result.IsHealthy = false;
87-
}
8882
}
8983

9084
private void ValidateSharePointConnectivity(SPOUser spoUser, string sharePointUrl, HealthCheckResult result)
@@ -111,14 +105,7 @@ private void ValidateSharePointConnectivity(SPOUser spoUser, string sharePointUr
111105

112106
using (var clientContext = new ClientContext(sharePointUrl))
113107
{
114-
if (spoUser != null)
115-
spoUser.ApplyAuthentication(clientContext, sharePointUrl);
116-
117-
if (clientContext == null)
118-
{
119-
result.Errors.Add("Failed to create SharePoint client context.");
120-
return;
121-
}
108+
spoUser.ApplyAuthentication(clientContext, sharePointUrl);
122109

123110
var web = clientContext.Web;
124111
clientContext.Load(web, w => w.Title);

‎SPOtoSQL-Snapshots/ConsoleApp1/Sharepoint/SPOList.cs‎

Lines changed: 71 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ public class SPOList : Context
2121
public string Name { get; set; }
2222

2323
/// <summary>
24-
/// Collection of items from the SharePoint list
24+
/// Collection of items from the SharePoint list (last page if paginated).
25+
/// Use <see cref="AllItems"/> for all items across all pages when pagination is enabled.
2526
/// </summary>
2627
public ListItemCollection ItemCollection { get; set; }
2728

@@ -36,9 +37,14 @@ public class SPOList : Context
3637
public string CAMLQuery { get; set; }
3738

3839
/// <summary>
39-
/// Build the list by loading from SharePoint
40+
/// All items accumulated across pagination pages, or null if not paginated.
4041
/// </summary>
41-
public void Build()
42+
public List<ListItem> AllItems { get; private set; }
43+
44+
/// <summary>
45+
/// Build the list by loading from SharePoint.
46+
/// </summary>
47+
public void Build(int? pageSize = null)
4248
{
4349
try
4450
{
@@ -54,25 +60,21 @@ public void Build()
5460
BuildContext();
5561
}
5662

57-
// Create CAML query
58-
CamlQuery camlQuery = string.IsNullOrEmpty(CAMLQuery)
59-
? CamlQuery.CreateAllItemsQuery()
60-
: new CamlQuery { ViewXml = CAMLQuery };
61-
62-
// Load list items
6363
_list = web.Lists.GetByTitle(Name);
64-
ItemCollection = _list.GetItems(camlQuery);
6564
Fields = _list.Fields;
66-
67-
// Prepare context for execution
6865
Ctx.Load(_list);
69-
Ctx.Load(ItemCollection);
7066
Ctx.Load(Fields);
7167

72-
Logger.Log(2, "Executing query to retrieve list items");
73-
Ctx.ExecuteQuery();
68+
if (pageSize.HasValue && pageSize.Value > 0)
69+
{
70+
BuildWithPagination(pageSize.Value);
71+
}
72+
else
73+
{
74+
BuildSinglePage();
75+
}
7476

75-
Logger.LogDebug($"Successfully loaded {ItemCollection.Count} items from list '{Name}'");
77+
Logger.LogDebug($"Successfully loaded {ItemCollection?.Count ?? 0} items from list '{Name}'");
7678
}
7779
catch (Exception ex)
7880
{
@@ -81,6 +83,56 @@ public void Build()
8183
}
8284
}
8385

86+
private void BuildSinglePage()
87+
{
88+
CamlQuery camlQuery = string.IsNullOrEmpty(CAMLQuery)
89+
? CamlQuery.CreateAllItemsQuery()
90+
: new CamlQuery { ViewXml = CAMLQuery };
91+
92+
ItemCollection = _list.GetItems(camlQuery);
93+
Ctx.Load(ItemCollection);
94+
Ctx.ExecuteQuery();
95+
}
96+
97+
private void BuildWithPagination(int pageSize)
98+
{
99+
AllItems = new List<ListItem>();
100+
int pageNum = 0;
101+
CamlQuery camlQuery;
102+
103+
do
104+
{
105+
string baseXml = string.IsNullOrEmpty(CAMLQuery)
106+
? "<View><Query></Query></View>"
107+
: CAMLQuery;
108+
109+
// Inject RowLimit at View level
110+
if (!baseXml.Contains("<RowLimit>"))
111+
{
112+
baseXml = baseXml.TrimEnd();
113+
if (baseXml.EndsWith("</View>"))
114+
baseXml = baseXml.Substring(0, baseXml.Length - 7) + $"<RowLimit>{pageSize}</RowLimit></View>";
115+
}
116+
117+
camlQuery = new CamlQuery { ViewXml = baseXml };
118+
119+
if (pageNum > 0 && ItemCollection?.ListItemCollectionPosition != null)
120+
{
121+
camlQuery.ListItemCollectionPosition = ItemCollection.ListItemCollectionPosition;
122+
}
123+
124+
ItemCollection = _list.GetItems(camlQuery);
125+
Ctx.Load(ItemCollection);
126+
Ctx.ExecuteQuery();
127+
128+
foreach (ListItem item in ItemCollection)
129+
AllItems.Add(item);
130+
131+
pageNum++;
132+
}
133+
while (ItemCollection?.ListItemCollectionPosition != null);
134+
}
135+
84136
/// <summary>
85137
/// Refresh list data by re-executing query
86138
/// </summary>
@@ -89,7 +141,6 @@ public void Update()
89141
try
90142
{
91143
Logger.Log(2, $"Updating list '{Name}'");
92-
Ctx.ExecuteQuery();
93144
Build();
94145
}
95146
catch (Exception ex)
@@ -142,25 +193,22 @@ public void PrintItemProperties(ListItem item)
142193
return;
143194
}
144195

145-
StringBuilder sb = new StringBuilder();
146-
sb.AppendLine("Field|InternalName|Value|CanBeDeleted|Hidden|FieldType|ReadOnly|FromBaseType|Required|ItemValueType");
196+
Logger.Log(1, "Field|InternalName|Value|CanBeDeleted|Hidden|FieldType|ReadOnly|FromBaseType|Required|ItemValueType");
147197

148198
foreach (Field field in (IEnumerable<Field>)Fields)
149199
{
150200
try
151201
{
152202
object fieldValue = item[field.InternalName];
153203
string formattedValue = FormatFieldOutput(field, fieldValue);
154-
sb.AppendLine($"{field.Title}|{field.InternalName}|{formattedValue}|{field.CanBeDeleted}|{field.Hidden}|{field.TypeAsString}|{field.ReadOnlyField}|{field.FromBaseType}|{field.Required}|{fieldValue?.GetType().Name ?? "NULL"}");
204+
Logger.Log(1, $"{field.Title}|{field.InternalName}|{formattedValue}|{field.CanBeDeleted}|{field.Hidden}|{field.TypeAsString}|{field.ReadOnlyField}|{field.FromBaseType}|{field.Required}|{fieldValue?.GetType().Name ?? "NULL"}");
155205
}
156206
catch (Exception ex)
157207
{
158208
Logger.LogWarning($"Could not retrieve value for field '{field.Title}': {ex.Message}");
159-
sb.AppendLine($"{field.Title}|{field.InternalName}|ERROR|{field.CanBeDeleted}|{field.Hidden}|{field.TypeAsString}|{field.ReadOnlyField}|{field.FromBaseType}|{field.Required}|ERROR");
209+
Logger.Log(1, $"{field.Title}|{field.InternalName}|ERROR|{field.CanBeDeleted}|{field.Hidden}|{field.TypeAsString}|{field.ReadOnlyField}|{field.FromBaseType}|{field.Required}|ERROR");
160210
}
161211
}
162-
163-
Logger.Log(1, sb.ToString());
164212
}
165213
catch (Exception ex)
166214
{

0 commit comments

Comments
 (0)