Skip to content

OmhStorageClient: add resolvePath() function#106

Merged
dzuluaga merged 2 commits intomainfrom
feature/resolve-path
Apr 23, 2025
Merged

OmhStorageClient: add resolvePath() function#106
dzuluaga merged 2 commits intomainfrom
feature/resolve-path

Conversation

@TranceLove
Copy link
Copy Markdown
Collaborator

@TranceLove TranceLove commented Oct 30, 2024

Summary

Adds resolvePath() function, to conveniently resolve a folder/file's node ID on cloud storage using their virtual path on the cloud storage.

Also updated FileViewer sample app to showcase use of folder size and storage quota information previously submitted in #97 and #98.

Demo

N/A (resolvePath() usually won't use directly in UI interaction)

Checklist:

  • Documentation is up to date to reflect these changes
  • Created Unit tests

@TranceLove TranceLove force-pushed the feature/resolve-path branch from cff8705 to 19a50e0 Compare November 2, 2024 22:17
@TranceLove TranceLove marked this pull request as ready for review November 3, 2024 13:06
@dzuluaga dzuluaga requested a review from Copilot April 15, 2025 22:25
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 30 out of 33 changed files in this pull request and generated no comments.

Files not reviewed (3)
  • apps/storage-sample/src/main/res/layout/fragment_file_viewer.xml: Language not supported
  • apps/storage-sample/src/main/res/layout/row_dialog_header.xml: Language not supported
  • apps/storage-sample/src/main/res/values/strings.xml: Language not supported
Comments suppressed due to low confidence (1)

packages/plugin-dropbox/src/main/java/com/openmobilehub/android/storage/plugin/dropbox/data/service/DropboxApiService.kt:267

  • Ensure that 'node' is definitely one of FolderMetadata or FileMetadata before casting. If an unexpected Metadata type is encountered, consider adding an explicit type check or fallback to avoid a potential ClassCastException.
return (node as? FolderMetadata)?.id ?: (node as FileMetadata).id

@dzuluaga
Copy link
Copy Markdown
Contributor

@TranceLove This looks good. Some nitpicks from Copilot. Could you please address the last comment?

Also, could you resolve the conflict in getting-started.md?

And nevermind my comment about technical debt from the #111. I see you added it to the sample app. Thanks!

CC - @itsme291

@TranceLove TranceLove force-pushed the feature/resolve-path branch from 19a50e0 to 42a5ae1 Compare April 18, 2025 08:54
@dzuluaga
Copy link
Copy Markdown
Contributor

Hey @TranceLove, there's another conflict to be resolved as the about function now includes queryFields optional parameter. Could you please resolve this conflict? Thanks!

<<<<<<< feature/resolve-path
    fun about(): Drive.About.Get = apiProvider.googleDriveApiService.about().get()

    fun queryNodeIdHaving(path: String): String? {
        val parts = path.splitPathToParts()
        var parentId = GoogleDriveGmsConstants.ROOT_FOLDER
        for (nodeName in parts) {
            val query = createQueryForNodeId(parentId, nodeName)
            val files = query.files
            if (files == null || files.isEmpty()) {
                return null
            } else {
                parentId = files.first().id
            }
        }
        return parentId
    }

    private fun createQueryForNodeId(parentId: String, nodeName: String): FileList =
        apiProvider.googleDriveApiService
            .files()
            .list()
            .setQ("'$parentId' in parents and name='$nodeName'").execute()
=======
    fun about(queryFields: String? = null): Drive.About.Get =
        apiProvider.googleDriveApiService.about().get().also {
            if (!queryFields.isNullOrEmpty()) {
                it.setFields(queryFields)
            }
        }
>>>>>>> main

Also updated FileViewer sample app to showcase use of folder size and storage quota information previously submitted.

Signed-off-by: Raymond Lai <airwave209gt@gmail.com>
@TranceLove TranceLove force-pushed the feature/resolve-path branch from 42a5ae1 to d26a10c Compare April 19, 2025 02:56
@dzuluaga dzuluaga self-assigned this Apr 22, 2025
Copy link
Copy Markdown
Contributor

@dzuluaga dzuluaga left a comment

Choose a reason for hiding this comment

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

Hey @TranceLove, thanks for the PR again. This is looking really good, could you check my last comments before merging? Thank you!

return apiClient.dropboxApiService.users().spaceUsage
}

fun queryNodeIdHaving(path: String): String? {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a suggestion: would it be possible to simplify queryNodeIdHaving() by using the Dropbox API’s getMetadata() method directly?

Since apiClient.dropboxApiService.files().getMetadata(path) already returns a FileMetadata or FolderMetadata (depending on the path), you might be able to replace the manual traversal logic with something like:

fun queryNodeIdHaving(path: String): String? {
    return try {
        val metadata = apiClient.dropboxApiService.files().getMetadata(path)
        when (metadata) {
            is FolderMetadata -> metadata.id
            is FileMetadata -> metadata.id
            else -> null
        }
    } catch (e: Exception) {
        null
    }
}

This would still handle files and folders, and gracefully return null if the path doesn’t exist or isn’t accessible. Curious if there’s a specific reason for the manual traversal — maybe to validate intermediate folders?

Let me know what you think!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch 👍 let's see how this change would work.

- Add slf4j-api as dependency. Should be better than android.util.Log, less reliant on Android classes and gives more flexibility.

Not depend on slf4j-android directly except in tests and sample app.
- Add debug log statements for path traversal and timing metric checks

Signed-off-by: Raymond Lai <airwave209gt@gmail.com>
@dzuluaga
Copy link
Copy Markdown
Contributor

Thanks @TranceLove. Will submit the changes, then we need to publish the assets to Maven central. Stay tuned.

@dzuluaga dzuluaga merged commit a33aeb1 into main Apr 23, 2025
4 checks passed
@dzuluaga dzuluaga deleted the feature/resolve-path branch April 23, 2025 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants