Skip to content

Commit 700f78e

Browse files
authored
Merge pull request #4 from willvelida/feature/mcp-best-practices
Implementing base tool class and refactoring existing tools
2 parents 43828ed + cdd7904 commit 700f78e

31 files changed

Lines changed: 5520 additions & 1048 deletions

.github/workflows/deployMcpServer.yml

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,70 @@ permissions:
1010
id-token: write
1111
pull-requests: write
1212

13+
env:
14+
DOTNET_VERSION: 9.0.x
15+
COVERAGE_PATH: ${{ github.workspace }}/coverage
16+
1317
jobs:
18+
env-setup:
19+
name: Setup Environment
20+
runs-on: ubuntu-latest
21+
outputs:
22+
dotnet-version: ${{ steps.set-output-defaults.outputs.dotnet-version }}
23+
coverage-path: ${{ steps.set-output-defaults.outputs.coverage-path }}
24+
steps:
25+
- name: set outputs with default values
26+
id: set-output-defaults
27+
run: |
28+
echo "dotnet-version=${{ env.DOTNET_VERSION }}" >> "$GITHUB_OUTPUT"
29+
echo "coverage-path=${{ env.COVERAGE_PATH }}" >> "$GITHUB_OUTPUT"
30+
31+
test-and-coverage:
32+
name: Run Tests and Generate Coverage
33+
needs: env-setup
34+
runs-on: ubuntu-latest
35+
steps:
36+
- uses: actions/checkout@v4
37+
name: Checkout code
38+
39+
- name: Setup .NET
40+
uses: actions/setup-dotnet@v4
41+
with:
42+
dotnet-version: ${{ needs.env-setup.outputs.dotnet-version }}
43+
44+
- name: Restore dependencies
45+
run: dotnet restore src/mcp-afl-server.sln
46+
47+
- name: Build solution
48+
run: dotnet build src/mcp-afl-server.sln --configuration Release --no-restore
49+
50+
- name: Create coverage directory
51+
run: mkdir -p ${{ needs.env-setup.outputs.coverage-path }}
52+
53+
- name: Run unit tests
54+
run: dotnet test ./test/mcp-afl-server.UnitTests/mcp-afl-server.UnitTests.csproj --configuration Release --verbosity normal --collect:"XPlat Code Coverage" --logger trx --results-directory ${{ needs.env-setup.outputs.coverage-path }}
55+
56+
- name: Merge Code Coverage reports
57+
run: |
58+
dotnet tool install -g dotnet-reportgenerator-globaltool
59+
reportgenerator "-reports:${{ needs.env-setup.outputs.coverage-path }}/**/coverage.cobertura.xml" "-targetdir:${{ needs.env-setup.outputs.coverage-path }}" -reporttypes:Cobertura
60+
61+
- name: Code Coverage Report
62+
uses: irongut/CodeCoverageSummary@v1.3.0
63+
with:
64+
filename: coverage/Cobertura.xml
65+
badge: true
66+
fail_below_min: false
67+
format: markdown
68+
hide_branch_rate: false
69+
hide_complexity: true
70+
indicators: true
71+
output: both
72+
thresholds: '60 80'
73+
1474
build-container-image:
1575
name: Build Docker Image
76+
needs: test-and-coverage
1677
runs-on: ubuntu-latest
1778
outputs:
1879
loginServer: ${{ steps.getacrserver.outputs.loginServer }}
@@ -58,7 +119,7 @@ jobs:
58119
59120
lint-bicep:
60121
name: Run Bicep Linter
61-
needs: build-container-image
122+
needs: test-and-coverage
62123
runs-on: ubuntu-latest
63124
steps:
64125
- uses: actions/checkout@v4
@@ -152,4 +213,4 @@ jobs:
152213
parameters-file: './infra/parameters.deployMcpServer.bicepparam'
153214
parameters: '{"imageName": "${{ needs.build-container-image.outputs.loginServer}}/afl-mcp-server:${{ github.sha }}"}'
154215

155-
216+

src/Tools/BaseAFLTool.cs

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
using Microsoft.Extensions.Logging;
2+
using System.Collections;
3+
using System.Net.Http.Json;
4+
using System.Text.Json;
5+
6+
// Create a base class for all AFL tools
7+
public abstract class BaseAFLTool
8+
{
9+
protected readonly HttpClient _httpClient;
10+
protected readonly ILogger _logger;
11+
12+
protected BaseAFLTool(HttpClient httpClient, ILogger logger)
13+
{
14+
_httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));
15+
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
16+
}
17+
18+
/// <summary>
19+
/// Executes an API call with standardized error handling and logging
20+
/// </summary>
21+
/// <typeparam name="T">The type to deserialize the result to</typeparam>
22+
/// <param name="endpoint">The API endpoint to call</param>
23+
/// <param name="operationName">A descriptive name for logging purposes</param>
24+
/// <param name="propertyName">The JSON property name to extract from the response</param>
25+
/// <param name="additionalValidation">Optional additional validation function</param>
26+
/// <returns>The deserialized result or default(T) on error</returns>
27+
protected async Task<T> ExecuteApiCallAsync<T>(
28+
string endpoint,
29+
string operationName,
30+
string propertyName,
31+
Func<T, bool> additionalValidation = null) where T : class, new()
32+
{
33+
_logger.LogInformation("Fetching data for {Operation} - Endpoint: {Endpoint}", operationName, endpoint);
34+
35+
try
36+
{
37+
var response = await _httpClient.GetAsync(endpoint);
38+
39+
if (!response.IsSuccessStatusCode)
40+
{
41+
_logger.LogError("API request failed. StatusCode: {StatusCode}, Endpoint: {Endpoint}",
42+
response.StatusCode, endpoint);
43+
return new T();
44+
}
45+
46+
var jsonElement = await response.Content.ReadFromJsonAsync<JsonElement>();
47+
48+
if (!jsonElement.TryGetProperty(propertyName, out var property))
49+
{
50+
_logger.LogWarning("No '{PropertyName}' property found in API response for {Operation}",
51+
propertyName, operationName);
52+
return new T();
53+
}
54+
55+
var result = JsonSerializer.Deserialize<T>(property.GetRawText());
56+
57+
if (result == null)
58+
{
59+
_logger.LogWarning("Failed to deserialize response for {Operation}", operationName);
60+
return new T();
61+
}
62+
63+
// Check if result is a collection and log appropriately
64+
if (result is ICollection collection)
65+
{
66+
if (collection.Count == 0)
67+
{
68+
_logger.LogInformation("No data found for {Operation}", operationName);
69+
}
70+
else
71+
{
72+
_logger.LogInformation("Successfully retrieved {Count} items for {Operation}",
73+
collection.Count, operationName);
74+
}
75+
}
76+
else
77+
{
78+
_logger.LogInformation("Successfully retrieved data for {Operation}", operationName);
79+
}
80+
81+
// Run additional validation if provided
82+
if (additionalValidation != null && !additionalValidation(result))
83+
{
84+
_logger.LogWarning("Additional validation failed for {Operation}", operationName);
85+
return new T();
86+
}
87+
88+
return result;
89+
}
90+
catch (HttpRequestException ex)
91+
{
92+
_logger.LogError(ex, "Network error for {Operation}", operationName);
93+
return new T();
94+
}
95+
catch (TaskCanceledException ex)
96+
{
97+
_logger.LogError(ex, "Timeout for {Operation}", operationName);
98+
return new T();
99+
}
100+
catch (JsonException ex)
101+
{
102+
_logger.LogError(ex, "JSON parsing error for {Operation}", operationName);
103+
return new T();
104+
}
105+
catch (Exception ex)
106+
{
107+
_logger.LogError(ex, "Unexpected error for {Operation}", operationName);
108+
return new T();
109+
}
110+
}
111+
112+
/// <summary>
113+
/// Validates input parameters and logs warnings for invalid values
114+
/// </summary>
115+
protected bool ValidateParameters(params (string name, object value, Func<object, bool> validator, string errorMessage)[] validations)
116+
{
117+
foreach (var (name, value, validator, errorMessage) in validations)
118+
{
119+
if (!validator(value))
120+
{
121+
_logger.LogWarning("Invalid {ParameterName} parameter: {Value}. {ErrorMessage}", name, value, errorMessage);
122+
return false;
123+
}
124+
}
125+
return true;
126+
}
127+
128+
// Common validation methods
129+
protected static bool IsValidYear(int year) => year >= 1897 && year <= DateTime.Now.Year + 1;
130+
protected static bool IsValidRound(int round) => round >= 1 && round <= 30;
131+
protected static bool IsValidId(int id) => id > 0;
132+
protected static bool IsValidString(string value) => !string.IsNullOrWhiteSpace(value);
133+
}

0 commit comments

Comments
 (0)