search

sillsdev / LfMerge

Send/Receive for languageforge.org
MIT License
2 stars 4 forks source link

OptionList items use GUID as key (plus debug!) #313

Open josephmyers opened 2 years ago

josephmyers commented 2 years ago

LfOptionListItem's use the GUID from LCM as their Key, instead of the abbreviation value. This fixes a bug where items without abbreviations weren't being synced. This change requires MongoDB data migration, which is performed just-in-time as part of this PR, and it also requires changes in Language Forge, which at time of writing haven't been done yet.

Please see checkin comments for detailed information regarding each checkin.

github-actions[bot] commented 2 years ago

Test Results

    2 files    20 suites   1m 23s :stopwatch: 312 tests 261 :heavy_check_mark: 20 :zzz: 31 :x: 315 runs  264 :heavy_check_mark: 20 :zzz: 31 :x:

For more details on these failures, see this check.

Results for commit 23a26f17.

:recycle: This comment has been updated with latest results.

josephmyers commented 1 year ago

What I'm struggling with today is the BSON "automagic" deserialization from MongoDB. A while back I changed multi-option fields like SemanticDomain to List<LfOptionListItem> to accurately represent the UI. This made the refactor less widespread, and everything was working fine.

However, after I ran "docker compose down -v" yesterday to get to a cleaner state, I have been getting critical errors during initial deserialization. MongoDB is complaining it Cannot deserialize a 'List<LfOptionListItem>' from BsonType 'Document'. This doesn't make a ton of sense, given that it is fine with LfOptionListItem's right above and with List's of LF classes right below.

It's an annoying sidetrack, and I am pretty confident it will prevent me from getting to a clean stopping point before my last day.

josephmyers commented 1 year ago

I just remembered that LF can add list options from the settings :(

This will be fine if LF is allowed to push new options with predefined GUID's to LCM, but if not the simplest thing to do would be to disallow the option add feature in LF. We have precedent for this.

josephmyers commented 1 year ago

Just hit another problem, ugh. This task!

For new projects, the MongoDB is getting partially populated before any initialization has happened. There is one option list, with items, but their GUID's are null. The screenshot below shows the DB just after LF started creating the project and just before LFMerge calls MongoConnection.Initialize()

image

Right after initial clone, we call the action TransferLcmToMongo to init the DB. But within that action (ConvertLcmToMongoLexicon.ConvertOptionListFromLcm) we are polling the DB in order to update the DB. So TransferLcmToMongo assumes you have a complete MongoDB usable as a baseline. So TransferLcmToMongo is effectively DB->LF->LCM->DB, instead of LCM->DB.

The best solution in regards to the integrity of the data would be for the action to simply do LCM->DB. It seems safe to assume that initial project setup would give you a clean DB straight from LCM. It's not clear why we're currently depending on the DB to set up the DB.

megahirt commented 1 year ago

I just remembered that LF can add list options from the settings :(

This will be fine if LF is allowed to push new options with predefined GUID's to LCM, but if not the simplest thing to do would be to disallow the option add feature in LF. We have precedent for this.

Actually, we just disabled adding option list items in a recent PR. So shouldn't be an issue. https://github.com/sillsdev/web-languageforge/issues/1552

megahirt commented 1 year ago

Just hit another problem, ugh. This task!

For new projects, the MongoDB is getting partially populated before any initialization has happened. There is one option list, with items, but their GUID's are null.

It seems reasonable to me to just remove the partial option list. We only support projects that come from flex initially so that code is essentially just getting in the way. A PR against web language Forge to remove the initialized option list would fix this problem

josephmyers commented 1 year ago

Just hit another problem, ugh. This task! For new projects, the MongoDB is getting partially populated before any initialization has happened. There is one option list, with items, but their GUID's are null.

It seems reasonable to me to just remove the partial option list. We only support projects that come from flex initially so that code is essentially just getting in the way. A PR against web language Forge to remove the initialized option list would fix this problem

Oh that is interesting. I was super stumped as to what was adding that.