ios: fix exporting files (CSV exports, log export, notes export) #2939
Conversation
|
Ping @Beerosagos in case you missed it |
| class GoEnvironment: NSObject, MobileserverGoEnvironmentInterfaceProtocol, UIDocumentInteractionControllerDelegate { | ||
| func getSaveFilename(_ fileName: String?) -> String { | ||
| let tempDirectory = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true) | ||
| let fileURL = tempDirectory.appendingPathComponent(fileName!) |
There was a problem hiding this comment.
I think we need a guard for fileName, like the one used in systemOpen for the urlString
There was a problem hiding this comment.
I am unsure how filename could actually be nil here, any clue? That's why I just used fileName!.
There was a problem hiding this comment.
I agree it should not happen, but from what I could understand, the app would crash if that happened. I just felt that in the future we may exploit this method somewhere else without realizing that it is not safe against null values. wdyt?
There was a problem hiding this comment.
I fixed it, but I still disagree 👅 this is in the GoEnv interface, so it will only ever be called by the Go backend, and there is no "nil" string in Go. I am unsure why gomobile even types this as optional, it seems almost like a bug.
There was a problem hiding this comment.
Ah now I see your point! I didn't consider that strings are not pointers in Go 😇 Then I agree with you. Feel free to put it back to how it was initially. Maybe it would be nice to have a comment explicitly stating why we don't need the guard, tho
| return scene.windows.first(where: { $0.isKeyWindow })?.rootViewController | ||
| } | ||
|
|
||
| func systemOpen(_ urlString: String?) throws { |
There was a problem hiding this comment.
The function doesn't throw any exception, I think you can remove the throws keyword
There was a problem hiding this comment.
I don't think we can remove it - this whole class adheres to MobileserverGoEnvironmentInterfaceProtocol, i.e. it follows the Environment interface in backend.go. I assume gomobile translates a Go error return to throws:
bitbox-wallet-app/backend/backend.go
Line 150 in c4867b8
benma
force-pushed
the
ios-export
branch
2 times, most recently
from
5302525
to
212d401
Compare
Similar to Android, we set `ExportsDir()` to `""` and let the backend environment `getSaveFilename()` construct the full path. Mobile handling of the dirs/files is a big hack and works on implicit assumptions, e.g. that `getSaveFilename()` is always called with `filepath.Join(exportsDir, filename)`, which is only the filename on mobile as `ExportsDir()` returns `""`. This should be cleaned up, but for now iOS is made to behave like Android in this regard. We use a temp directory for these files, so the files will be cleaned up automatically after a while.
Similar to Android, we set
ExportsDir()to""and let the backend environmentgetSaveFilename()construct the full path.Mobile handling of the dirs/files is a big hack and works on implicit assumptions, e.g. that
getSaveFilename()is always called withfilepath.Join(exportsDir, filename), which is only the filename on mobile asExportsDir()returns"". This should be cleaned up, but for now iOS is made to behave like Android in this regard.