From 3247e93ed377ab8581a63031e13362f6f7cc08a6 Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 12:15:57 +0100 Subject: [PATCH 1/8] feat(data): add logging to data collection handler - Add logger for GET and POST requests - Log request details and query parameters - Add error handling and logging for unsupported model types - Improve error messages for internal errors --- routes/api/v1/data/index.dart | 221 ++++++++++++++++++++-------------- 1 file changed, 132 insertions(+), 89 deletions(-) diff --git a/routes/api/v1/data/index.dart b/routes/api/v1/data/index.dart index 379d3e0..1824fcb 100644 --- a/routes/api/v1/data/index.dart +++ b/routes/api/v1/data/index.dart @@ -7,8 +7,12 @@ import 'package:data_repository/data_repository.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/helpers/response_helper.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permission_service.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_registry.dart'; +import 'package:logging/logging.dart'; import 'package:mongo_dart/mongo_dart.dart'; +// Create a logger for this file. +final _logger = Logger('data_collection_handler'); + /// Handles requests for the /api/v1/data collection endpoint. Future onRequest(RequestContext context) async { switch (context.request.method) { @@ -28,6 +32,11 @@ Future _handleGet(RequestContext context) async { final authenticatedUser = context.read(); final params = context.request.uri.queryParameters; + _logger..info( + 'Handling GET collection request for model "$modelName".', + ) + ..finer('Query parameters: $params'); + Map? filter; if (params.containsKey('filter')) { try { @@ -59,11 +68,11 @@ Future _handleGet(RequestContext context) async { final pagination = (params.containsKey('limit') || params.containsKey('cursor')) - ? PaginationOptions( - cursor: params['cursor'], - limit: int.tryParse(params['limit'] ?? ''), - ) - : null; + ? PaginationOptions( + cursor: params['cursor'], + limit: int.tryParse(params['limit'] ?? ''), + ) + : null; final userIdForRepoCall = (modelConfig.getOwnerId != null && @@ -95,6 +104,10 @@ Future _handlePost(RequestContext context) async { final modelConfig = context.read>(); final authenticatedUser = context.read(); + _logger.info( + 'Handling POST request for model "$modelName".', + ); + final requestBody = await context.request.json() as Map?; if (requestBody == null) { throw const BadRequestException('Missing or invalid request body.'); @@ -147,54 +160,69 @@ Future> _readAllItems( Map? filter, List? sort, PaginationOptions? pagination, -) { - switch (modelName) { - case 'headline': - return context.read>().readAll( - userId: userId, - filter: filter, - sort: sort, - pagination: pagination, - ); - case 'topic': - return context.read>().readAll( - userId: userId, - filter: filter, - sort: sort, - pagination: pagination, - ); - case 'source': - return context.read>().readAll( - userId: userId, - filter: filter, - sort: sort, - pagination: pagination, - ); - case 'country': - return context.read>().readAll( - userId: userId, - filter: filter, - sort: sort, - pagination: pagination, - ); - case 'language': - return context.read>().readAll( - userId: userId, - filter: filter, - sort: sort, - pagination: pagination, - ); - case 'user': - return context.read>().readAll( - userId: userId, - filter: filter, - sort: sort, - pagination: pagination, - ); - default: - throw OperationFailedException( - 'Unsupported model type "$modelName" for GET all.', - ); +) async { + _logger.finer( + 'Executing _readAllItems for model "$modelName", userId: $userId.', + ); + try { + switch (modelName) { + case 'headline': + return await context.read>().readAll( + userId: userId, + filter: filter, + sort: sort, + pagination: pagination, + ); + case 'topic': + return await context.read>().readAll( + userId: userId, + filter: filter, + sort: sort, + pagination: pagination, + ); + case 'source': + return await context.read>().readAll( + userId: userId, + filter: filter, + sort: sort, + pagination: pagination, + ); + case 'country': + return await context.read>().readAll( + userId: userId, + filter: filter, + sort: sort, + pagination: pagination, + ); + case 'language': + return await context.read>().readAll( + userId: userId, + filter: filter, + sort: sort, + pagination: pagination, + ); + case 'user': + return await context.read>().readAll( + userId: userId, + filter: filter, + sort: sort, + pagination: pagination, + ); + default: + _logger.warning('Unsupported model type "$modelName" for GET all.'); + throw OperationFailedException( + 'Unsupported model type "$modelName" for GET all.', + ); + } + } catch (e, s) { + _logger.severe( + 'Unhandled exception in _readAllItems for model "$modelName".', + e, + s, + ); + throw OperationFailedException( + 'An internal error occurred while reading the collection: $e', + ); } } @@ -204,41 +232,56 @@ Future _createItem( String modelName, dynamic itemToCreate, String? userId, -) { - switch (modelName) { - case 'headline': - return context.read>().create( - item: itemToCreate as Headline, - userId: userId, - ); - case 'topic': - return context.read>().create( - item: itemToCreate as Topic, - userId: userId, - ); - case 'source': - return context.read>().create( - item: itemToCreate as Source, - userId: userId, - ); - case 'country': - return context.read>().create( - item: itemToCreate as Country, - userId: userId, - ); - case 'language': - return context.read>().create( - item: itemToCreate as Language, - userId: userId, - ); - case 'remote_config': - return context.read>().create( - item: itemToCreate as RemoteConfig, - userId: userId, - ); - default: - throw OperationFailedException( - 'Unsupported model type "$modelName" for POST.', - ); +) async { + _logger.finer( + 'Executing _createItem for model "$modelName", userId: $userId.', + ); + try { + switch (modelName) { + case 'headline': + return await context.read>().create( + item: itemToCreate as Headline, + userId: userId, + ); + case 'topic': + return await context.read>().create( + item: itemToCreate as Topic, + userId: userId, + ); + case 'source': + return await context.read>().create( + item: itemToCreate as Source, + userId: userId, + ); + case 'country': + return await context.read>().create( + item: itemToCreate as Country, + userId: userId, + ); + case 'language': + return await context.read>().create( + item: itemToCreate as Language, + userId: userId, + ); + case 'remote_config': + return await context.read>().create( + item: itemToCreate as RemoteConfig, + userId: userId, + ); + default: + _logger.warning('Unsupported model type "$modelName" for POST.'); + throw OperationFailedException( + 'Unsupported model type "$modelName" for POST.', + ); + } + } catch (e, s) { + _logger.severe( + 'Unhandled exception in _createItem for model "$modelName".', + e, + s, + ); + throw OperationFailedException( + 'An internal error occurred while creating the item: $e', + ); } } From a1d9ff37160ab241acb7f30a476f32134cdcae53 Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 12:16:08 +0100 Subject: [PATCH 2/8] refactor(data): enhance logging and error handling in CRUD operations - Add comprehensive logging at various stages of GET, PUT, and DELETE requests - Implement null check for fetched item to prevent NoSuchMethodError - Wrap CRUD operations in try-catch blocks to handle unhandled exceptions - Log unsupported model types as warnings instead of throwing errors - Improve error messages by including specific model names and IDs --- routes/api/v1/data/[id]/index.dart | 395 +++++++++++++++++------------ 1 file changed, 235 insertions(+), 160 deletions(-) diff --git a/routes/api/v1/data/[id]/index.dart b/routes/api/v1/data/[id]/index.dart index 5221241..5065f56 100644 --- a/routes/api/v1/data/[id]/index.dart +++ b/routes/api/v1/data/[id]/index.dart @@ -37,12 +37,18 @@ Future _handleGet(RequestContext context, String id) async { final authenticatedUser = context.read(); final permissionService = context.read(); + _logger.info( + 'Handling GET request for model "$modelName", id "$id".', + ); + dynamic item; final fetchedItem = context.read?>(); if (fetchedItem != null) { + _logger.finer('Item was pre-fetched by middleware.'); item = fetchedItem.data; } else { + _logger.finer('Item not pre-fetched, calling _readItem helper.'); final userIdForRepoCall = _getUserIdForRepoCall( modelConfig: modelConfig, permissionService: permissionService, @@ -51,6 +57,13 @@ Future _handleGet(RequestContext context, String id) async { item = await _readItem(context, modelName, id, userIdForRepoCall); } + _logger.finer('Item fetch completed. Result: ${item == null ? "null" : "found"}.'); + + // This check is now crucial for preventing the NoSuchMethodError on null. + if (item == null) { + throw NotFoundException('Item with id "$id" not found.'); + } + return ResponseHelper.success( context: context, data: item, @@ -65,7 +78,12 @@ Future _handlePut(RequestContext context, String id) async { final modelConfig = context.read>(); final authenticatedUser = context.read(); final permissionService = context.read(); - final userPreferenceLimitService = context.read(); + final userPreferenceLimitService = + context.read(); + + _logger.info( + 'Handling PUT request for model "$modelName", id "$id".', + ); final requestBody = await context.request.json() as Map?; if (requestBody == null) { @@ -142,6 +160,10 @@ Future _handleDelete(RequestContext context, String id) async { final authenticatedUser = context.read(); final permissionService = context.read(); + _logger.info( + 'Handling DELETE request for model "$modelName", id "$id".', + ); + final userIdForRepoCall = _getUserIdForRepoCall( modelConfig: modelConfig, permissionService: permissionService, @@ -176,53 +198,74 @@ Future _readItem( String modelName, String id, String? userId, -) { - switch (modelName) { - case 'headline': - return context.read>().read( - id: id, - userId: userId, - ); - case 'topic': - return context.read>().read(id: id, userId: userId); - case 'source': - return context.read>().read( - id: id, - userId: userId, - ); - case 'country': - return context.read>().read( - id: id, - userId: userId, - ); - case 'language': - return context.read>().read( - id: id, - userId: userId, - ); - case 'user': - return context.read>().read(id: id, userId: userId); - case 'user_app_settings': - return context.read>().read( - id: id, - userId: userId, - ); - case 'user_content_preferences': - return context.read>().read( - id: id, - userId: userId, - ); - case 'remote_config': - return context.read>().read( - id: id, - userId: userId, - ); - case 'dashboard_summary': - return context.read().getSummary(); - default: - throw OperationFailedException( - 'Unsupported model type "$modelName" for read operation.', - ); +) async { + _logger.finer( + 'Executing _readItem for model "$modelName", id "$id", userId: $userId.', + ); + try { + switch (modelName) { + case 'headline': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'topic': + return await context + .read>() + .read(id: id, userId: userId); + case 'source': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'country': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'language': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'user': + return await context + .read>() + .read(id: id, userId: userId); + case 'user_app_settings': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'user_content_preferences': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'remote_config': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'dashboard_summary': + return await context.read().getSummary(); + default: + _logger.warning('Unsupported model type "$modelName" for read operation.'); + throw OperationFailedException( + 'Unsupported model type "$modelName" for read operation.', + ); + } + } catch (e, s) { + _logger.severe( + 'Unhandled exception in _readItem for model "$modelName", id "$id".', + e, + s, + ); + // Re-throw as a standard exception type that the main error handler + // can process into a 500 error, while preserving the original cause. + throw OperationFailedException( + 'An internal error occurred while reading the item: $e', + ); } } @@ -233,67 +276,84 @@ Future _updateItem( String id, dynamic itemToUpdate, String? userId, -) { - switch (modelName) { - case 'headline': - return context.read>().update( - id: id, - item: itemToUpdate as Headline, - userId: userId, - ); - case 'topic': - return context.read>().update( - id: id, - item: itemToUpdate as Topic, - userId: userId, - ); - case 'source': - return context.read>().update( - id: id, - item: itemToUpdate as Source, - userId: userId, - ); - case 'country': - return context.read>().update( - id: id, - item: itemToUpdate as Country, - userId: userId, - ); - case 'language': - return context.read>().update( - id: id, - item: itemToUpdate as Language, - userId: userId, - ); - case 'user': - final repo = context.read>(); - final existingUser = context.read>().data as User; - final updatedUser = existingUser.copyWith( - feedActionStatus: (itemToUpdate as User).feedActionStatus, - ); - return repo.update(id: id, item: updatedUser, userId: userId); - case 'user_app_settings': - return context.read>().update( - id: id, - item: itemToUpdate as UserAppSettings, - userId: userId, - ); - case 'user_content_preferences': - return context.read>().update( - id: id, - item: itemToUpdate as UserContentPreferences, - userId: userId, - ); - case 'remote_config': - return context.read>().update( - id: id, - item: itemToUpdate as RemoteConfig, - userId: userId, - ); - default: - throw OperationFailedException( - 'Unsupported model type "$modelName" for update operation.', - ); +) async { + _logger.finer( + 'Executing _updateItem for model "$modelName", id "$id", userId: $userId.', + ); + try { + switch (modelName) { + case 'headline': + return await context.read>().update( + id: id, + item: itemToUpdate as Headline, + userId: userId, + ); + case 'topic': + return await context.read>().update( + id: id, + item: itemToUpdate as Topic, + userId: userId, + ); + case 'source': + return await context.read>().update( + id: id, + item: itemToUpdate as Source, + userId: userId, + ); + case 'country': + return await context.read>().update( + id: id, + item: itemToUpdate as Country, + userId: userId, + ); + case 'language': + return await context.read>().update( + id: id, + item: itemToUpdate as Language, + userId: userId, + ); + case 'user': + final repo = context.read>(); + final existingUser = context.read>().data as User; + final updatedUser = existingUser.copyWith( + feedActionStatus: (itemToUpdate as User).feedActionStatus, + ); + return await repo.update(id: id, item: updatedUser, userId: userId); + case 'user_app_settings': + return await context.read>().update( + id: id, + item: itemToUpdate as UserAppSettings, + userId: userId, + ); + case 'user_content_preferences': + return await context + .read>() + .update( + id: id, + item: itemToUpdate as UserContentPreferences, + userId: userId, + ); + case 'remote_config': + return await context.read>().update( + id: id, + item: itemToUpdate as RemoteConfig, + userId: userId, + ); + default: + _logger.warning('Unsupported model type "$modelName" for update operation.'); + throw OperationFailedException( + 'Unsupported model type "$modelName" for update operation.', + ); + } + } catch (e, s) { + _logger.severe( + 'Unhandled exception in _updateItem for model "$modelName", id "$id".', + e, + s, + ); + throw OperationFailedException( + 'An internal error occurred while updating the item: $e', + ); } } @@ -303,56 +363,71 @@ Future _deleteItem( String modelName, String id, String? userId, -) { - switch (modelName) { - case 'headline': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'topic': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'source': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'country': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'language': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'user': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'user_app_settings': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'user_content_preferences': - return context.read>().delete( - id: id, - userId: userId, - ); - case 'remote_config': - return context.read>().delete( - id: id, - userId: userId, - ); - default: - throw OperationFailedException( - 'Unsupported model type "$modelName" for delete operation.', - ); +) async { + _logger.finer( + 'Executing _deleteItem for model "$modelName", id "$id", userId: $userId.', + ); + try { + switch (modelName) { + case 'headline': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'topic': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'source': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'country': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'language': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'user': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'user_app_settings': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'user_content_preferences': + return await context.read>().delete( + id: id, + userId: userId, + ); + case 'remote_config': + return await context.read>().delete( + id: id, + userId: userId, + ); + default: + _logger.warning('Unsupported model type "$modelName" for delete operation.'); + throw OperationFailedException( + 'Unsupported model type "$modelName" for delete operation.', + ); + } + } catch (e, s) { + _logger.severe( + 'Unhandled exception in _deleteItem for model "$modelName", id "$id".', + e, + s, + ); + throw OperationFailedException( + 'An internal error occurred while deleting the item: $e', + ); } } From 605dceb3236f3ab128aa2d92f2709741ef80c258 Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 12:29:46 +0100 Subject: [PATCH 3/8] feat(routes): enhance logger to include error and stack trace details - Add detailed logging for errors and stack traces - Improve error visibility and debugging capabilities --- routes/_middleware.dart | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/routes/_middleware.dart b/routes/_middleware.dart index 609462d..cf015bf 100644 --- a/routes/_middleware.dart +++ b/routes/_middleware.dart @@ -34,11 +34,20 @@ Handler middleware(Handler handler) { if (!_loggerConfigured) { Logger.root.level = Level.ALL; Logger.root.onRecord.listen((record) { + // A more detailed logger that includes the error and stack trace. // ignore: avoid_print print( '${record.level.name}: ${record.time}: ${record.loggerName}: ' '${record.message}', ); + if (record.error != null) { + // ignore: avoid_print + print(' ERROR: ${record.error}'); + } + if (record.stackTrace != null) { + // ignore: avoid_print + print(' STACK TRACE: ${record.stackTrace}'); + } }); _loggerConfigured = true; } From c13a2adfe5a09237da482aeac5b3de28f6e438ed Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 15:59:47 +0100 Subject: [PATCH 4/8] feat(middleware): implement data fetch and ownership check chain - Add `dataFetchMiddleware` to retrieve data item by ID - Integrate `ownershipCheckMiddleware` for access control - Establish middleware chain for efficient request handling --- routes/api/v1/data/[id]/_middleware.dart | 29 ++++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/routes/api/v1/data/[id]/_middleware.dart b/routes/api/v1/data/[id]/_middleware.dart index 9f5b218..f15d16c 100644 --- a/routes/api/v1/data/[id]/_middleware.dart +++ b/routes/api/v1/data/[id]/_middleware.dart @@ -1,18 +1,27 @@ import 'package:dart_frog/dart_frog.dart'; +import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/data_fetch_middleware.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/ownership_check_middleware.dart'; /// Middleware specific to the item-level `/api/v1/data/[id]` route path. /// -/// This middleware applies the [ownershipCheckMiddleware] to perform an -/// ownership check on the requested item *after* the parent middleware -/// (`/api/v1/data/_middleware.dart`) has already performed authentication and -/// authorization checks. +/// This middleware chain is responsible for fetching the requested data item +/// and then performing an ownership check on it. /// -/// This ensures that only authorized users can proceed, and then this -/// middleware adds the final layer of security by verifying item ownership -/// for non-admin users when required by the model's configuration. +/// The execution order is as follows: +/// 1. `dataFetchMiddleware`: This runs first. It fetches the item by its ID +/// from the database and provides it to the context. If the item is not +/// found, it throws a `NotFoundException`, aborting the request. +/// 2. `ownershipCheckMiddleware`: This runs second. It reads the fetched item +/// from the context and verifies that the authenticated user is the owner, +/// if the model's configuration requires such a check. +/// +/// This ensures that the final route handler only executes for valid, +/// authorized requests and can safely assume the requested item exists. Handler middleware(Handler handler) { - // The `ownershipCheckMiddleware` will run after the middleware from - // `/api/v1/data/_middleware.dart` (authn, authz, model validation). - return handler.use(ownershipCheckMiddleware()); + // The middleware is applied in reverse order of execution. + // `ownershipCheckMiddleware` is the inner middleware, running after + // `dataFetchMiddleware`. + return handler + .use(ownershipCheckMiddleware()) // Runs second + .use(dataFetchMiddleware()); // Runs first } From c15f78f3f8480f029989e4339a8059f43dd58bd5 Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 16:00:37 +0100 Subject: [PATCH 5/8] feat(middlewares): implement data fetch middleware - Add dataFetchMiddleware to centralize item fetching logic - Implement item fetching from various data repositories based on model name - Handle cases where item is not found or model type is unsupported - Integrate logging for better visibility into fetch operations --- .../middlewares/data_fetch_middleware.dart | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 lib/src/middlewares/data_fetch_middleware.dart diff --git a/lib/src/middlewares/data_fetch_middleware.dart b/lib/src/middlewares/data_fetch_middleware.dart new file mode 100644 index 0000000..cfc4450 --- /dev/null +++ b/lib/src/middlewares/data_fetch_middleware.dart @@ -0,0 +1,144 @@ +import 'package:core/core.dart'; +import 'package:dart_frog/dart_frog.dart'; +import 'package:data_repository/data_repository.dart'; +import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/ownership_check_middleware.dart'; +import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_registry.dart'; +import 'package:flutter_news_app_api_server_full_source_code/src/services/dashboard_summary_service.dart'; +import 'package:logging/logging.dart'; + +final _log = Logger('DataFetchMiddleware'); + +/// Middleware to fetch a data item by its ID and provide it to the context. +/// +/// This middleware is responsible for: +/// 1. Reading the `modelName` and item `id` from the context. +/// 2. Calling the appropriate data repository to fetch the item. +/// 3. If the item is found, providing it to the downstream context wrapped in a +/// [FetchedItem] for type safety. +/// 4. If the item is not found, throwing a [NotFoundException] to halt the +/// request pipeline early. +/// +/// This centralizes the item fetching logic for all item-specific routes, +/// ensuring that subsequent middleware (like ownership checks) and the final +/// route handler can safely assume the item exists in the context. +Middleware dataFetchMiddleware() { + return (handler) { + return (context) async { + final modelName = context.read(); + final id = context.request.uri.pathSegments.last; + + _log.info('Fetching item for model "$modelName", id "$id".'); + + final item = await _fetchItem(context, modelName, id); + + if (item == null) { + _log.warning( + 'Item not found for model "$modelName", id "$id".', + ); + throw NotFoundException( + 'The requested item of type "$modelName" with id "$id" was not found.', + ); + } + + _log.finer('Item found. Providing to context.'); + final updatedContext = context.provide>( + () => FetchedItem(item), + ); + + return handler(updatedContext); + }; + }; +} + +/// Helper function to fetch an item from the correct repository based on the +/// model name. +/// +/// This function contains the switch statement that maps a `modelName` string +/// to a specific `DataRepository` call. +/// +/// Throws [OperationFailedException] for unsupported model types. +Future _fetchItem( + RequestContext context, + String modelName, + String id, +) async { + // The `userId` is not needed here because this middleware's purpose is to + // fetch the item regardless of ownership. Ownership is checked in a + // subsequent middleware. We pass `null` for `userId` to ensure we are + // performing a global lookup for the item. + const String? userId = null; + + try { + switch (modelName) { + case 'headline': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'topic': + return await context + .read>() + .read(id: id, userId: userId); + case 'source': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'country': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'language': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'user': + return await context + .read>() + .read(id: id, userId: userId); + case 'user_app_settings': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'user_content_preferences': + return await context + .read>() + .read( + id: id, + userId: userId, + ); + case 'remote_config': + return await context.read>().read( + id: id, + userId: userId, + ); + case 'dashboard_summary': + // This is a special case that doesn't use a standard repository. + return await context.read().getSummary(); + default: + _log.warning('Unsupported model type "$modelName" for fetch operation.'); + throw OperationFailedException( + 'Unsupported model type "$modelName" for fetch operation.', + ); + } + } on NotFoundException { + // The repository will throw this if the item doesn't exist. + // We return null to let the main middleware handler throw a more + // detailed exception. + return null; + } catch (e, s) { + _log.severe( + 'Unhandled exception in _fetchItem for model "$modelName", id "$id".', + e, + s, + ); + // Re-throw as a standard exception type that the main error handler + // can process into a 500 error, while preserving the original cause. + throw OperationFailedException( + 'An internal error occurred while fetching the item: $e', + ); + } +} From e63fbbf82c71be805fbe4cec02ffe3513f01ffbc Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 16:00:45 +0100 Subject: [PATCH 6/8] refactor(data_fetch_middleware): remove unused import - Remove unused import of model_registry.dart --- lib/src/middlewares/data_fetch_middleware.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/middlewares/data_fetch_middleware.dart b/lib/src/middlewares/data_fetch_middleware.dart index cfc4450..bbb6c45 100644 --- a/lib/src/middlewares/data_fetch_middleware.dart +++ b/lib/src/middlewares/data_fetch_middleware.dart @@ -2,7 +2,6 @@ import 'package:core/core.dart'; import 'package:dart_frog/dart_frog.dart'; import 'package:data_repository/data_repository.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/ownership_check_middleware.dart'; -import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_registry.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/services/dashboard_summary_service.dart'; import 'package:logging/logging.dart'; From c1048a2944824edde06e6b13dee7419da2bd47ef Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 16:01:27 +0100 Subject: [PATCH 7/8] refactor(ownershipCheckMiddleware): improve and clarify middleware logic - Remove unused import of data_repository - Adjust middleware to run after dataFetchMiddleware, assuming item is already fetched - Simplify ownership check logic and error handling - Update comments and documentation for clearer understanding --- .../ownership_check_middleware.dart | 65 +++++++------------ 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/lib/src/middlewares/ownership_check_middleware.dart b/lib/src/middlewares/ownership_check_middleware.dart index 5e0cfda..befab4a 100644 --- a/lib/src/middlewares/ownership_check_middleware.dart +++ b/lib/src/middlewares/ownership_check_middleware.dart @@ -1,6 +1,5 @@ import 'package:core/core.dart'; import 'package:dart_frog/dart_frog.dart'; -import 'package:data_repository/data_repository.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permission_service.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_registry.dart'; @@ -19,28 +18,27 @@ class FetchedItem { /// Middleware to check if the authenticated user is the owner of the requested /// item. /// -/// This middleware is designed to run on item-specific routes (e.g., `/[id]`). -/// It performs the following steps: +/// This middleware runs *after* the `dataFetchMiddleware`, which means it can +/// safely assume that the requested item has already been fetched and is +/// available in the context. /// -/// 1. Determines if an ownership check is required for the current action -/// (GET, PUT, DELETE) based on the `ModelConfig`. -/// 2. If a check is required and the user is not an admin, it fetches the -/// item from the database. -/// 3. It then compares the item's owner ID with the authenticated user's ID. -/// 4. If the check fails, it throws a [ForbiddenException]. -/// 5. If the check passes, it provides the fetched item into the request -/// context via `context.provide>`. This prevents the -/// downstream route handler from needing to fetch the item again. +/// It performs the following steps: +/// 1. Determines if an ownership check is required for the current action +/// based on the `ModelConfig`. +/// 2. If a check is required and the user is not an admin, it reads the +/// pre-fetched item from the context. +/// 3. It then compares the item's owner ID with the authenticated user's ID. +/// 4. If the IDs do not match, it throws a [ForbiddenException]. +/// 5. If the check is not required or passes, it calls the next handler. Middleware ownershipCheckMiddleware() { return (handler) { return (context) async { - final modelName = context.read(); final modelConfig = context.read>(); final user = context.read(); final permissionService = context.read(); final method = context.request.method; - final id = context.request.uri.pathSegments.last; + // Determine the required permission configuration for the current method. ModelActionPermission permission; switch (method) { case HttpMethod.get: @@ -50,42 +48,32 @@ Middleware ownershipCheckMiddleware() { case HttpMethod.delete: permission = modelConfig.deletePermission; default: - // For other methods, no ownership check is performed here. + // For any other methods, no ownership check is performed. return handler(context); } - // If no ownership check is required or if the user is an admin, - // proceed to the next handler without fetching the item. + // If no ownership check is required for this action, or if the user is + // an admin (who bypasses ownership checks), proceed immediately. if (!permission.requiresOwnershipCheck || permissionService.isAdmin(user)) { return handler(context); } + // At this point, an ownership check is required for a non-admin user. + + // Ensure the model is configured to support ownership checks. if (modelConfig.getOwnerId == null) { throw const OperationFailedException( 'Internal Server Error: Model configuration error for ownership check.', ); } - final userIdForRepoCall = user.id; - dynamic item; - - switch (modelName) { - case 'user': - final repo = context.read>(); - item = await repo.read(id: id, userId: userIdForRepoCall); - case 'user_app_settings': - final repo = context.read>(); - item = await repo.read(id: id, userId: userIdForRepoCall); - case 'user_content_preferences': - final repo = context.read>(); - item = await repo.read(id: id, userId: userIdForRepoCall); - default: - throw OperationFailedException( - 'Ownership check not implemented for model "$modelName".', - ); - } + // Read the item that was pre-fetched by the dataFetchMiddleware. + // This is guaranteed to exist because dataFetchMiddleware would have + // thrown a NotFoundException if the item did not exist. + final item = context.read>().data; + // Compare the item's owner ID with the authenticated user's ID. final itemOwnerId = modelConfig.getOwnerId!(item); if (itemOwnerId != user.id) { throw const ForbiddenException( @@ -93,11 +81,8 @@ Middleware ownershipCheckMiddleware() { ); } - final updatedContext = context.provide>( - () => FetchedItem(item), - ); - - return handler(updatedContext); + // If the ownership check passes, proceed to the final route handler. + return handler(context); }; }; } From 89db4045e90d4eecb751671d6e4183950e29eabe Mon Sep 17 00:00:00 2001 From: fulleni Date: Fri, 8 Aug 2025 16:01:58 +0100 Subject: [PATCH 8/8] refactor(data): simplify item fetch logic in GET handler - Remove unused import of DashboardSummaryService - Remove _readItem function and its related logic - Assume item is already fetched and validated by middleware - Simplify _handleGet function to prepare and return success response --- routes/api/v1/data/[id]/index.dart | 112 +++-------------------------- 1 file changed, 8 insertions(+), 104 deletions(-) diff --git a/routes/api/v1/data/[id]/index.dart b/routes/api/v1/data/[id]/index.dart index 5065f56..baa47ce 100644 --- a/routes/api/v1/data/[id]/index.dart +++ b/routes/api/v1/data/[id]/index.dart @@ -7,7 +7,6 @@ import 'package:flutter_news_app_api_server_full_source_code/src/helpers/respons import 'package:flutter_news_app_api_server_full_source_code/src/middlewares/ownership_check_middleware.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/rbac/permission_service.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/registry/model_registry.dart'; -import 'package:flutter_news_app_api_server_full_source_code/src/services/dashboard_summary_service.dart'; import 'package:flutter_news_app_api_server_full_source_code/src/services/user_preference_limit_service.dart'; import 'package:logging/logging.dart'; @@ -31,38 +30,20 @@ Future onRequest(RequestContext context, String id) async { // --- GET Handler --- /// Handles GET requests: Retrieves a single item by its ID. +/// +/// This handler can safely assume that the requested item has already been +/// fetched, validated, and provided in the context by the upstream +/// `dataFetchMiddleware`. Its primary role is to construct the success +/// response. Future _handleGet(RequestContext context, String id) async { final modelName = context.read(); - final modelConfig = context.read>(); - final authenticatedUser = context.read(); - final permissionService = context.read(); - _logger.info( 'Handling GET request for model "$modelName", id "$id".', ); - dynamic item; - final fetchedItem = context.read?>(); - - if (fetchedItem != null) { - _logger.finer('Item was pre-fetched by middleware.'); - item = fetchedItem.data; - } else { - _logger.finer('Item not pre-fetched, calling _readItem helper.'); - final userIdForRepoCall = _getUserIdForRepoCall( - modelConfig: modelConfig, - permissionService: permissionService, - authenticatedUser: authenticatedUser, - ); - item = await _readItem(context, modelName, id, userIdForRepoCall); - } - - _logger.finer('Item fetch completed. Result: ${item == null ? "null" : "found"}.'); - - // This check is now crucial for preventing the NoSuchMethodError on null. - if (item == null) { - throw NotFoundException('Item with id "$id" not found.'); - } + // The item is guaranteed to be present by the dataFetchMiddleware. + final item = context.read>().data; + _logger.finer('Item was pre-fetched by middleware. Preparing response.'); return ResponseHelper.success( context: context, @@ -192,83 +173,6 @@ String? _getUserIdForRepoCall({ : null; } -/// Encapsulates the logic for reading a single item by its type. -Future _readItem( - RequestContext context, - String modelName, - String id, - String? userId, -) async { - _logger.finer( - 'Executing _readItem for model "$modelName", id "$id", userId: $userId.', - ); - try { - switch (modelName) { - case 'headline': - return await context.read>().read( - id: id, - userId: userId, - ); - case 'topic': - return await context - .read>() - .read(id: id, userId: userId); - case 'source': - return await context.read>().read( - id: id, - userId: userId, - ); - case 'country': - return await context.read>().read( - id: id, - userId: userId, - ); - case 'language': - return await context.read>().read( - id: id, - userId: userId, - ); - case 'user': - return await context - .read>() - .read(id: id, userId: userId); - case 'user_app_settings': - return await context.read>().read( - id: id, - userId: userId, - ); - case 'user_content_preferences': - return await context.read>().read( - id: id, - userId: userId, - ); - case 'remote_config': - return await context.read>().read( - id: id, - userId: userId, - ); - case 'dashboard_summary': - return await context.read().getSummary(); - default: - _logger.warning('Unsupported model type "$modelName" for read operation.'); - throw OperationFailedException( - 'Unsupported model type "$modelName" for read operation.', - ); - } - } catch (e, s) { - _logger.severe( - 'Unhandled exception in _readItem for model "$modelName", id "$id".', - e, - s, - ); - // Re-throw as a standard exception type that the main error handler - // can process into a 500 error, while preserving the original cause. - throw OperationFailedException( - 'An internal error occurred while reading the item: $e', - ); - } -} - /// Encapsulates the logic for updating an item by its type. Future _updateItem( RequestContext context,