Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT] feat: Text*Formatter impl & ASP.NET Core API example #224

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions OSLC4Net_SDK/.dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
**/.classpath
**/.dockerignore
**/.env
**/.git
**/.gitignore
**/.project
**/.settings
**/.toolstarget
**/.vs
**/.vscode
**/*.*proj.user
**/*.dbmdl
**/*.jfm
**/azds.yaml
**/bin
**/charts
**/docker-compose*
**/Dockerfile*
**/node_modules
**/npm-debug.log
**/obj
**/secrets.dev.yaml
**/values.dev.yaml
LICENSE
README.md
!**/.gitignore
!.git/HEAD
!.git/config
!.git/packed-refs
!.git/refs/heads/**
2 changes: 2 additions & 0 deletions OSLC4Net_SDK/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
<PackageVersion Include="Microsoft.Extensions.Hosting" Version="8.0.1" />
<PackageVersion Include="Microsoft.Extensions.Hosting.Abstractions" Version="9.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging" Version="8.0.1" />
<PackageVersion Include="Swashbuckle.AspNetCore" Version="6.6.2" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="9.0.0" />
<PackageVersion Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.21.0" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using Microsoft.AspNetCore.Http.Extensions;
using Microsoft.AspNetCore.Mvc;

namespace OSLC4NetExamples.Server.NetCoreApi.Controllers;
[ApiController]
[Route("[controller]")]
public class WeatherForecastController : ControllerBase
{
private static readonly string[] Summaries = new[]
{
"Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
};

private readonly ILogger<WeatherForecastController> _logger;

public WeatherForecastController(ILogger<WeatherForecastController> logger)
{
_logger = logger;
}

[HttpGet]
[Route("oslc")]
public OSLC4Net.Core.Model.ServiceProvider Oslc()
{
var sp = new OSLC4Net.Core.Model.ServiceProvider();
sp.SetAbout(new Uri(Request.GetEncodedUrl()));
sp.SetDescription("test me");
return sp;
}
Comment on lines +21 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Address OSLC implementation concerns.

The current OSLC endpoint implementation has several issues:

  1. Missing proper content type negotiation for RDF/XML or JSON-LD
  2. Insufficient OSLC service provider details (missing services, query capabilities, etc.)
  3. Potential security risk in reflecting raw URLs without sanitization

Consider implementing proper content negotiation:

 [HttpGet]
 [Route("oslc")]
+[Produces("application/rdf+xml", "application/ld+json")]
 public OSLC4Net.Core.Model.ServiceProvider Oslc()
 {
     var sp = new OSLC4Net.Core.Model.ServiceProvider();
-    sp.SetAbout(new Uri(Request.GetEncodedUrl()));
+    // Use a configured base URL instead of raw request URL
+    sp.SetAbout(new Uri(Configuration["BaseUrl"] + Request.Path));
     sp.SetDescription("test me");
+    // Add required OSLC service definitions
+    sp.SetServices(CreateServiceDescriptions());
     return sp;
 }

Committable suggestion skipped: line range outside the PR's diff.


[HttpGet(Name = "GetWeatherForecast")]
[Route("forecast")]
public IEnumerable<WeatherForecast> Get()
{
return Enumerable.Range(1, 5).Select(index => new WeatherForecast
{
Date = DateOnly.FromDateTime(DateTime.Now.AddDays(index)),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve DateTime handling for web API scenarios.

Using DateTime.Now directly in a web API can lead to inconsistencies across different time zones and server locations.

Consider this improvement:

-            Date = DateOnly.FromDateTime(DateTime.Now.AddDays(index)),
+            Date = DateOnly.FromDateTime(DateTime.UtcNow.AddDays(index)),

For better timezone handling, consider:

  1. Accepting timezone information from the client
  2. Using DateTimeOffset instead of DateTime
  3. Returning timestamps in UTC and letting clients handle conversion
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Date = DateOnly.FromDateTime(DateTime.Now.AddDays(index)),
Date = DateOnly.FromDateTime(DateTime.UtcNow.AddDays(index)),

TemperatureC = Random.Shared.Next(-20, 55),
Summary = Summaries[Random.Shared.Next(Summaries.Length)]
})
.ToArray();
}
Comment on lines +31 to +42
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve API robustness and configurability.

The current implementation needs several improvements:

  1. Add input parameters for date range and forecast count
  2. Implement response caching to improve performance
  3. Move magic numbers to configuration
 [HttpGet(Name = "GetWeatherForecast")]
 [Route("forecast")]
+[ResponseCache(Duration = 60)] // Cache for 1 minute
+[ProducesResponseType(typeof(IEnumerable<WeatherForecast>), StatusCodes.Status200OK)]
+[ProducesResponseType(StatusCodes.Status400BadRequest)]
-public IEnumerable<WeatherForecast> Get()
+public ActionResult<IEnumerable<WeatherForecast>> Get(
+    [FromQuery] DateOnly? startDate,
+    [FromQuery] int days = 5)
 {
-    return Enumerable.Range(1, 5).Select(index => new WeatherForecast
+    if (days <= 0 || days > _configuration.GetValue<int>("MaxForecastDays", 7))
+    {
+        return BadRequest("Invalid number of days requested");
+    }
+
+    var start = startDate ?? DateOnly.FromDateTime(DateTime.Now);
+    return Ok(Enumerable.Range(1, days).Select(index => new WeatherForecast
     {
-        Date = DateOnly.FromDateTime(DateTime.Now.AddDays(index)),
+        Date = start.AddDays(index),
         TemperatureC = Random.Shared.Next(-20, 55),
         Summary = Summaries[Random.Shared.Next(Summaries.Length)]
     })
-    .ToArray();
+    .ToArray());

Committable suggestion skipped: line range outside the PR's diff.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# See https://aka.ms/customizecontainer to learn how to customize your debug container and how Visual Studio uses this Dockerfile to build your images for faster debugging.

# This stage is used when running from VS in fast mode (Default for Debug configuration)
FROM mcr.microsoft.com/dotnet/aspnet:8.0 AS base
USER app
WORKDIR /app
EXPOSE 8080
EXPOSE 8081


# This stage is used to build the service project
FROM mcr.microsoft.com/dotnet/sdk:8.0 AS build
ARG BUILD_CONFIGURATION=Release
WORKDIR /src
COPY ["Examples/Directory.Build.props", "Examples/"]
COPY ["Examples/OSLC4NetExamples.Server.NetCoreApi/OSLC4NetExamples.Server.NetCoreApi.csproj", "Examples/OSLC4NetExamples.Server.NetCoreApi/"]
RUN dotnet restore "./Examples/OSLC4NetExamples.Server.NetCoreApi/OSLC4NetExamples.Server.NetCoreApi.csproj"
COPY . .
WORKDIR "/src/Examples/OSLC4NetExamples.Server.NetCoreApi"
RUN dotnet build "./OSLC4NetExamples.Server.NetCoreApi.csproj" -c $BUILD_CONFIGURATION -o /app/build

# This stage is used to publish the service project to be copied to the final stage
FROM build AS publish
ARG BUILD_CONFIGURATION=Release
RUN dotnet publish "./OSLC4NetExamples.Server.NetCoreApi.csproj" -c $BUILD_CONFIGURATION -o /app/publish /p:UseAppHost=false

# This stage is used in production or when running from VS in regular mode (Default when not using the Debug configuration)
FROM base AS final
WORKDIR /app
COPY --from=publish /app/publish .
ENTRYPOINT ["dotnet", "OSLC4NetExamples.Server.NetCoreApi.dll"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<UserSecretsId>9d68bd64-651d-4baa-9904-6369c55b51cc</UserSecretsId>
<DockerDefaultTargetOS>Linux</DockerDefaultTargetOS>
<DockerfileContext>..\..</DockerfileContext>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" />
<PackageReference Include="Swashbuckle.AspNetCore" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\OSLC4Net.Server.Providers\OSLC4Net.Server.Providers.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@OSLC4NetExamples.Server.NetCoreApi_HostAddress = http://localhost:5065

GET {{OSLC4NetExamples.Server.NetCoreApi_HostAddress}}/weatherforecast/
Accept: application/json

###
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using OSLC4Net.Server.Providers;

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.

builder.Services.AddControllers(options =>
{
options.InputFormatters.Insert(0, new OslcRdfInputFormatter());
options.OutputFormatters.Insert(0, new OslcRdfOutputFormatter());
});
// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();

var app = builder.Build();

// Configure the HTTP request pipeline.
if (app.Environment.IsDevelopment())
{
// app.UseSwagger();
// app.UseSwaggerUI();
}

app.UseHttpsRedirection();

app.UseAuthorization();

app.MapControllers();
Comment on lines +25 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance security and error handling middleware configuration.

The current middleware setup has some potential gaps:

  1. Authorization is configured without authentication
  2. Missing global error handling
  3. No CORS policy for API access

Consider adding these essential middleware components:

 app.UseHttpsRedirection();

+app.UseExceptionHandler("/error");
+app.UseHsts();
+
+// Configure CORS if needed
+app.UseCors();
+
+// Add authentication before authorization
+app.UseAuthentication();
 app.UseAuthorization();

 app.MapControllers();

Committable suggestion skipped: line range outside the PR's diff.


app.Run();
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
{
"profiles": {
"http": {
"commandName": "Project",
"launchBrowser": true,
"launchUrl": "swagger",
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
},
"dotnetRunMessages": true,
"applicationUrl": "http://localhost:5065"
},
"https": {
"commandName": "Project",
"launchBrowser": true,
"launchUrl": "swagger",
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
},
"dotnetRunMessages": true,
"applicationUrl": "https://localhost:7270;http://localhost:5065"
},
"IIS Express": {
"commandName": "IISExpress",
"launchBrowser": true,
"launchUrl": "swagger",
"environmentVariables": {
"ASPNETCORE_ENVIRONMENT": "Development"
}
},
"Container (Dockerfile)": {
"commandName": "Docker",
"launchBrowser": true,
"launchUrl": "{Scheme}://{ServiceHost}:{ServicePort}/swagger",
"environmentVariables": {
"ASPNETCORE_HTTPS_PORTS": "8081",
"ASPNETCORE_HTTP_PORTS": "8080"
},
"publishAllPorts": true,
"useSSL": true
}
},
"$schema": "http://json.schemastore.org/launchsettings.json",
"iisSettings": {
"windowsAuthentication": false,
"anonymousAuthentication": true,
"iisExpress": {
"applicationUrl": "http://localhost:23478",
"sslPort": 44387
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace OSLC4NetExamples.Server.NetCoreApi;

public class WeatherForecast
{
public DateOnly Date { get; set; }

public int TemperatureC { get; set; }

public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);

public string? Summary { get; set; }
Comment on lines +5 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix temperature conversion formula and add data validation.

The current Celsius to Fahrenheit conversion formula uses an approximate division (0.5556). For better accuracy and maintainability:

  1. Use the exact formula: °F = °C × (9/5) + 32
  2. Add data validation for reasonable temperature ranges
-    public DateOnly Date { get; set; }
+    public DateOnly Date { get; set; }
+
+    private int temperatureC;
+    public int TemperatureC
+    {
+        get => temperatureC;
+        set
+        {
+            if (value < -273) // Absolute zero in Celsius
+                throw new ArgumentOutOfRangeException(nameof(value), "Temperature cannot be below absolute zero");
+            temperatureC = value;
+        }
+    }
 
-    public int TemperatureC { get; set; }
-
-    public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);
+    public int TemperatureF => 32 + (int)(TemperatureC * 9.0 / 5.0);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public DateOnly Date { get; set; }
public int TemperatureC { get; set; }
public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);
public string? Summary { get; set; }
public DateOnly Date { get; set; }
private int temperatureC;
public int TemperatureC
{
get => temperatureC;
set
{
if (value < -273) // Absolute zero in Celsius
throw new ArgumentOutOfRangeException(nameof(value), "Temperature cannot be below absolute zero");
temperatureC = value;
}
}
public int TemperatureF => 32 + (int)(TemperatureC * 9.0 / 5.0);
public string? Summary { get; set; }

}
Comment on lines +1 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Replace WeatherForecast with OSLC-specific example models

The WeatherForecast class and its controller are indeed template code from ASP.NET Core that should be replaced. The codebase contains rich OSLC-specific examples that would be more appropriate:

  • The OSLC4Net_NETFramework/OSLC4Net.StockQuoteSample/ directory contains a complete OSLC sample with proper models like StockQuote
  • The OSLC4Net_SDK/OSLC4Net.ChangeManagement/ directory contains OSLC Change Management models like ChangeRequest
  • The OSLC4Net_SDK/OSLC4Net.Client/Oslc/Resources/ directory has various OSLC resource models (TestCase, Requirement, etc.)

Consider replacing the WeatherForecast example with one of these domain-specific OSLC models to better demonstrate OSLC4Net functionality in the NetCoreApi example project.

🔗 Analysis chain

Verify if this template code should be replaced with domain-specific models.

This appears to be the default ASP.NET Core Web API template's WeatherForecast example. Consider whether this should be replaced with actual OSLC-specific domain models that demonstrate OSLC4Net functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if this is just template code or if it's actually used in OSLC-specific examples
rg -l "WeatherForecast" | grep -v "Weather"

Length of output: 41


Script:

#!/bin/bash
# Let's first check all files containing WeatherForecast without filtering
rg -l "WeatherForecast"

# Then check for any controllers or services using this model
ast-grep --pattern 'class $_ { $$$ WeatherForecast $$$ }'

# Check if there are any OSLC-specific models in the codebase that could serve as examples
fd -e cs -e java | xargs rg "OSLC" -l

Length of output: 17185

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"Logging": {
"LogLevel": {
"Default": "Information",
"Microsoft.AspNetCore": "Warning"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"Logging": {
"LogLevel": {
"Default": "Information",
"Microsoft.AspNetCore": "Warning"
}
},
"AllowedHosts": "*"
}
Original file line number Diff line number Diff line change
Expand Up @@ -422,14 +422,14 @@ private Type GetMemberType(Type type)
return null;
}

private static bool ImplementsGenericType(Type genericType, Type typeToTest)
public static bool ImplementsGenericType(Type genericType, Type typeToTest)
{
var isParentGeneric = genericType.IsGenericType;

return ImplementsGenericType(genericType, typeToTest, isParentGeneric);
}

private static bool ImplementsGenericType(Type genericType, Type typeToTest,
public static bool ImplementsGenericType(Type genericType, Type typeToTest,
bool isParentGeneric)
{
if (typeToTest == null)
Expand All @@ -449,7 +449,7 @@ private static bool ImplementsGenericType(Type genericType, Type typeToTest,
return ImplementsGenericType(genericType, typeToTest.BaseType, isParentGeneric);
}

private static Type[] GetChildClassParameterArguments(Type genericType, Type typeToTest)
public static Type[] GetChildClassParameterArguments(Type genericType, Type typeToTest)
{
var isParentGeneric = genericType.IsGenericType;

Expand All @@ -469,12 +469,12 @@ private static Type[] GetChildClassParameterArguments(Type genericType, Type typ
}
}

private static bool ImplementsICollection(Type type)
public static bool ImplementsICollection(Type type)
{
return type.IsGenericType && typeof(ICollection<>) == type.GetGenericTypeDefinition();
Comment on lines +472 to 474
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance ICollection check to handle derived interfaces.

The current implementation only checks for direct ICollection implementation. Consider using Type.GetInterfaces() to check for derived interfaces as well.

Apply this enhancement:

     public static bool ImplementsICollection(Type type)
     {
-        return type.IsGenericType && typeof(ICollection<>) == type.GetGenericTypeDefinition();
+        if (!type.IsGenericType)
+        {
+            return false;
+        }
+        
+        if (type.GetGenericTypeDefinition() == typeof(ICollection<>))
+        {
+            return true;
+        }
+
+        return type.GetInterfaces()
+            .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollection<>));
     }

Also add XML documentation:

+    /// <summary>
+    /// Determines whether the specified type implements ICollection{T} directly or through inheritance.
+    /// </summary>
+    /// <param name="type">The type to check.</param>
+    /// <returns>True if the type implements ICollection{T}; otherwise, false.</returns>
     public static bool ImplementsICollection(Type type)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static bool ImplementsICollection(Type type)
{
return type.IsGenericType && typeof(ICollection<>) == type.GetGenericTypeDefinition();
/// <summary>
/// Determines whether the specified type implements ICollection{T} directly or through inheritance.
/// </summary>
/// <param name="type">The type to check.</param>
/// <returns>True if the type implements ICollection{T}; otherwise, false.</returns>
public static bool ImplementsICollection(Type type)
{
if (!type.IsGenericType)
{
return false;
}
if (type.GetGenericTypeDefinition() == typeof(ICollection<>))
{
return true;
}
return type.GetInterfaces()
.Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollection<>));
}

}

private class NonClosingStreamWriter : StreamWriter
public class NonClosingStreamWriter : StreamWriter
{
public NonClosingStreamWriter(Stream stream)
: base(stream)
Expand Down
Loading
Loading