Skip to content

android: expand SAF FileOps implementation #675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 5, 2025
Merged

android: expand SAF FileOps implementation #675

merged 3 commits into from
Aug 5, 2025

Conversation

kari-ts
Copy link
Collaborator

@kari-ts kari-ts commented Jul 2, 2025

This expands the SAF FileOps to implement the refactored FileOps

Updates tailscale/corp#29211

@kari-ts kari-ts force-pushed the safresume branch 2 times, most recently from 618294d to 8370ee8 Compare July 3, 2025 23:32
@kari-ts kari-ts force-pushed the safresume branch 4 times, most recently from 18f446a to fcd9bb0 Compare July 22, 2025 19:41
This expands the SAF FileOps to implement the refactored FileOps

Updates tailscale/corp#29211

Signed-off-by: kari-ts <[email protected]>
Copy link
Contributor

@nickkhyl nickkhyl left a comment

Choose a reason for hiding this comment

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

(still reviewing)

@@ -175,14 +175,45 @@ type OutputStream interface {

// ShareFileHelper corresponds to the Kotlin ShareFileHelper class
type ShareFileHelper interface {
// OpenFileWriter creates or truncates a file named fileName
// and returns an OutputStream for writing to it from the beginning.
// Returns nil if the file cannot be opened.
Copy link
Contributor

Choose a reason for hiding this comment

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

Other interfaces defined in libtailscale and implemented in Kotlin suggest that exceptions are supported will be translated to Go errors.

// GetSyspolicyStringValue returns the current string value for the given system policy.
GetSyspolicyStringValue(key string) (string, error)

@Throws(
IOException::class, GeneralSecurityException::class, MDMSettings.NoSuchKeyException::class)
override fun getSyspolicyStringValue(key: String): String {
val setting = MDMSettings.allSettingsByKey[key]?.flow?.value
if (setting?.isSet != true) {
throw MDMSettings.NoSuchKeyException()
}
return setting.value?.toString() ?: ""
}

If that's correct, is there a reason we chose to report errors by returning nil values here and in other ShareFileHelper methods?

Copy link
Contributor

@nickkhyl nickkhyl left a comment

Choose a reason for hiding this comment

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

LGTM with the androidFileInfo marshalling fixed and other feedback considered.

}
uri, err := ops.helper.GetFileURI(name)
if err != nil {
return nil, "", err
Copy link
Contributor

Choose a reason for hiding this comment

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

Close wc if GetFileURI fails?

OSS and Version updated to 1.87.25-t0f15e4419-gde3b6dbfd

Signed-off-by: kari-ts <[email protected]>
@kari-ts kari-ts merged commit e71641a into main Aug 5, 2025
4 checks passed
@kari-ts kari-ts deleted the safresume branch August 5, 2025 17:39
barnstar pushed a commit that referenced this pull request Aug 7, 2025
* android: expand SAF FileOps implementation

This expands the SAF FileOps to implement the refactored FileOps

Updates tailscale/corp#29211

Signed-off-by: kari-ts <[email protected]>

(cherry picked from commit e71641a)

Signed-off-by: kari-ts <[email protected]>
barnstar pushed a commit that referenced this pull request Aug 7, 2025
* android: expand SAF FileOps implementation

This expands the SAF FileOps to implement the refactored FileOps

Updates tailscale/corp#29211

Signed-off-by: kari-ts <[email protected]>

(cherry picked from commit e71641a)

Signed-off-by: kari-ts <[email protected]>
Signed-off-by: Jonathan Nobels <[email protected]>
barnstar added a commit that referenced this pull request Aug 7, 2025
* android: expand SAF FileOps implementation

This expands the SAF FileOps to implement the refactored FileOps

Updates tailscale/corp#29211



(cherry picked from commit e71641a)

Signed-off-by: kari-ts <[email protected]>
Signed-off-by: Jonathan Nobels <[email protected]>
Co-authored-by: kari-ts <[email protected]>
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.

2 participants