Update CORS config; add architecture discussion comments
- Make CORS policy environment-aware: allow any origin in development, restrict to configured origins in production. - Add detailed comments in CatalogService.cs and ICatalogRepository.cs discussing generic CRUD services, CQRS with MediatR, and repository interface placement, including both Copilot's and Hakan's perspectives. - No functional changes to service or repository logic.
This commit is contained in:
@@ -14,20 +14,22 @@ builder.Services.AddEndpointsApiExplorer();
|
|||||||
builder.Services.AddSwaggerGen();
|
builder.Services.AddSwaggerGen();
|
||||||
|
|
||||||
// TODO: allow listed origins configured in appsettings.json
|
// TODO: allow listed origins configured in appsettings.json
|
||||||
|
// In any case, dont let them to free to use without cors. if there is no origin specified, block all.
|
||||||
|
// In development you can keep it easy.
|
||||||
builder.Services.AddCors(options =>
|
builder.Services.AddCors(options =>
|
||||||
{
|
{
|
||||||
var origins = builder.Configuration.GetSection("Cors:AllowedOrigins").Get<string[]>() ?? Array.Empty<string>();
|
|
||||||
options.AddDefaultPolicy(policy =>
|
options.AddDefaultPolicy(policy =>
|
||||||
{
|
{
|
||||||
if (origins.Length > 0)
|
if(builder.Environment.IsDevelopment())
|
||||||
{
|
{
|
||||||
policy.WithOrigins(origins)
|
policy.AllowAnyOrigin()
|
||||||
.AllowAnyHeader()
|
.AllowAnyHeader()
|
||||||
.AllowAnyMethod();
|
.AllowAnyMethod();
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
policy.AllowAnyOrigin()
|
var origins = builder.Configuration.GetSection("Cors:AllowedOrigins").Get<string[]>() ?? [];
|
||||||
|
policy.WithOrigins(origins)
|
||||||
.AllowAnyHeader()
|
.AllowAnyHeader()
|
||||||
.AllowAnyMethod();
|
.AllowAnyMethod();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,9 +6,33 @@ using DbFirst.Domain;
|
|||||||
namespace DbFirst.Application.Catalogs;
|
namespace DbFirst.Application.Catalogs;
|
||||||
|
|
||||||
//TODO: create generic service to reduce code duplication
|
//TODO: create generic service to reduce code duplication
|
||||||
//TODO (TR): kod tekrar?n? azaltmak için generic bir servis/basit CRUD altyap?s? ekleyin
|
|
||||||
|
/* Copilot's Response:
|
||||||
|
A generic CRUD base service adds little value in your case:
|
||||||
|
|
||||||
|
Pros:
|
||||||
|
• Less boilerplate for simple entities without special logic.
|
||||||
|
• Uniform CRUD signatures.
|
||||||
|
|
||||||
|
Cons/Practical here:
|
||||||
|
• Domain logic differs per entity(unique title check, setting audit fields, forbidding title changes, stored procs with output GUID).
|
||||||
|
• Generic services tend to be diluted by virtual methods/hooks for special cases—ending up with per-entity overrides and little real gain.
|
||||||
|
• With stored procedures and output parameters, the pattern doesn’t fit cleanly because operations aren’t symmetric (separate procs for insert/update/delete).
|
||||||
|
|
||||||
|
Conclusion: For this solution a generic service would be more overhead than benefit. If you later have multiple very similar entities without special logic,
|
||||||
|
you could consider a lightweight generic interface/base; for now, the specialized service implementation is cleaner. */
|
||||||
|
|
||||||
|
/* Hakan's Response:
|
||||||
|
* No, it absolutely makes sense to create a generic service using Options pattern. So, you can easily inject your SQL queries or stored procedure names via configuration.
|
||||||
|
* see: https://docs.microsoft.com/en-us/dotnet/core/extensions/options
|
||||||
|
*/
|
||||||
|
|
||||||
//TODO: implement CQRS pattern with MediatR
|
//TODO: implement CQRS pattern with MediatR
|
||||||
//TODO (TR): CQRS desenini MediatR ile uygulay?n
|
|
||||||
|
/* Hakan's response
|
||||||
|
* Here is the main part. We dont even need a service layer if we implement CQRS with MediatR at least for CRUD operations.
|
||||||
|
*/
|
||||||
|
|
||||||
public class CatalogService : ICatalogService
|
public class CatalogService : ICatalogService
|
||||||
{
|
{
|
||||||
private readonly ICatalogRepository _repository;
|
private readonly ICatalogRepository _repository;
|
||||||
|
|||||||
@@ -3,6 +3,38 @@ using DbFirst.Domain;
|
|||||||
|
|
||||||
namespace DbFirst.Domain.Repositories;
|
namespace DbFirst.Domain.Repositories;
|
||||||
|
|
||||||
|
// TODO: instead of creating interface per entity, consider using generic repository pattern (eg. IRepository<T>) to reduce code duplication.
|
||||||
|
|
||||||
|
/* Copilot's Response:
|
||||||
|
|
||||||
|
A generic Repository<T> isn’t really worthwhile here:
|
||||||
|
• Reads from the view are generic, but inserts/updates/deletes go through stored procedures with special parameters/output GUIDs.You’d need lots of exceptions/overrides—little gain.
|
||||||
|
• Operations aren’t symmetric (separate procs for insert/update/delete with output handling and reload), so a one-size-fits-all CRUD pattern doesn’t fit well.
|
||||||
|
• Better to keep the specialized repo.If you want reuse, extract small helpers (e.g., for proc calls/output parameters/reload) instead of forcing a generic repository. */
|
||||||
|
|
||||||
|
|
||||||
|
/* Hakan's Response:
|
||||||
|
* I think it still makes sense. An execute SQL method can be added to repository. The relevant SQL can be executed in the application layer.
|
||||||
|
* Because the application layer is no longer dependent on a method like CreateAsync. It is now directly dependent on that procedure.
|
||||||
|
*/
|
||||||
|
|
||||||
|
// TODO: move to application layer as a part of clean architecture
|
||||||
|
|
||||||
|
/* Copilot's Response:
|
||||||
|
|
||||||
|
• Common: Keep the interface in the Domain layer, implementation in Infrastructure. Domain defines the data access contract, Infrastructure provides the implementation.
|
||||||
|
That matches typical DDD/Clean Architecture.
|
||||||
|
• Alternative: Move the interface to the Application layer. Then only Application depends on Infrastructure, Domain stays isolated from infra contracts.
|
||||||
|
Pro: Domain contains only models/rules.
|
||||||
|
Con: If Domain logic needs repos, it would depend on Application, weakening the layer separation.
|
||||||
|
Recommendation: Keep the interface in Domain. It’s conventional, clean, and avoids a Domain -> Application dependency. Implementation stays in Infrastructure. */
|
||||||
|
|
||||||
|
/* Hakan's Response:
|
||||||
|
* I don't think there's any point in keeping repository interfaces in the domain layer. These interfaces will be used in the application layer, not the domain layer.
|
||||||
|
* Furthermore, this is how they are used in the most popular clean architecture pattern.
|
||||||
|
* See: https://github.com/jasontaylordev/CleanArchitecture/blob/main/src/Infrastructure/Identity/IdentityService.cs
|
||||||
|
*/
|
||||||
|
|
||||||
public interface ICatalogRepository
|
public interface ICatalogRepository
|
||||||
{
|
{
|
||||||
Task<List<VwmyCatalog>> GetAllAsync(CancellationToken cancellationToken = default);
|
Task<List<VwmyCatalog>> GetAllAsync(CancellationToken cancellationToken = default);
|
||||||
|
|||||||
Reference in New Issue
Block a user