Optimize detection for migration to nwtsty #108
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
ToDo: Gomobile |
Instead of returning nil when there are no TagMaps to be merged, it's better to return an empty slice.
AndreasSko
force-pushed
the
migrateBible
branch
5 times, most recently
from
03222a9
to
91cd766
Compare
AndreasSko
force-pushed
the
migrateBible
branch
from
91cd766
to
fa34cc9
Compare
AndreasSko
changed the title
|
ToDo:
|
AndreasSko
force-pushed
the
migrateBible
branch
2 times, most recently
from
3289bc6
to
13b66e6
Compare
The current approach of determining the need of migrating from `nwt` to `nwtsty` (introduced with !46) is using a heuristic, which basically checks if one side has significantly more `nwtsty` entries for a specific language than the other side. This heuristic worked quite well, but can fail if the difference between both sides is not high enough. This can lead to collisions of UserMarkGUIDs. This introduces a new approach, which will check both sides for the same UserMarkGUID. If it finds a common GUID, it will check if the location of one UserMark is in `nwt`, while the other one is in `nwstsy`. If this is the case, it indicates the need for migrating to `nwtsty`, as obviously one side has already been migrated. The logic for the migration is now bundled in kind of a "PreMergeHook", which bundles necessary functionality for preparing the databases for merging.
AndreasSko
force-pushed
the
migrateBible
branch
from
13b66e6
to
933ac69
Compare
The migration from `nwt` to `nwtsty` is working properly for "simple" locations, e.g. bible chapters. However, the locations for appendices have DocumentIDs, which are not easily migratable (we would need to find the documentID corresponding to the `nwstsy`). Therefore, we skip those locations. Skipping the locations, however, introduces the risk of UserMark collisions again, as the merger is using the locationIDs for finding those. As this is a very limited edge-case, I decided to simply remove the duplicate from the old `nwt` side completely. This shouldn't be a problem, as markings on the `nwstsy` can be considered newer.
AndreasSko
force-pushed
the
migrateBible
branch
from
933ac69
to
10ec326
Compare
AndreasSko
added a commit
to AndreasSko/ios-jwlm
that referenced
this pull request
Oct 16, 2021
In some situations it could happen that the merger was not able to detect that one backup had already been migrated to the Study Edition of the Revised New World Translation, while the other one still was using the Regular Edition. This could lead to errors like: > Could not create SQLite database for exporting: [...] UNIQUE constraint failed: UserMark.UserMarkGuid This version improved the detection of such cases, preventing those kinds of errors in the future. More details: AndreasSko/go-jwlm#108
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current approach of determining the need for migrating from
nwttonwtsty(introduced with #46) is using a heuristic, which basically checks if one side has significantly morenwtstyentries for a specific language than the other side.This heuristic worked quite well, but can fail if the difference between both sides is not high enough. This can lead to collisions of UserMarkGUIDs:
This introduces a new approach, which will check both sides for the same UserMarkGUID. If it finds a common GUID, it will check if the location of one UserMark is in
nwt, while the other one is innwstsy. If this is the case, it indicates the need for migrating tonwtsty, as obviously one side has already been migrated.The logic for the migration is now bundled in kind of a "PreMergeHook", which bundles necessary functionality for preparing the databases for merging.
Cleanup userMark duplicates after nwtsty migration
The migration from
nwttonwtstyis working properly for "simple" locations, e.g. Bible chapters. However, the locations for appendices have DocumentIDs, which are not easily migratable (we would need to find the documentID corresponding to thenwstsy). Therefore, we skip those locations.Skipping the locations, however, introduces the risk of UserMark collisions again, as the merger is using the locationIDs for finding those. As this is a very limited edge-case, I decided to simply remove the duplicate from the old
nwtside completely. This shouldn't be a problem, as markings on thenwstsycan be considered newer.closes #105
closes #97
closes AndreasSko/ios-jwlm#36