Skip to content

Commit 5fb0d35

Browse files
fullenifulleni
authored
Merge pull request #37 from flutter-news-app-full-source-code/enhance-the-data-route-crud
Enhance the data route crud
2 parents fda3b2e + 6af09ef commit 5fb0d35

File tree

1 file changed

+64
-170
lines changed

1 file changed

+64
-170
lines changed

‎routes/api/v1/data/[id]/index.dart‎

Lines changed: 64 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -75,92 +75,63 @@ Future<Response> _handleGet(
7575
User authenticatedUser,
7676
PermissionService permissionService,
7777
) async {
78-
// Authorization check is handled by authorizationMiddleware before this.
79-
// This handler only needs to perform the ownership check if required.
78+
// Authorization and ownership checks are handled by middleware before this.
79+
// This handler's job is to fetch and return the data.
8080

8181
dynamic item;
8282

83-
// Determine userId for repository call based on ModelConfig (for data scoping)
84-
String? userIdForRepoCall;
85-
// If the model is user-owned, pass the authenticated user's ID to the repository
86-
// for filtering. Otherwise, pass null.
87-
// Note: This is for data *scoping* by the repository, not the permission check.
88-
// We infer user-owned based on the presence of getOwnerId function.
89-
if (modelConfig.getOwnerId != null &&
90-
!permissionService.isAdmin(authenticatedUser)) {
91-
userIdForRepoCall = authenticatedUser.id;
92-
} else {
93-
userIdForRepoCall = null;
94-
}
83+
// Check if the ownership middleware already fetched the item for a check.
84+
// This avoids a redundant database call.
85+
final fetchedItem = context.read<FetchedItem<dynamic>?>();
9586

96-
// Repository exceptions (like NotFoundException) will propagate up to the
97-
// main onRequest try/catch (which is now removed, so they go to errorHandler).
98-
switch (modelName) {
99-
case 'headline':
100-
final repo = context.read<DataRepository<Headline>>();
101-
item = await repo.read(id: id, userId: userIdForRepoCall);
102-
case 'topic':
103-
final repo = context.read<DataRepository<Topic>>();
104-
item = await repo.read(id: id, userId: userIdForRepoCall);
105-
case 'source':
106-
final repo = context.read<DataRepository<Source>>();
107-
item = await repo.read(id: id, userId: userIdForRepoCall);
108-
case 'country':
109-
final repo = context.read<DataRepository<Country>>();
110-
item = await repo.read(id: id, userId: userIdForRepoCall);
111-
case 'language':
112-
final repo = context.read<DataRepository<Language>>();
113-
item = await repo.read(id: id, userId: userIdForRepoCall);
114-
case 'user': // Handle User model specifically if needed, or rely on generic
115-
final repo = context.read<DataRepository<User>>();
116-
item = await repo.read(id: id, userId: userIdForRepoCall);
117-
case 'user_app_settings': // New case for UserAppSettings
118-
final repo = context.read<DataRepository<UserAppSettings>>();
119-
item = await repo.read(id: id, userId: userIdForRepoCall);
120-
case 'user_content_preferences': // New case for UserContentPreferences
121-
final repo = context.read<DataRepository<UserContentPreferences>>();
122-
item = await repo.read(id: id, userId: userIdForRepoCall);
123-
case 'remote_config': // New case for RemoteConfig (read by admin)
124-
final repo = context.read<DataRepository<RemoteConfig>>();
125-
item = await repo.read(
126-
id: id,
127-
userId: userIdForRepoCall,
128-
); // userId should be null for AppConfig
129-
case 'dashboard_summary':
130-
final service = context.read<DashboardSummaryService>();
131-
item = await service.getSummary();
132-
default:
133-
// This case should ideally be caught by middleware, but added for safety
134-
// Throw an exception to be caught by the errorHandler
135-
throw OperationFailedException(
136-
'Unsupported model type "$modelName" reached handler.',
137-
);
138-
}
139-
140-
// --- Handler-Level Ownership Check (for GET item) ---
141-
// This check is needed if the ModelConfig for GET item requires ownership
142-
// AND the user is NOT an admin (admins can bypass ownership checks).
143-
if (modelConfig.getItemPermission.requiresOwnershipCheck &&
144-
!permissionService.isAdmin(authenticatedUser)) {
145-
// Ensure getOwnerId is provided for models requiring ownership check
146-
if (modelConfig.getOwnerId == null) {
147-
_logger.severe(
148-
'Configuration Error: Model "$modelName" requires '
149-
'ownership check for GET item but getOwnerId is not provided.',
150-
);
151-
// Throw an exception to be caught by the errorHandler
152-
throw const OperationFailedException(
153-
'Internal Server Error: Model configuration error.',
154-
);
155-
}
156-
157-
final itemOwnerId = modelConfig.getOwnerId!(item);
158-
if (itemOwnerId != authenticatedUser.id) {
159-
// If the authenticated user is not the owner, deny access.
160-
// Throw ForbiddenException to be caught by the errorHandler
161-
throw const ForbiddenException(
162-
'You do not have permission to access this specific item.',
163-
);
87+
if (fetchedItem != null) {
88+
// If the item was pre-fetched by the middleware, use it directly.
89+
item = fetchedItem.data;
90+
} else {
91+
// If no ownership check was required (e.g., for an admin or public
92+
// resource), the item was not pre-fetched, so we fetch it now.
93+
final userIdForRepoCall =
94+
(modelConfig.getOwnerId != null &&
95+
!permissionService.isAdmin(authenticatedUser))
96+
? authenticatedUser.id
97+
: null;
98+
99+
// Repository exceptions (like NotFoundException) will propagate up.
100+
switch (modelName) {
101+
case 'headline':
102+
final repo = context.read<DataRepository<Headline>>();
103+
item = await repo.read(id: id, userId: userIdForRepoCall);
104+
case 'topic':
105+
final repo = context.read<DataRepository<Topic>>();
106+
item = await repo.read(id: id, userId: userIdForRepoCall);
107+
case 'source':
108+
final repo = context.read<DataRepository<Source>>();
109+
item = await repo.read(id: id, userId: userIdForRepoCall);
110+
case 'country':
111+
final repo = context.read<DataRepository<Country>>();
112+
item = await repo.read(id: id, userId: userIdForRepoCall);
113+
case 'language':
114+
final repo = context.read<DataRepository<Language>>();
115+
item = await repo.read(id: id, userId: userIdForRepoCall);
116+
case 'user':
117+
final repo = context.read<DataRepository<User>>();
118+
item = await repo.read(id: id, userId: userIdForRepoCall);
119+
case 'user_app_settings':
120+
final repo = context.read<DataRepository<UserAppSettings>>();
121+
item = await repo.read(id: id, userId: userIdForRepoCall);
122+
case 'user_content_preferences':
123+
final repo = context.read<DataRepository<UserContentPreferences>>();
124+
item = await repo.read(id: id, userId: userIdForRepoCall);
125+
case 'remote_config':
126+
final repo = context.read<DataRepository<RemoteConfig>>();
127+
item = await repo.read(id: id, userId: userIdForRepoCall);
128+
case 'dashboard_summary':
129+
final service = context.read<DashboardSummaryService>();
130+
item = await service.getSummary();
131+
default:
132+
throw OperationFailedException(
133+
'Unsupported model type "$modelName" reached handler.',
134+
);
164135
}
165136
}
166137

@@ -455,94 +426,17 @@ Future<Response> _handleDelete(
455426
User authenticatedUser,
456427
PermissionService permissionService,
457428
) async {
458-
// Authorization check is handled by authorizationMiddleware before this.
459-
// This handler only needs to perform the ownership check if required.
460-
461-
// Determine userId for repository call based on ModelConfig (for data scoping/ownership enforcement)
462-
String? userIdForRepoCall;
463-
// If the model is user-owned, pass the authenticated user's ID to the repository
464-
// for ownership enforcement. Otherwise, pass null.
465-
if (modelConfig.getOwnerId != null &&
466-
!permissionService.isAdmin(authenticatedUser)) {
467-
userIdForRepoCall = authenticatedUser.id;
468-
} else {
469-
userIdForRepoCall = null;
470-
}
471-
472-
// --- Handler-Level Ownership Check (for DELETE) ---
473-
// For DELETE, we need to fetch the item *before* attempting deletion
474-
// to perform the ownership check if required.
475-
dynamic itemToDelete;
476-
if (modelConfig.deletePermission.requiresOwnershipCheck &&
477-
!permissionService.isAdmin(authenticatedUser)) {
478-
// Ensure getOwnerId is provided for models requiring ownership check
479-
if (modelConfig.getOwnerId == null) {
480-
_logger.severe(
481-
'Configuration Error: Model "$modelName" requires '
482-
'ownership check for DELETE but getOwnerId is not provided.',
483-
);
484-
// Throw an exception to be caught by the errorHandler
485-
throw const OperationFailedException(
486-
'Internal Server Error: Model configuration error.',
487-
);
488-
}
489-
// Fetch the item to check ownership. Use userIdForRepoCall for scoping.
490-
// Repository exceptions (like NotFoundException) will propagate up to the errorHandler.
491-
switch (modelName) {
492-
case 'headline':
493-
final repo = context.read<DataRepository<Headline>>();
494-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
495-
case 'topic':
496-
final repo = context.read<DataRepository<Topic>>();
497-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
498-
case 'source':
499-
final repo = context.read<DataRepository<Source>>();
500-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
501-
case 'country':
502-
final repo = context.read<DataRepository<Country>>();
503-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
504-
case 'language':
505-
final repo = context.read<DataRepository<Language>>();
506-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
507-
case 'user':
508-
final repo = context.read<DataRepository<User>>();
509-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
510-
case 'user_app_settings': // New case for UserAppSettings
511-
final repo = context.read<DataRepository<UserAppSettings>>();
512-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
513-
case 'user_content_preferences': // New case for UserContentPreferences
514-
final repo = context.read<DataRepository<UserContentPreferences>>();
515-
itemToDelete = await repo.read(id: id, userId: userIdForRepoCall);
516-
case 'remote_config': // New case for RemoteConfig (delete by admin)
517-
final repo = context.read<DataRepository<RemoteConfig>>();
518-
itemToDelete = await repo.read(
519-
id: id,
520-
userId: userIdForRepoCall,
521-
); // userId should be null for AppConfig
522-
default:
523-
_logger.severe(
524-
'Unsupported model type "$modelName" reached _handleDelete ownership check.',
525-
);
526-
// Throw an exception to be caught by the errorHandler
527-
throw OperationFailedException(
528-
'Unsupported model type "$modelName" reached handler.',
529-
);
530-
}
531-
532-
// Perform the ownership check if the item was found
533-
if (itemToDelete != null) {
534-
final itemOwnerId = modelConfig.getOwnerId!(itemToDelete);
535-
if (itemOwnerId != authenticatedUser.id) {
536-
// If the authenticated user is not the owner, deny access.
537-
// Throw ForbiddenException to be caught by the errorHandler
538-
throw const ForbiddenException(
539-
'You do not have permission to delete this specific item.',
540-
);
541-
}
542-
}
543-
// If itemToDelete is null here, it means the item wasn't found during the read.
544-
// The subsequent delete call will likely throw NotFoundException, which is correct.
545-
}
429+
// Authorization and ownership checks are handled by the middleware.
430+
// The `ownershipCheckMiddleware` has already verified that the user is
431+
// the owner if required. This handler's only job is to perform the deletion.
432+
433+
// Determine the userId for the repository call. For non-admins, this
434+
// provides an additional layer of security at the database level.
435+
final userIdForRepoCall =
436+
(modelConfig.getOwnerId != null &&
437+
!permissionService.isAdmin(authenticatedUser))
438+
? authenticatedUser.id
439+
: null;
546440

547441
// Allow repository exceptions (e.g., NotFoundException) to propagate
548442
// upwards to be handled by the standard error handling mechanism.

0 commit comments

Comments
 (0)