Skip to content

Conversation

@laevandus
Copy link
Contributor

🎯 Goal

Make sure that changed managed object contexts do not trigger adding new data in removeAllData

📝 Summary

In the state layer branch we have a case where channel was not properly deleted on logout. That branch also has additional managed object context which is aggressively merging changes from other contexts (this seems to be related).

  1. Log in with A
  2. Open channel with A and B
  3. Logout and log in with B
  4. Open channel with A and B → issue happens (assert telling multiple ChannelDTOs for the same cid)

🛠 Implementation

Reset contexts when we logout for making sure we discard any active changes and do not try to save them when later the code path calls writableContext.save.

🎨 Showcase

From: feature/chat-state-layer
325224767-97655b0b-25bb-4976-9270-dabc040fe7e2

🧪 Manual Testing Notes

Follow the steps above.

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change follows zero ⚠️ policy (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

@laevandus laevandus requested a review from a team as a code owner April 29, 2024 06:31
@Stream-SDK-Bot
Copy link
Collaborator

StreamChat XCMetrics

target metric benchmark branch performance status
MessageList Hitches total duration 10 ms 3.34 ms 66.6% 🔼 🟢
Duration 2.6 s 2.55 s 1.92% 🔼 🟢
Hitch time ratio 4 ms per s 1.31 ms per s 67.25% 🔼 🟢
Frame rate 79 fps 78.66 fps 0.43% 🔼 🟢
Number of hitches 1 0.4 60.0% 🔼 🟢
ChannelList Hitches total duration 12.5 ms 0.0 ms 100.0% 🔼 🟢
Duration 2.6 s 0.83 s 68.08% 🔼 🟢
Hitch time ratio 5 ms per s 0.0 ms per s 100.0% 🔼 🟢
Frame rate 76 fps 62.81 fps 17.36% 🔼 🟢
Number of hitches 1.2 0.0 100.0% 🔼 🟢

Copy link
Contributor

@martinmitrevski martinmitrevski left a comment

Choose a reason for hiding this comment

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

LGTM ✅ Please check before merging the failing e2e tests (cc @testableapple)

@laevandus laevandus force-pushed the fix/clear-context-before-delete branch from 48175e1 to 96cc881 Compare April 29, 2024 09:49
@laevandus
Copy link
Contributor Author

I will hold this one until we have released 4.53.0.

@laevandus laevandus force-pushed the fix/clear-context-before-delete branch from 96cc881 to 33f20ec Compare April 30, 2024 06:03
@laevandus laevandus force-pushed the fix/clear-context-before-delete branch from 33f20ec to 3d28c5a Compare May 2, 2024 06:05
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 2, 2024

@laevandus laevandus merged commit daaa4dd into develop May 2, 2024
@laevandus laevandus deleted the fix/clear-context-before-delete branch May 2, 2024 07:44
@laevandus laevandus mentioned this pull request May 6, 2024
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.

4 participants