Skip to content

.count use same flow as findMany #2163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions packages/runtime/src/enhancements/node/policy/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type PostWriteCheckRecord = {
preValue?: any;
};

type FindOperations = 'findUnique' | 'findUniqueOrThrow' | 'findFirst' | 'findFirstOrThrow' | 'findMany';
type FindOperations = 'findUnique' | 'findUniqueOrThrow' | 'findFirst' | 'findFirstOrThrow' | 'findMany' | 'count';

/**
* Prisma proxy handler for injecting access policy check.
Expand Down Expand Up @@ -144,6 +144,10 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
this.logger.info(`[policy] \`${actionName}\` ${this.model}:\n${formatObject(_args)}`);
}

if (actionName === 'count') {
_args.select = true;
}

const result = await this.modelClient[actionName](_args);
return this.policyUtils.postProcessForRead(result, this.model, origArgs);
}
Expand Down Expand Up @@ -1650,16 +1654,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
}

count(args: any) {
return createDeferredPromise(() => {
// inject policy conditions
args = args ? this.policyUtils.safeClone(args) : {};
this.policyUtils.injectAuthGuardAsWhere(this.prisma, args, this.model, 'read');

if (this.shouldLogQuery) {
this.logger.info(`[policy] \`count\` ${this.model}:\n${formatObject(args)}`);
}
return this.modelClient.count(args);
});
return createDeferredPromise<unknown[]>(() => this.doFind(args, 'count', () => []));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Incorrect return type and fallback for count operation.

The count method implementation has several issues:

  1. Return type mismatch: Count operations should return a number or count object, not Promise<unknown[]>
  2. Incorrect fallback: The fallback () => [] returns an empty array, but count operations should return 0 when no results match
  3. API incompatibility: Using doFind for count operations may not work correctly with Prisma's count API

Consider this approach instead:

-count(args: any) {
-    return createDeferredPromise<unknown[]>(() => this.doFind(args, 'count', () => []));
-}
+count(args: any): Promise<number> {
+    return createDeferredPromise<number>(async () => {
+        const _args = this.policyUtils.safeClone(args);
+        if (!this.policyUtils.injectForRead(this.prisma, this.model, _args)) {
+            if (this.shouldLogQuery) {
+                this.logger.info(`[policy] \`count\` ${this.model}: unconditionally denied`);
+            }
+            return 0;
+        }
+        
+        if (this.shouldLogQuery) {
+            this.logger.info(`[policy] \`count\` ${this.model}:\n${formatObject(_args)}`);
+        }
+        
+        return this.modelClient.count(_args);
+    });
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return createDeferredPromise<unknown[]>(() => this.doFind(args, 'count', () => []));
count(args: any): Promise<number> {
return createDeferredPromise<number>(async () => {
const _args = this.policyUtils.safeClone(args);
if (!this.policyUtils.injectForRead(this.prisma, this.model, _args)) {
if (this.shouldLogQuery) {
this.logger.info(`[policy] \`count\` ${this.model}: unconditionally denied`);
}
return 0;
}
if (this.shouldLogQuery) {
this.logger.info(`[policy] \`count\` ${this.model}:\n${formatObject(_args)}`);
}
return this.modelClient.count(_args);
});
}
🤖 Prompt for AI Agents
In packages/runtime/src/enhancements/node/policy/handler.ts at line 1657, the
count method incorrectly returns a Promise of an unknown array and uses an empty
array as fallback, which is incompatible with expected count results. To fix
this, change the return type to a number or appropriate count object, replace
the fallback with a function returning 0, and avoid using doFind for count
operations by directly invoking Prisma's count API or a method designed for
counting.

}

//#endregion
Expand Down