Skip to content

Conversation

@matt-ramotar
Copy link
Collaborator

@matt-ramotar matt-ramotar commented Feb 22, 2025

Closes #667
Closes #578

This PR is fixing a deadlock in eager conflict resolution. To improve code quality, this PR also:

  • Expands on RealMutableStore documentation
  • Extracts logging into an internal interface and provides a default implementation to the RealMutableStore
  • Introduces internal test utility classes
  • Adds RealMutableStore unit tests

@codecov
Copy link

codecov bot commented Feb 22, 2025

Codecov Report

Attention: Patch coverage is 78.78788% with 14 lines in your changes missing coverage. Please review.

Project coverage is 79.12%. Comparing base (120d13b) to head (982bf7a).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...vefoundation/store/store5/impl/RealMutableStore.kt 77.58% 6 Missing and 7 partials ⚠️
...ativefoundation/store/store5/impl/DefaultLogger.kt 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   75.43%   79.12%   +3.69%     
==========================================
  Files          38       40       +2     
  Lines         920      939      +19     
  Branches      168      169       +1     
==========================================
+ Hits          694      743      +49     
+ Misses        142      120      -22     
+ Partials       84       76       -8     
Files with missing lines Coverage Δ
.../org/mobilenativefoundation/store/store5/Logger.kt 100.00% <100.00%> (ø)
...ativefoundation/store/store5/impl/DefaultLogger.kt 85.71% <85.71%> (ø)
...vefoundation/store/store5/impl/RealMutableStore.kt 77.37% <77.58%> (+17.05%) ⬆️

... and 4 files with indirect coverage changes

@matt-ramotar matt-ramotar changed the title Fix Eager Conflict Resolution Deadlock Fix Eager Conflict Resolution Deadlock and Improve Mutable Store Code Quality Feb 22, 2025
@digitalbuddha
Copy link
Contributor

nit, can we try to keep the prs single purpose for easier review <3

* @param message The debug message to log.
*/
fun debug(message: String)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

no info?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We're not currently using info and this component isn't currently being exposed to the consumer. There's an open request to improve logging #638. My plan is to replace this then with logging hooks delegated to the user

Copy link
Contributor

@digitalbuddha digitalbuddha left a comment

Choose a reason for hiding this comment

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

thank you for adding tests <3

@matt-ramotar matt-ramotar merged commit 6801344 into main Feb 25, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG] Deadlock in tryEagerlyResolveConflicts [BUG] Deadlock during synchronization in MutableStore

3 participants