Skip to content

Commit 5517013

Browse files
Roopan-MicrosoftRoopan-Microsoft
authored
Merge pull request #584 from microsoft/PSL-US-38849
fix: Improved and fixed code quality issues
2 parents ce71d54 + 6673eee commit 5517013

42 files changed

Lines changed: 615 additions & 815 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎App/backend-api/Microsoft.GS.DPS.Host/API/ChatHost/Chat.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public static void AddAPIs(WebApplication app)
1717
ChatRequestValidator validator,
1818
ChatHost chatHost) =>
1919
{
20-
if(validator.Validate(request).IsValid == false)
20+
if (!validator.Validate(request).IsValid)
2121
{
2222
return Results.BadRequest();
2323
}
@@ -37,7 +37,7 @@ public static void AddAPIs(WebApplication app)
3737
ChatRequestValidator validator,
3838
ChatHost chatHost) =>
3939
{
40-
if (validator.Validate(request).IsValid == false)
40+
if (!validator.Validate(request).IsValid)
4141
{
4242
return Results.BadRequest();
4343
}

‎App/backend-api/Microsoft.GS.DPS.Host/API/KernelMemory/KernelMemory.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,13 @@ DPS.API.KernelMemory kernelMemory
9393
await kernelMemory.DeleteDocument(documentId);
9494
return Results.Ok(new DocumentDeletedResult() { IsDeleted = true });
9595
}
96+
#pragma warning disable CA1031 // Must catch all to log and keep the process alive
9697
catch (Exception ex)
9798
{
99+
app.Logger.LogError(ex, "An error occurred while deleting a document.");
98100
return Results.BadRequest(new DocumentDeletedResult() { IsDeleted = false });
99101
}
102+
#pragma warning restore CA1031
100103
})
101104
.DisableAntiforgery();
102105

‎App/backend-api/Microsoft.GS.DPS.Host/API/UserInterface/UserInterface.cs‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ public static void AddAPIs(WebApplication app)
3030
var document = await documentRepository.FindByDocumentIdAsync(DocumentId);
3131

3232
//Check if the thumbnail is already in the cache
33-
if (thumbnails.ContainsKey(document.MimeType))
33+
if (thumbnails.TryGetValue(document.MimeType, out var thumbnail))
3434
{
35-
return Results.File(thumbnails[document.MimeType], "image/png");
35+
return Results.File(thumbnail, "image/png");
3636
}
3737
else
3838
{
@@ -108,14 +108,7 @@ public static void AddAPIs(WebApplication app)
108108
{
109109
DPS.Storage.Document.Entities.Document result = await documents.GetDocument(DocumentId);
110110

111-
if (result == null)
112-
{
113-
return Results.NotFound();
114-
}
115-
else
116-
{
117-
return Results.Ok(result);
118-
}
111+
return result == null ? Results.NotFound() : Results.Ok(result);
119112
}
120113
)
121114
.DisableAntiforgery();

‎App/backend-api/Microsoft.GS.DPS.Host/DependencyConfiguration/ServiceDependencies.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public static void Inject(IHostApplicationBuilder builder)
2828
.AddSingleton<Microsoft.GS.DPS.API.UserInterface.DataCacheManager>()
2929
.AddSingleton<Microsoft.SemanticKernel.Kernel>(x =>
3030
{
31-
var aiService = x.GetRequiredService<IOptions<AIServices>>().Value;
3231
return Kernel.CreateBuilder()
3332
.AddAzureOpenAIChatCompletion(deploymentName: builder.Configuration.GetSection("Application:AIServices:GPT-4o-mini")["ModelName"] ?? "",
3433
endpoint: builder.Configuration.GetSection("Application:AIServices:GPT-4o-mini")["Endpoint"] ?? "",

‎App/backend-api/Microsoft.GS.DPS.Host/Helpers/AzureCredentialHelper.cs‎

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,28 @@
1-
using System;
2-
using System.Threading.Tasks;
3-
using Azure.Core;
4-
using Azure.Identity;
5-
6-
namespace Microsoft.GS.DPSHost.Helpers
7-
{
8-
/// <summary>
9-
/// The Azure Credential Helper class
10-
/// </summary>
11-
public static class AzureCredentialHelper
12-
{
13-
/// <summary>
14-
/// Get the Azure Credentials based on the environment type
15-
/// </summary>
16-
/// <param name="clientId">The client Id in case of User assigned Managed identity</param>
17-
/// <returns>The Credential Object</returns>
18-
public static TokenCredential GetAzureCredential(string? clientId = null)
19-
{
20-
var env = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Production";
21-
22-
if (string.Equals(env, "Development", StringComparison.OrdinalIgnoreCase))
23-
{
24-
return new DefaultAzureCredential(); // CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in development
25-
}
26-
else
27-
{
28-
return clientId != null
29-
? new ManagedIdentityCredential(clientId)
30-
: new ManagedIdentityCredential();
31-
}
32-
}
33-
}
1+
using System;
2+
using System.Threading.Tasks;
3+
using Azure.Core;
4+
using Azure.Identity;
5+
6+
namespace Microsoft.GS.DPSHost.Helpers
7+
{
8+
/// <summary>
9+
/// The Azure Credential Helper class
10+
/// </summary>
11+
public static class AzureCredentialHelper
12+
{
13+
/// <summary>
14+
/// Get the Azure Credentials based on the environment type
15+
/// </summary>
16+
/// <param name="clientId">The client Id in case of User assigned Managed identity</param>
17+
/// <returns>The Credential Object</returns>
18+
public static TokenCredential GetAzureCredential(string? clientId = null)
19+
{
20+
var env = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Production";
21+
22+
// CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in development
23+
return string.Equals(env, "Development", StringComparison.OrdinalIgnoreCase)
24+
? new DefaultAzureCredential() // CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in development
25+
: (clientId != null ? new ManagedIdentityCredential(clientId) : new ManagedIdentityCredential());
26+
}
27+
}
3428
}

‎App/backend-api/Microsoft.GS.DPS/API/ChatHost/ChatHost.cs‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ static ChatHost()
4949
var assemblyLocation = Assembly.GetExecutingAssembly().Location;
5050
var assemblyDirectory = System.IO.Path.GetDirectoryName(assemblyLocation);
5151
// binding assembly directory with file path (Prompts/Chat_SystemPrompt.txt)
52-
var systemPromptFilePath = System.IO.Path.Combine(assemblyDirectory, "Prompts/Chat_SystemPrompt.txt");
52+
var systemPromptFilePath = System.IO.Path.Combine(assemblyDirectory, "Prompts", "Chat_SystemPrompt.txt");
5353
ChatHost.s_systemPrompt = System.IO.File.ReadAllText(systemPromptFilePath);
5454
ChatHost.s_assistancePrompt =
5555
@"
@@ -74,15 +74,7 @@ Please feel free to ask me any questions related to those documents and contents
7474

7575
private async Task<ChatSession> makeNewSession(string? chatSessionId)
7676
{
77-
var sessionId = string.Empty;
78-
if(string.IsNullOrEmpty(chatSessionId))
79-
{
80-
sessionId = Guid.NewGuid().ToString();
81-
}
82-
else
83-
{
84-
sessionId = chatSessionId;
85-
}
77+
var sessionId = string.IsNullOrEmpty(chatSessionId) ? Guid.NewGuid().ToString() : chatSessionId;
8678

8779
//Create New Chat History
8880
this.chatHistory = new ChatHistory();
@@ -92,7 +84,7 @@ private async Task<ChatSession> makeNewSession(string? chatSessionId)
9284
//Create a new ChatSession Entity for Saving into Azure Cosmos
9385
return new ChatSession()
9486
{
95-
SessionId = sessionId, // New Session ID
87+
SessionId = this.sessionId, // New Session ID
9688
StartTime = DateTime.UtcNow // Session Created Time
9789
};
9890

‎App/backend-api/Microsoft.GS.DPS/API/KernelMemory/KernelMemory.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ static KernelMemory()
3636
var assemblyLocation = Assembly.GetExecutingAssembly().Location;
3737
var assemblyDirectory = System.IO.Path.GetDirectoryName(assemblyLocation);
3838
// binding assembly directory with file path (Prompts/KeywordExtract_SystemPrompt.txt)
39-
var systemPromptFilePath = System.IO.Path.Combine(assemblyDirectory, "Prompts/KeywordExtract_SystemPrompt.txt");
39+
var systemPromptFilePath = System.IO.Path.Combine(assemblyDirectory, "Prompts", "KeywordExtract_SystemPrompt.txt");
4040
KernelMemory.keywordExtractorPrompt = System.IO.File.ReadAllText(systemPromptFilePath);
4141
}
4242

@@ -197,7 +197,7 @@ private async Task<string> getSummary(string documentId, string fileName)
197197

198198
return keywordDict;
199199
}
200-
catch (Exception ex)
200+
catch (Exception)
201201
{
202202
return new Dictionary<string, string>();
203203
}

‎App/backend-api/Microsoft.GS.DPS/API/UserInterface/DataCacheManager.cs‎

Lines changed: 83 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,83 @@
1-
using System;
2-
using System.Collections.Generic;
3-
using System.Linq;
4-
using System.Threading.Tasks;
5-
using System.Timers;
6-
using Microsoft.GS.DPS.Storage.Document;
7-
using Timers =System.Timers;
8-
9-
namespace Microsoft.GS.DPS.API.UserInterface
10-
{
11-
public class DataCacheManager
12-
{
13-
private readonly DocumentRepository _documentRepository;
14-
private Dictionary<string, List<string>> _keywordCache;
15-
private readonly Timers.Timer _cacheTimer;
16-
private readonly object _cacheLock = new object();
17-
18-
public DataCacheManager(DocumentRepository documentRepository)
19-
{
20-
_documentRepository = documentRepository;
21-
_keywordCache = new Dictionary<string, List<string>>();
22-
_cacheTimer = new Timers.Timer(5 * 60 * 1000); // 5 minutes
23-
_cacheTimer.Elapsed += async (sender, e) => await RefreshCacheAsync();
24-
_cacheTimer.Start();
25-
}
26-
27-
public async Task<Dictionary<string, List<string>>> GetConsolidatedKeywordsAsync()
28-
{
29-
if (_keywordCache.Count == 0)
30-
{
31-
await RefreshCacheAsync();
32-
}
33-
34-
lock (_cacheLock)
35-
{
36-
return new Dictionary<string, List<string>>(_keywordCache);
37-
}
38-
}
39-
40-
public async Task RefreshCacheAsync()
41-
{
42-
var consolidatedKeywords = new Dictionary<string, List<string>>();
43-
var documents = await _documentRepository.GetAllDocuments();
44-
45-
foreach (var document in documents)
46-
{
47-
if (document.Keywords != null)
48-
{
49-
foreach (var keywordDict in document.Keywords)
50-
{
51-
if (!consolidatedKeywords.ContainsKey(keywordDict.Key))
52-
{
53-
consolidatedKeywords[keywordDict.Key] = new List<string>();
54-
}
55-
56-
var values = keywordDict.Value.Split(',').Select(v => v.Trim()).ToArray();
57-
58-
foreach (var value in values)
59-
{
60-
if (!consolidatedKeywords[keywordDict.Key].Contains(value))
61-
{
62-
consolidatedKeywords[keywordDict.Key].Add(value);
63-
}
64-
}
65-
66-
consolidatedKeywords[keywordDict.Key] = consolidatedKeywords[keywordDict.Key].OrderBy(v => v).ToList();
67-
}
68-
}
69-
}
70-
71-
consolidatedKeywords = consolidatedKeywords.OrderBy(k => k.Key).ToDictionary(k => k.Key, v => v.Value);
72-
73-
lock (_cacheLock)
74-
{
75-
_keywordCache = consolidatedKeywords;
76-
}
77-
}
78-
79-
public void ManualRefresh()
80-
{
81-
_cacheTimer.Stop();
82-
_cacheTimer.Start();
83-
Task.Run(async () => await RefreshCacheAsync());
84-
}
85-
}
86-
}
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Threading.Tasks;
5+
using System.Timers;
6+
using Microsoft.GS.DPS.Storage.Document;
7+
using Timers =System.Timers;
8+
9+
namespace Microsoft.GS.DPS.API.UserInterface
10+
{
11+
public class DataCacheManager
12+
{
13+
private readonly DocumentRepository _documentRepository;
14+
private Dictionary<string, List<string>> _keywordCache;
15+
private readonly Timers.Timer _cacheTimer;
16+
private readonly object _cacheLock = new object();
17+
18+
public DataCacheManager(DocumentRepository documentRepository)
19+
{
20+
_documentRepository = documentRepository;
21+
_keywordCache = new Dictionary<string, List<string>>();
22+
_cacheTimer = new Timers.Timer(5 * 60 * 1000); // 5 minutes
23+
_cacheTimer.Elapsed += async (sender, e) => await RefreshCacheAsync();
24+
_cacheTimer.Start();
25+
}
26+
27+
public async Task<Dictionary<string, List<string>>> GetConsolidatedKeywordsAsync()
28+
{
29+
if (_keywordCache.Count == 0)
30+
{
31+
await RefreshCacheAsync();
32+
}
33+
34+
lock (_cacheLock)
35+
{
36+
return new Dictionary<string, List<string>>(_keywordCache);
37+
}
38+
}
39+
40+
public async Task RefreshCacheAsync()
41+
{
42+
var consolidatedKeywords = new Dictionary<string, List<string>>();
43+
var documents = await _documentRepository.GetAllDocuments();
44+
45+
foreach (var document in documents.Where(d => d.Keywords != null))
46+
{
47+
foreach (var keywordDict in document.Keywords)
48+
{
49+
if (!consolidatedKeywords.ContainsKey(keywordDict.Key))
50+
{
51+
consolidatedKeywords[keywordDict.Key] = new List<string>();
52+
}
53+
54+
var values = keywordDict.Value.Split(',').Select(v => v.Trim()).ToArray();
55+
56+
foreach (var value in values)
57+
{
58+
if (!consolidatedKeywords[keywordDict.Key].Contains(value))
59+
{
60+
consolidatedKeywords[keywordDict.Key].Add(value);
61+
}
62+
}
63+
64+
consolidatedKeywords[keywordDict.Key] = consolidatedKeywords[keywordDict.Key].OrderBy(v => v).ToList();
65+
}
66+
}
67+
68+
consolidatedKeywords = consolidatedKeywords.OrderBy(k => k.Key).ToDictionary(k => k.Key, v => v.Value);
69+
70+
lock (_cacheLock)
71+
{
72+
_keywordCache = consolidatedKeywords;
73+
}
74+
}
75+
76+
public void ManualRefresh()
77+
{
78+
_cacheTimer.Stop();
79+
_cacheTimer.Start();
80+
Task.Run(async () => await RefreshCacheAsync());
81+
}
82+
}
83+
}

0 commit comments

Comments
 (0)