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

Update offsetExists Function to Use Async Cache Check #1263

Closed
wants to merge 1 commit into from

Conversation

valzargaming
Copy link
Member

@valzargaming valzargaming commented Nov 19, 2024

Description

This PR updates the offsetExists function in the AbstractRepository class to enhance functionality by integrating an asynchronous cache check. The previous implementation used a direct parent call for offsetExists. The new implementation leverages the cache component, ensuring a more accurate check of the cache state.

Changes Made

  • Updated offsetExists from: return parent::offsetExists($offset) to return await($this->cache->has($offset)).
  • This change ensures the repository adheres to an asynchronous design pattern and integrates seamlessly with the cache system.

Benefits

  • Adds forward compatibility for the previously deprecated offsetExists function, allowing it to remain functional with the new cache system.

@valzargaming valzargaming requested review from Log1x and Exanlv November 19, 2024 21:16
@bwoebi
Copy link

bwoebi commented Jan 6, 2025

I'm randomly seeing this - and would like to note that this will be quite the breaking change as it would require users to put the offsetExists call into an async callback. At least unless you make all discord event callbacks async too.
Currently awaiting (without wrapping into async) will ultimately stall ratchet in its callbacks, causing the current pending websocket packets to be dequeued after the callback possibly causing out-of-order callback delivery. (and this may cause the connection to become invalid for example)

@valzargaming
Copy link
Member Author

I'm more than a little disappointed that you're the only person to point that out.

@valzargaming valzargaming deleted the AbstractRepository-enhancement branch January 8, 2025 16:04
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