Fix: Fixed an issue where Google Drive wasn't displayed when mounted as a folder

[wharvex]

Summary

• Registry-artifact-based Google Drive mounting point detection

  • Add method for reading Google Drive registry artifacts and parsing them as JSON.

  • Add method for safely traversing the resulting JSON and creating/returning new CloudProvider objects based on the data inside.

  • Call these methods after the old approach that uses the "root_preference_sqlite" file. Google Drive now appears under Cloud Drives" in the sidebar regardless of whether it is mounted as a drive or a folder.

• GoogleDriveCloudDetector private fields

  • Add private fields to GoogleDriveCloudDetector (GDCD) for the strings used to access/traverse Google Drive registry artifacts.

  • Add field to GDCD for the logger service so we can log warnings when artifact access/traversal attempts fail.

• Icon-file-getting logic

  • Create method for getting the icon file so we can reuse this logic.

  • Improve null safety of icon file getting logic.

Resolved / Related Issues

• Bug: Google Drive doesn't show in Cloud Drives when mounts as a folder #15698

Steps used to test these changes

1. Under Google Drive Preferences -> Settings -> Google Drive Streaming Location, choose "Folder" or "Drive Letter".

   1. Folder

      1. I opened Files and looked at the sidebar and "Google Drive" was there under "Cloud Drives".

      2. I clicked on "Google Drive" and it opened my drive with all its contents viewable and accessible, as expected. I opened a few of my files (images and pdfs) and they opened as expected.

   2. Drive Letter

      1. I opened Files and looked at the sidebar and "Google Drive" was there under "Cloud Drives".

      2. I clicked on "Google Drive" and it opened my drive with all its contents viewable and accessible, as expected. I opened a few of my files (images and pdfs) and they opened as expected.

2. I also found that everything worked exactly the same if I had the entire old approach that uses the "root_preference_sqlite" file commented out. However, I did not want to commit it in this state because my testing was not especially thorough, but it might be worth looking into whether this will work now entirely without the old approach.

[0x5bfa]

Thank you for the contribution.
Is this ready?

[wharvex]

@0x5bfa No problem. I would say it's ready to be merged in the sense that it seems to be working based on the steps I followed in the "Steps used to test these changes" section above.

However, I just wanted to get a second opinion on the code I wrote on lines 191-196.

When Google Drive is mounted as a folder, the value stored in mount_point_path will be a proper path that can be passed to a method like StorageFolder.GetFolderFromPathAsync without issue.

But when Google Drive is mounted as a drive, the value stored in mount_point_path is JUST the drive letter (e.g. "G"), and as such causes an error when passed to a method like StorageFolder.GetFolderFromPathAsync.

The solution I came up with for this issue was to check if the value stored in mount_point_path is exactly one character in length, and if it is, get a DriveInfo object for that character and get the proper useable path from that object.

The effectiveness/safety of this solution depends on two assumptions:

• If the value stored in mount_point_path is one character in length, it must be a drive letter.
• This drive letter will be a valid drive (if it's not, the DriveInfo constructor will throw an ArgumentException).

My particular solution is what allows us to get a useable path from the registry-artifact method regardless of Google Drive's mount point, and therefore what would allow us to potentially remove the old "root_preference_sqlite" mount point detection method if we wanted.

So I was just curious what your team's thoughts might be on my approach and on this issue in general.

[wharvex]

I just made a new commit that makes the logic that gets the valid path for a single-character mount_point_path value a little safer.

In my opinion, it is now ready to merge. But please let me know if there is anything you want me to change.

[Josh65-2201]

I found two minor issues, Google Drive set to mirror to a user selected folder will show both the redirect drive/folder (This show one My Drive folder) and another that goes directly to the store folder location.
Preferably it would only show the redirect drive/folder and add /My Drive to the path, then hide all other locations. This would make it consistently open the direct files in both stream and mirror modes.

Also the Google drive is now always shown in the drives section instead of being hidden like 3.5.6

[wharvex]

I found two minor issues, Google Drive set to mirror to a user selected folder will show both the redirect drive/folder (This show one folder that redirects to the store location) and another that goes directly to the store location. Preferably it would only show the direct folder in mirror mode. or it can only show the streaming drive/folder and add the /My Drive then hide all other locations. This would make it consistently open the direct files in both stream and mirror modes.

Also the Google drive is now always shown in the drives section instead of being hidden like 3.5.6

Thank you for noticing this, I'll take a look at it tonight or this weekend. And I'm sorry but can you please let me know what "3.5.6" refers to?

[Josh65-2201]

3.5.6 is the current preview version

[wharvex]

@Josh65-2201 , I looked into the first issue today. I am still working on it, but I just wanted to share what I've found.

I took screenshots of the sidebar as it appears on my computer under the following conditions (all with my Google Drive set to mirror files):

1. With my code commented-out and with Google Drive mounted as a drive
   
2. With my code active and with Google Drive mounted as a drive
   
3. With my code commented-out and with Google Drive mounted as a folder
   
4. With my code active and with Google Drive mounted as a folder
   

I am thinking of changing my code so that it appends "My Drive" to the path I get from the registry, so it will be weeded out as a duplicate DriveItem in CloudDrivesManager.UpdateDrivesAsync() if a DriveItem with Path set to the "My Drive" path was already added by the Sqlite detection approach. Or please let me know if you would prefer a different solution.
I just realized this won't work because the path I get from the registry is the streaming path, and when Google Drive is set to mirror and mount streaming as a folder, the Sqlite "last_seen_absolute_path" column contains the mirror (non-streaming) path. I will look into other solutions this week and update the thread with what I find.

As for the second issue, under what conditions is Google Drive hidden in 3.5.6?
Are you saying it's an issue that it's no longer hidden under these conditions?
My understanding was that my pull request should result in Google Drive no longer being hidden from the drives section when Google Drive is mounted as a folder.

[Josh65-2201]

Google drive should always be hidden from the drives section and only show in the cloud drives section.

[wharvex]

Google drive should always be hidden from the drives section and only show in the cloud drives section.

Ah okay, I will look into that as well, thank you.

[wharvex]

Sorry for the delay, I'm still investigating how to tell from the Sqlite database tables ("roots" and "media") that Google Drive is mirrored as opposed to just streaming. I'm hoping to have some real time to work on it this weekend.

In the meantime, I just wanted to ask, did it cause any issues that I updated the branch the other day? I just clicked the "Update branch" button on GitHub on this PR and then pulled the changes down to my local branch. But I just noticed there is a notification now that @yaira2 "force-pushed the main branch." Was this to fix something I did? And is the way I updated the branch (with a merge commit as opposed to rebase) the preferred way?

[yaira2]

In the meantime, I just wanted to ask, did it cause any issues that I updated the branch the other day? I just clicked the "Update branch" button on GitHub on this PR and then pulled the changes down to my local branch. But I just noticed there is a notification now that @yaira2 "force-pushed the main branch." Was this to fix something I did? And is the way I updated the branch (with a merge commit as opposed to rebase) the preferred way?

It's unrelated.. That said, we also have protections enabled, so you don't have to worry about accidently pushing to main.

[wharvex]

Just to give an update, today I figured out how to create logs in separate files of the database inspections I've been doing.

I have made a commit to my GoogleDriveDetection branch with this logging in place.

I plan to remove this logging once I determine and can demonstrate:

A) what this implementation of GetProviders "yield returns",

B) what the contents of various Google Drive artifacts are (e.g. registry keys and Sqlite databases), and

C) whether any of these artifacts contain useful information for determining user preference configurations that tell us when we should or shouldn't yield return certain paths.

I plan to use the logging to gather data related to A, B, and C under these conditions:

My code active; Streaming; folder-mounted
My code active; Streaming; drive-mounted
My code active; Mirrored; folder-mounted
My code active; Mirrored; drive-mounted
My code inactive; Streaming; folder-mounted
My code inactive; Streaming; drive-mounted
My code inactive; Mirrored; folder-mounted
My code inactive; Mirrored; drive-mounted

I also learned about the existence of a "mirror_metadata_sqlite.db" file from this article, it might be useful to try to lookup this database as well.

[yaira2]

Thanks for the update.

[wharvex]

With the two commits that I just made, I think that the first issue Josh
mentioned has been fixed (see my copied commit message below).

I have also confirmed that when running the main branch, that Google Drive still
appears under Drives in the sidebar (which is the second issue Josh mentioned).
As such, I think this issue is not caused by getting Google Drive paths from the registry in
GoogleDriveCloudDetector#GetProviders. I would be happy to investigate this
issue, but maybe in a separate PR.

I still have logging in place in case you want to use it to confirm the current
behavior of everything. Let me know if you think it looks good and I'll make a
new commit removing all the logging.

Begin copied commit messages:

AddMyDriveToPathAndValidate; is_my_drive

Renamed ValidatePath to AddMyDriveToPathAndValidate, and made it also append
`My Drive' to the path it is given.

The only path we ever get from the media table is the drive letter
(if Google Drive streaming location is set to Drive letter, otherwise we get nothing
from the media table) followed by :\. As such, I added a call to
`AddMyDriveToPathAndValidate' on this path.

We only find paths in the roots table whose is_my_drive is true when My Drive syncing options is set to Mirror files. I believe these paths are the ones @Josh65-2201
was referring to when he said

Google Drive set to mirror to a user selected folder will show both the
redirect drive/folder (This show one My Drive folder) and another that goes
directly to the store folder location.

The path we get from roots is the one that "goes directly to the store folder
location" (hence its is_my_drive is true).

With this PR, we now get the "redirect drive/folder" as well, from the registry.
As such, since Josh said

Preferably it would only show the redirect drive/folder and add /My Drive to
the path, then hide all other locations

I added a call to AddMyDriveToPathAndValidate on every path we get from the
registry, and I made the roots table read loop continue if it finds a path
whose is_my_drive is true.

my drive shortcut fix; registry logging

Handled the case where My Drive is a shortcut (when "My Drive syncing options"
is set to "Mirror files").

Added logging for registry values.

[wharvex]

I just brought the fork up-to-date with the main repo and made a few minor formatting changes. The CI was failing at first, it was that same issue with the Axe accessibility test with sibling elements, and then there was an x64 Release build fail for some reason (it wouldn't say why), but now everything seems good.

Please let me know if you want me to change anything else.

[0x5bfa]

Code wise, LGTM.
I haven't checked behavior yet.

[Josh65-2201]

Still works as expected since my last review

[wharvex]

Thanks for checking.

[wharvex]

Is there anything I can do to help with testing? I made this screen recording of a test I did just now running the repo with my changes under four different config conditions, just to document how I ran it and what I looked for.

https://drive.google.com/file/d/1bUYS0T3HQcxmzROW_wlNlP8zQjxze035/view?usp=sharing