Refactor and enhance domain models, services, and error handling#3
Merged
Conversation
- Removed unnecessary variable in RabbitMQPublisherChannelPool. - Updated RabbitMQSerializerExtensions to use null-forgiving operator for serialization. Enhance BiUMRouteAttribute and CrudController - Modified BiUMRouteAttribute to store domain code and adjusted inheritance. - Refactored CrudController methods to simplify async calls and improve readability. Update Domain CRUD Commands and DTOs - Changed properties in SaveDomainCrudCommand and related classes to required. - Enhanced mapping in DomainCrudDto and DomainCrudsDto to handle potential nulls. Refine EncryptionHelper and BaseDbContext - Improved EncryptionHelper by centralizing key and IV generation. - Made EntitySaveChangesInterceptor properties nullable in BaseDbContext. Adjust BaseRepository and DbContextInitialiser - Changed access modifiers for AddMessage methods in BaseRepository. - Updated DbContextInitialiser to use protected members for better encapsulation. Revise CrudService Implementation - Refactored CRUD operations in CrudService to utilize DbContext directly. - Enhanced SQL generation methods to be static and improved readability. Fix TranslationService Error Handling - Updated TranslationService to provide clearer error messages for missing translations.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request performs a comprehensive refactoring of the BiUM library with a focus on code quality, type safety, and maintainability. The changes introduce stricter nullable reference type enforcement, convert public APIs to more restrictive access levels, and add required modifiers to essential string properties across domain models.
Changes:
- Refactored BaseRepository and DbContextInitialiser from public to abstract with protected members, enforcing inheritance patterns
- Added
requiredmodifiers to string properties across DTOs, commands, and domain models to prevent null reference issues - Enhanced nullable reference type enforcement by treating nullable warnings as compilation errors
- Updated TranslationService error handling with hardcoded error messages
- Converted several instance Mapping methods to static in AutoMapper DTOs
- Added nuget.config for centralized package source management
Reviewed changes
Copilot reviewed 69 out of 69 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BiUM.Specialized/Services/TranslationService.cs | Modified error message generation with hardcoded code |
| src/BiUM.Specialized/Services/Crud/CrudService.cs | Refactored constructor, removed unused fields |
| src/BiUM.Specialized/Services/Crud/CrudService.*.cs | Made methods static, renamed fields to follow conventions, updated context references |
| src/BiUM.Specialized/Database/BaseRepository.cs | Changed to abstract, converted fields to properties, changed visibility to protected |
| src/BiUM.Specialized/Database/DbContextInitialiser.cs | Changed to abstract, reordered constructor parameters, converted to properties |
| src/BiUM.Specialized/Common/API/CrudController.cs | Refactored GET endpoints, changed method visibility |
| src/BiUM.Specialized/Common/API/BiUMRouteAttribute.cs | Expanded from primary constructor pattern, restricted AttributeTargets |
| src/BiUM.Specialized/Common/Utils/EncryptionHelper.cs | Refactored to use static fields for encryption key/IV |
| src/BiUM.Infrastructure/Services/Caching/Redis/RedisClient.cs | Added null checks for disabled state |
| src/BiUM.Core/MessageBroker/IBaseEntityEvent.cs | Removed audit-related properties, kept only Test property |
| src/BiUM.Core/Database/IBaseRepository.cs | Removed SaveChangesAsync from interface |
| src/BiUM.Core/Constants/Application.cs | Deleted file with BiAppOrigins constant |
| src/BiUM.Core/Common/Configs/*.cs | Added required modifiers to configuration properties |
| Directory.Build.props | Added nullable warnings as compilation errors |
| nuget.config | New file for package source configuration |
| Multiple sample and domain files | Added required modifiers to string properties, changed Mapping methods to static |
Comments suppressed due to low confidence (2)
src/BiUM.Infrastructure/Services/Caching/Redis/RedisClient.cs:226
- This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
src/BiUM.Infrastructure/Services/Caching/Redis/RedisClient.cs:388 - This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…Exception for null deserialized messages
…Air/BiUM into features/no-warning
… to instance method
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…assword-based key derivation
…Air/BiUM into features/no-warning
…ameter in Decrypt method
…d initialize collections
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements across the codebase, focusing on stricter nullability enforcement, improved DTO definitions, and minor refactoring for consistency and correctness. The most significant changes are the widespread use of the C# 11
requiredkeyword for DTO properties, updates to project and solution configuration to treat nullable warnings as errors, and various minor code quality and correctness enhancements.Nullability and DTO Improvements:
requiredkeyword to key properties in DTOs and event models (such asName,Code,CoinCode,Key, etc.) to enforce presence at compile time, improving data integrity and reducing runtime null reference errors. [1] [2] [3] [4] [5] [6] [7] [8] [9]Currencyentity to initializeCurrencyTranslationsas an empty collection by default, ensuring it is never null.Project and Build Configuration:
nuget.configto the solution and configured NuGet package sources and mapping. [1] [2]Directory.Build.propsto treat nullable warnings as errors, enforcing stricter null safety across the project.Command and Handler Improvements:
using System;in relevant files for completeness. [1] [2]Code Quality and Refactoring:
TestDbContextInitialiserclass static and improved constructor parameter names and order for consistency and correctness. [1] [2]