Skip to content
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

backend: Pass handle token to method calls for easier identification and association with their backend logic #252

Merged
merged 4 commits into from
Dec 24, 2024

Conversation

arun-mani-j
Copy link
Contributor

In the current state of ashpd::backend, it is difficult to associate each request with the backend logic. So any further calls on the request is left in ambiguity.

For example, let's say two apps call Account.GetUserInformation. Our implementation shows dialogs that lets the user enter their details. Now, if one of these apps cancel their request with Request.close(), then it is impossible (or difficult) for implementer to know which dialog must be closed.

An easy fix to this problem is to pass the object path of request on the method calls. Now the backend can associate each request with its backend logic, like say a hash map of requests to dialogs. Then, when close method is called, it can identify which dialog to close and can close it.

I'm a Rust newbie, so not sure if clone()ing paths are a good idea or if we can tackle this problem from a different angle. 😅
Thank you

@arun-mani-j arun-mani-j changed the title backend: Pass object path to method calls for easier identification and association with their backend logic backend: Pass handle token to method calls for easier identification and association with their backend logic Dec 19, 2024
@arun-mani-j
Copy link
Contributor Author

v1:

  • Change from using object path to handle token.

src/desktop/mod.rs Outdated Show resolved Hide resolved
@bilelmoussaoui
Copy link
Owner

HandleToken also needs to derive various traits like Hash, PartialEq,Eq and so on

Copy link
Owner

@bilelmoussaoui bilelmoussaoui left a comment

Choose a reason for hiding this comment

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

otherwise lgtm

src/desktop/handle_token.rs Outdated Show resolved Hide resolved
src/backend/print.rs Show resolved Hide resolved
Will be used in subsequent commits for situations where we need to
identify a `Request` but passing `ObjectPath` directly will be too much
of exposure. Since handle tokens are supposed to be as unique as possible as
per the specifications, we shall use them for these situations.
@arun-mani-j
Copy link
Contributor Author

v2:

  • HandleToken:
    • Derive Eq, PartialEq and Hash on backend feature.
    • Use .expect while converting from path.
    • Fix clippy warning on used imports on non-backend.
  • Desktop:
    • Export HandleToken only on backend feature.
  • Access and Print of Backend:
    • Silence Clippy warning of too many arguments.
  • Tree:
    • Perform indent in the same commit.

By having a handle token, an implementer can now associate further calls
on the `Request` with its backend logic.

For example, suppose an app A calls `Account.GetUserInformation()`
followed by an app B. The implementer shows a dialog for each of them
that lets the user give their response. Now, if any of these apps call
`Request.close()`, then the implementer does not know which dialog to
close, as it did not have any way to associate the `Request` to its
backend logic in the first place.

This commit solves this issue.
This lets the implementer to know on which `Request` `close` was called.
@arun-mani-j
Copy link
Contributor Author

v3:

  • Access and Print of backend:
    • Silence clippy only for the method.

@bilelmoussaoui bilelmoussaoui merged commit d8965dd into bilelmoussaoui:master Dec 24, 2024
@bilelmoussaoui
Copy link
Owner

Thanks!
Won't release this anytime soon as it contains breaking API changes. Feel free to use ashpd from git and find what other missing/broken things in ashpd that would make implementing the posh portals easier.

@arun-mani-j
Copy link
Contributor Author

Thank you!
We are iterating the plan for Phosh's new portal in Rust at https://gitlab.gnome.org/arun-mani-j/xdg-desktop-portal-phosh/-/merge_requests/1.
Thanks again for the support and ashpd :)

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