Skip to content

Commit 13c1219

Browse files
fullenifulleni
committed
refactor(data): optimize item fetching logic in GET request handler
- Remove inline userIdForRepoCall logic and move to a more concise version - Implement a check for pre-fetched items to avoid redundant database calls - Simplify model fetching logic with a more compact switch case structure - Update comments to reflect the current logic flow and responsibilities
1 parent 65f900b commit 13c1219

File tree

1 file changed

+53
-82
lines changed

1 file changed

+53
-82
lines changed

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

Lines changed: 53 additions & 82 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-
}
95-
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-
}
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>?>();
13986

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

0 commit comments

Comments
 (0)