Skip to content

feat(access): add categories permissions and fix subscribe deadlocks - #197

Open
parfeon wants to merge 2 commits into
masterfrom
docs/CLEN-3779-CLEN-3758-CLEN-3339-readme-categories-abba
Open

parfeon wants to merge 2 commits into
masterfrom
docs/CLEN-3779-CLEN-3758-CLEN-3339-readme-categories-abba

Conversation

@parfeon

@parfeon parfeon commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

feat(access): add categories permissions to grant token

Add ChannelsCategoryPermission and UuidsCategoryPermission, the categories option for
grant_token, and parsing of category permissions in parse_token. Parsed token resources and
patterns are optional now.

fix(subscribe): fix DataStream and dispatcher deadlocks

Fix DataStream deadlock between push_data and poll_next which took queue and waker locks
in opposite order, and EventDispatcher deadlock between stream creation and invalidate.

refactor(client): remove redundant format! references

Remove redundant references in format! arguments of PubNubClientInstance, access and presence
request builders.

build(cargo): update manifest and dev dependencies

Add keywords, enable access feature by default, update wiremock and cucumber, and register
the integration test target.

docs(readme): update PubNub Rust SDK header

Update README.md PubNub Rust SDK header.

test(access): cover category grants and token parsing

Add unit and integration tests for categories-only grants, all permission kinds, token parsing and
malformed tokens.

test(subscribe): cover stream lock order and wake-ups

Add tests for DataStream and EventDispatcher lock ordering, listener wake-ups and delivery of
dispatched batches.

Add `ChannelsCategoryPermission` and `UuidsCategoryPermission`, the `categories` option for
`grant_token`, and parsing of category permissions in `parse_token`. Parsed token `resources` and
`patterns` are optional now.

fix(subscribe): fix `DataStream` and dispatcher deadlocks

Fix `DataStream` deadlock between `push_data` and `poll_next` which took `queue` and `waker` locks
in opposite order, and `EventDispatcher` deadlock between stream creation and `invalidate`.

refactor(client): remove redundant `format!` references

Remove redundant references in `format!` arguments of `PubNubClientInstance`, access and presence
request builders.

build(cargo): update manifest and dev dependencies

Add `keywords`, enable `access` feature by default, update `wiremock` and `cucumber`, and register
the `integration` test target.

docs(readme): update PubNub Rust SDK header

Update `README.md` PubNub Rust SDK header.

test(access): cover category grants and token parsing

Add unit and integration tests for categories-only grants, all permission kinds, token parsing and
malformed tokens.

test(subscribe): cover stream lock order and wake-ups

Add tests for `DataStream` and `EventDispatcher` lock ordering, listener wake-ups and delivery of
dispatched batches.
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 1 medium · 1 minor

Results:
2 new issues

Category Results
BestPractice 1 minor
Complexity 1 medium

View in Codacy

🟢 Metrics 66 complexity · 124 duplication

Metric Results
Complexity 66
Duplication 124

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@Xavrax Xavrax left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall LGTM

Comment thread Cargo.toml Outdated

# Enables all default features
default = ["publish", "subscribe", "serde", "reqwest", "std", "blocking", "tokio"]
default = ["publish", "subscribe", "serde", "reqwest", "std", "blocking", "tokio", "access"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update the readme (so lib.rs) about it, please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was for test purposes and didn't roll it back.

Comment thread src/dx/access/builders/grant_token.rs Outdated

if let Some(categories) = self.categories.flatten() {
categories.iter().try_for_each(|cat| {
if !["channels", "uuids"].contains(&cat.id().as_str()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is cat.id() correct here? Shoouldn't we do something like resource_type() here?

Comment thread src/dx/access/payloads.rs
ResourceType::UserId => {
uuids.insert(perm.id().to_string(), *perm.value());
}
_ => {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we sure we want to have deafult here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But other ResourceType variants can't be used for resource or pattern. The only two which is left can be used only for categories key under permissions.

Comment thread src/dx/access/payloads.rs
///
/// This type used by [`GrantTokenPayload`] to store permissions for specific
/// resource type.
#[cfg_attr(feature = "serde", derive(serde::Serialize))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should always add deriverable types for pub structs.

Suggested change
#[cfg_attr(feature = "serde", derive(serde::Serialize))]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is more pub structs in the file, but very few are compatible with Copy trait because of HashMap, so instead I'm providing maximum possible list for each.

Comment thread src/dx/parse_token.rs Outdated

impl From<u8> for CategoryPermissions {
fn from(int: u8) -> Self {
CategoryPermissions { get: int & 32 != 0 }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand the AND but please keep this 32 with some name.

Gate `data_stream`, request helpers and `blocking` import so narrow feature builds compile.

refactor(parse_token): decouple parse_token from access feature

Move permission bits to `permission_bits` in core utils.
`parse_token` builds without `access`. `access` re-exports the bits.

refactor: address issues reported by PR reviewer

docs(readme): standardize README to SDK template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants