Add ValkeyModule_ACLCheckCommandPermissionsForCurrentUser API

[rjd15372]

Introduces a new module API function that allows modules to check if a command can be executed by the current module context user according to ACL rules.

This function provides a convenient wrapper around the existing ValkeyModule_ACLCheckCommandPermissions API by automatically using the user associated with the module context, eliminating the need for modules to manually retrieve the user.

This addition simplifies ACL permission checking for modules that need to validate commands against the current context user.

[codecov]

Codecov Report

❌ Patch coverage is 7.69231% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.54%. Comparing base (0646749) to head (f65208b).
⚠️ Report is 5 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/module.c	7.69%	12 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2690      +/-   ##
============================================
+ Coverage     72.18%   72.54%   +0.35%     
============================================
  Files           128      128              
  Lines         71037    71254     +217     
============================================
+ Hits          51277    51690     +413     
+ Misses        19760    19564     -196     

Files with missing lines	Coverage Δ
src/module.c	9.77% <7.69%> (-0.01%)	⬇️

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dvkashapov]

Awesome! Definitely useful in terms of #2309 :)

[zuiderkwast]

This looks like a convenience function equivalent (modulo minor details) to this code:

    ValkeyModuleString *username = ValkeyModule_GetCurrentUserName(ctx);
    ValkeyModuleUser *user = ValkeyModule_GetModuleUserFromUserName(username);
    return ValkeyModule_ACLCheckCommandPermissions(user, argv, argc);

Is this correct? If it's already possible to do this from a module, then why add this? Performance or just convenience?

[rjd15372]

This looks like a convenience function equivalent (modulo minor details) to this code:

    ValkeyModuleString *username = ValkeyModule_GetCurrentUserName(ctx);
    ValkeyModuleUser *user = ValkeyModule_GetModuleUserFromUserName(username);
    return ValkeyModule_ACLCheckCommandPermissions(user, argv, argc);

Is this correct? If it's already possible to do this from a module, then why add this? Performance or just convenience?

It's correct, but the ValkeyModule_GetModuleUserFromUserName function allocates memory, which will introduce a performance penalty in the lua code that calls commands.
Therefore, it's not just for convenience, it's mainly about performance.

[zuiderkwast]

This looks like a convenience function equivalent (modulo minor details) to this code:

    ValkeyModuleString *username = ValkeyModule_GetCurrentUserName(ctx);
    ValkeyModuleUser *user = ValkeyModule_GetModuleUserFromUserName(username);
    return ValkeyModule_ACLCheckCommandPermissions(user, argv, argc);

Is this correct? If it's already possible to do this from a module, then why add this? Performance or just convenience?

It's correct, but the ValkeyModule_GetModuleUserFromUserName function allocates memory, which will introduce a performance penalty in the lua code that calls commands. Therefore, it's not just for convenience, it's mainly about performance.

OK, good to know.

Maybe it's not a bottleneck but temporary allocations like this are always annoying. It's a sign of a suboptimal API, I suppose.

[zuiderkwast]

This LGTM

[madolson]

This entire API is because we're trying to bypass the allocation and do it on the stack here. When the API was written, we cared more about abstraction then performance. If we think performance is so critical, I would rather just ditch the abstraction and directly return the user from the module APIs.

Glancing through the code I don't see any incompatibility with that.

[rjd15372]

Are you proposing to make ValkeyModuleUser a type alias to user?

In that case we would probably need to add a field in the user struct to distinguish between users created by modules, and users created in the core.

[madolson]

In that case we would probably need to add a field in the user struct to distinguish between users created by modules, and users created in the core.

Yes. We don't expect a large number of users, so adding an extra field should be safe. I think we even have a bit field we could use if we wanted to avoid adding any memory at all.