Bug 2053557 - remove all logins without decrypting#7467
Conversation
bendk
left a comment
bendk
left a comment
There was a problem hiding this comment.
This looks good to me, I just had a question about one of the queries
| let ids: Vec<String> = self.db.query_rows_and_then_cached( | ||
| "SELECT guid FROM loginsL WHERE is_deleted = 0 | ||
| UNION ALL | ||
| SELECT guid FROM loginsM WHERE is_overridden = 0", |
There was a problem hiding this comment.
Why do you need the second query? I would have thought the loginsL one would be enough. I'm guessing I'm missing something, could you add a comment explaining. The same applies to delete_all_except_fxa.
There was a problem hiding this comment.
This is a very good question which I have to admin I cannot answer. I looked at the get_all query and replicated it for the needs here. Throughout the code we union against both tables. 🤔
There was a problem hiding this comment.
I think that's the way logins work: https://github.com/mozilla/application-services/blob/main/components/logins/src/db.rs#L7-L24
When we want to fetch a record, we need to look in both loginsL and loginsM for the data.
There was a problem hiding this comment.
Ahh, so if a record isn't updated then there won't be a row in loginsL. I always forget that and picture it working the opposite way. This all makes sense and seems good to merge to me.
There was a problem hiding this comment.
yeah, this bites me regularly too. It's "clever", but borders on "too clever" 😅
3 participants
https://bugzilla.mozilla.org/show_bug.cgi?id=2053557
Pull Request checklist
[ci full]to the PR title.