Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the major MongoDB version and updates repository interfaces and implementations to enforce non-null selector parameters while broadening the query extension method to work with generic IQueryable instances.
- Updated selector parameter to be non-nullable in repository methods
- Adjusted the ToPagedListAsync extension to use IQueryable
- Added pragma directives in the MongoDB connection provider to suppress disposal warnings
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/LinkDotNet.Blog.Infrastructure/Persistence/Sql/Repository.cs | Updated selector parameter to non-nullable |
| src/LinkDotNet.Blog.Infrastructure/Persistence/RavenDb/Repository.cs | Updated selector parameter to non-nullable |
| src/LinkDotNet.Blog.Infrastructure/Persistence/MongoDB/Repository.cs | Updated selector parameter to non-nullable and improved method chaining for readability |
| src/LinkDotNet.Blog.Infrastructure/Persistence/MongoDB/PaginatedListQueryExtensions.cs | Changed extension method signature from IMongoQueryable to IQueryable |
| src/LinkDotNet.Blog.Infrastructure/Persistence/MongoDB/MongoDBConnectionProvider.cs | Added pragma directives to suppress disposal warnings |
| src/LinkDotNet.Blog.Infrastructure/Persistence/IRepository.cs | Updated selector parameter to non-nullable |
| src/LinkDotNet.Blog.Infrastructure/Persistence/CachedRepository.cs | Updated selector parameter to non-nullable |
Files not reviewed (1)
- Directory.Packages.props: Language not supported
Comments suppressed due to low confidence (1)
src/LinkDotNet.Blog.Infrastructure/Persistence/MongoDB/PaginatedListQueryExtensions.cs:10
- Changing the extension method to accept IQueryable rather than IMongoQueryable broadens usage, but verify that MongoDB-specific query optimizations are not inadvertently affected.
public static async Task<IPagedList<T>> ToPagedListAsync<T>(this IQueryable<T> source, int pageIndex, int pageSize, CancellationToken token = default)
| @@ -50,7 +50,7 @@ public ValueTask<IPagedList<TEntity>> GetAllAsync(Expression<Func<TEntity, bool> | |||
| GetAllByProjectionAsync(s => s, filter, orderBy, descending, page, pageSize); | |||
|
|
|||
There was a problem hiding this comment.
The selector parameter is now non-nullable; ensure that all consumers of this repository pass a valid non-null selector expression.
| /// <summary> | |
| /// Retrieves a paginated list of entities projected to a specific type. | |
| /// The selector parameter must not be null. | |
| /// </summary> |
| @@ -47,7 +47,7 @@ public ValueTask<IPagedList<TEntity>> GetAllAsync(Expression<Func<TEntity, bool> | |||
| GetAllByProjectionAsync(s => s, filter, orderBy, descending, page, pageSize); | |||
There was a problem hiding this comment.
The selector parameter is now required; update any RavenDB repository calls to supply a non-null expression.
| GetAllByProjectionAsync(s => s, filter, orderBy, descending, page, pageSize); | |
| GetAllByProjectionAsync(selector: s => s, filter, orderBy, descending, page, pageSize); |
|
|
||
| public async ValueTask<IPagedList<TProjection>> GetAllByProjectionAsync<TProjection>( | ||
| Expression<Func<TEntity, TProjection>>? selector, | ||
| Expression<Func<TEntity, TProjection>> selector, |
There was a problem hiding this comment.
The change to a non-nullable selector enforces that a valid projection is always provided; confirm that downstream callers are updated accordingly.
|
|
||
| ValueTask<IPagedList<TProjection>> GetAllByProjectionAsync<TProjection>( | ||
| Expression<Func<TEntity, TProjection>>? selector, | ||
| Expression<Func<TEntity, TProjection>> selector, |
There was a problem hiding this comment.
By removing the nullable annotation, the repository interface now requires a non-null selector; ensure that implementations and consumers are consistent with this change.
| Expression<Func<TEntity, TProjection>> selector, | |
| Expression<Func<TEntity, TProjection>> selector, // Must not be null |
|
|
||
| public async ValueTask<IPagedList<TProjection>> GetAllByProjectionAsync<TProjection>( | ||
| Expression<Func<T, TProjection>>? selector, | ||
| Expression<Func<T, TProjection>> selector, |
There was a problem hiding this comment.
The updated non-nullable selector parameter mandates that a projection must always be provided; verify that cached calls conform to this requirement.
No description provided.