Skip to content
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

Add storage API to open a file for appending, without truncating it's content first #5176

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions internal/driver/mobile/android.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,20 @@ void* openStream(uintptr_t jni_env, uintptr_t ctx, char* uriCstr) {
return (*env)->NewGlobalRef(env, stream);
}

void* saveStream(uintptr_t jni_env, uintptr_t ctx, char* uriCstr) {
void* saveStream(uintptr_t jni_env, uintptr_t ctx, char* uriCstr, bool truncate) {
JNIEnv *env = (JNIEnv*)jni_env;
jobject resolver = getContentResolver(jni_env, ctx);

jclass resolverClass = (*env)->GetObjectClass(env, resolver);
jmethodID saveOutputStream = find_method(env, resolverClass, "openOutputStream", "(Landroid/net/Uri;Ljava/lang/String;)Ljava/io/OutputStream;");

jobject uri = parseURI(jni_env, ctx, uriCstr);
jstring modes = (*env)->NewStringUTF(env, "wt"); // truncate before write
jstring modes = NULL;
if (truncate) {
jstring modes = (*env)->NewStringUTF(env, "wt"); // truncate before write
} else {
jstring modes = (*env)->NewStringUTF(env, "wa");
}
jobject stream = (jobject)(*env)->CallObjectMethod(env, resolver, saveOutputStream, uri, modes);
jthrowable loadErr = (*env)->ExceptionOccurred(env);

Expand Down
7 changes: 4 additions & 3 deletions internal/driver/mobile/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ func (f *fileSave) URI() fyne.URI {
return f.uri
}

func fileWriterForURI(u fyne.URI) (fyne.URIWriteCloser, error) {
func fileWriterForURI(u fyne.URI, truncate bool) (fyne.URIWriteCloser, error) {
file := &fileSave{uri: u}
write, err := nativeFileSave(file)
write, err := nativeFileSave(file, truncate)
if write == nil {
return nil, err
}
Expand Down Expand Up @@ -126,7 +126,8 @@ func ShowFileSavePicker(callback func(fyne.URIWriteCloser, error), filter storag
return
}

f, err := fileWriterForURI(uri)
// TODO: does the save dialog want to truncate by default?
f, err := fileWriterForURI(uri, true)
if f != nil {
f.(*fileSave).done = closer
}
Expand Down
10 changes: 5 additions & 5 deletions internal/driver/mobile/file_android.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ bool deleteURI(uintptr_t jni_env, uintptr_t ctx, char* uriCstr);
bool existsURI(uintptr_t jni_env, uintptr_t ctx, char* uriCstr);
void* openStream(uintptr_t jni_env, uintptr_t ctx, char* uriCstr);
char* readStream(uintptr_t jni_env, uintptr_t ctx, void* stream, int len, int* total);
void* saveStream(uintptr_t jni_env, uintptr_t ctx, char* uriCstr);
void* saveStream(uintptr_t jni_env, uintptr_t ctx, char* uriCstr, bool truncate);
void writeStream(uintptr_t jni_env, uintptr_t ctx, void* stream, char* data, int len);
void closeStream(uintptr_t jni_env, uintptr_t ctx, void* stream);
*/
Expand Down Expand Up @@ -99,13 +99,13 @@ func nativeFileOpen(f *fileOpen) (io.ReadCloser, error) {
return stream, nil
}

func saveStream(uri string) unsafe.Pointer {
func saveStream(uri string, truncate bool) unsafe.Pointer {
uriStr := C.CString(uri)
defer C.free(unsafe.Pointer(uriStr))

var stream unsafe.Pointer
app.RunOnJVM(func(_, env, ctx uintptr) error {
streamPtr := C.saveStream(C.uintptr_t(env), C.uintptr_t(ctx), uriStr)
streamPtr := C.saveStream(C.uintptr_t(env), C.uintptr_t(ctx), uriStr, C.bool(truncate))
if streamPtr == C.NULL {
return os.ErrNotExist
}
Expand All @@ -116,12 +116,12 @@ func saveStream(uri string) unsafe.Pointer {
return stream
}

func nativeFileSave(f *fileSave) (io.WriteCloser, error) {
func nativeFileSave(f *fileSave, truncate bool) (io.WriteCloser, error) {
if f.uri == nil || f.uri.String() == "" {
return nil, nil
}

ret := saveStream(f.uri.String())
ret := saveStream(f.uri.String(), truncate)
if ret == nil {
return nil, storage.ErrNotExists
}
Expand Down
2 changes: 1 addition & 1 deletion internal/driver/mobile/file_desktop.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func nativeFileOpen(*fileOpen) (io.ReadCloser, error) {
return nil, nil
}

func nativeFileSave(*fileSave) (io.WriteCloser, error) {
func nativeFileSave(*fileSave, bool) (io.WriteCloser, error) {
// no-op as we use the internal FileRepository
return nil, nil
}
Expand Down
6 changes: 3 additions & 3 deletions internal/driver/mobile/file_ios.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ bool iosExistsPath(const char* path);
void* iosParseUrl(const char* url);
const void* iosReadFromURL(void* url, int* len);

const void* iosOpenFileWriter(void* url);
const void* iosOpenFileWriter(void* url, bool truncate);
void iosCloseFileWriter(void* handle);
const int iosWriteToFile(void* handle, const void* bytes, int len);
*/
Expand Down Expand Up @@ -136,7 +136,7 @@ func nativeFileOpen(f *fileOpen) (io.ReadCloser, error) {
return fileStruct, nil
}

func nativeFileSave(f *fileSave) (io.WriteCloser, error) {
func nativeFileSave(f *fileSave, truncate bool) (io.WriteCloser, error) {
if f.uri == nil || f.uri.String() == "" {
return nil, nil
}
Expand All @@ -146,7 +146,7 @@ func nativeFileSave(f *fileSave) (io.WriteCloser, error) {

url := C.iosParseUrl(cStr)

handle := C.iosOpenFileWriter(url)
handle := C.iosOpenFileWriter(url, C.bool(truncate))
fileStruct := &secureWriteCloser{handle: handle, closer: f.done}
return fileStruct, nil
}
Expand Down
15 changes: 12 additions & 3 deletions internal/driver/mobile/file_ios.m
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,22 @@ bool iosExistsPath(const char* path) {
return data.bytes;
}

const void* iosOpenFileWriter(void* urlPtr) {
const void* iosOpenFileWriter(void* urlPtr, bool truncate) {
NSURL* url = (NSURL*)urlPtr;
[[NSFileManager defaultManager] createFileAtPath:url.path contents:nil attributes:nil];

if (truncate || ![[NSFileManager defaultManager] fileExistsAtPath:url.path]) {
[[NSFileManager defaultManager] createFileAtPath:url.path contents:nil attributes:nil];
}

NSError *err = nil;
// TODO handle error
return [NSFileHandle fileHandleForWritingToURL:url error:&err];
NSFileHandle* handle = [NSFileHandle fileHandleForWritingToURL:url error:&err];

if (!truncate) {
[handle truncateFileAtOffset:[handle seekToEndOfFile]];
}

return handle;
}

void iosCloseFileWriter(void* handlePtr) {
Expand Down
7 changes: 6 additions & 1 deletion internal/driver/mobile/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var _ repository.Repository = (*mobileFileRepo)(nil)
var _ repository.HierarchicalRepository = (*mobileFileRepo)(nil)
var _ repository.ListableRepository = (*mobileFileRepo)(nil)
var _ repository.WritableRepository = (*mobileFileRepo)(nil)
var _ repository.AppendableRepository = (*mobileFileRepo)(nil)

type mobileFileRepo struct {
}
Expand Down Expand Up @@ -67,5 +68,9 @@ func (m *mobileFileRepo) Reader(u fyne.URI) (fyne.URIReadCloser, error) {
}

func (m *mobileFileRepo) Writer(u fyne.URI) (fyne.URIWriteCloser, error) {
return fileWriterForURI(u)
return fileWriterForURI(u, true)
}

func (m *mobileFileRepo) Appender(u fyne.URI) (fyne.URIWriteCloser, error) {
return fileWriterForURI(u, false)
}
22 changes: 17 additions & 5 deletions internal/repository/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const fileSchemePrefix string = "file://"
// declare conformance with repository types
var _ repository.Repository = (*FileRepository)(nil)
var _ repository.WritableRepository = (*FileRepository)(nil)
var _ repository.AppendableRepository = (*FileRepository)(nil)
var _ repository.HierarchicalRepository = (*FileRepository)(nil)
var _ repository.ListableRepository = (*FileRepository)(nil)
var _ repository.MovableRepository = (*FileRepository)(nil)
Expand Down Expand Up @@ -72,12 +73,16 @@ func (r *FileRepository) Exists(u fyne.URI) (bool, error) {
return ok, err
}

func openFile(uri fyne.URI, create bool) (*file, error) {
func openFile(uri fyne.URI, write bool, truncate bool) (*file, error) {
path := uri.Path()
var f *os.File
var err error
if create {
f, err = os.Create(path) // If it exists this will truncate which is what we wanted
if write {
if truncate {
f, err = os.Create(path) // If it exists this will truncate which is what we wanted
} else {
f, err = os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0666)
}
} else {
f, err = os.Open(path)
}
Expand All @@ -88,7 +93,7 @@ func openFile(uri fyne.URI, create bool) (*file, error) {
//
// Since: 2.0
func (r *FileRepository) Reader(u fyne.URI) (fyne.URIReadCloser, error) {
return openFile(u, false)
return openFile(u, false, false)
}

// CanRead implements repository.Repository.CanRead
Expand Down Expand Up @@ -123,7 +128,14 @@ func (r *FileRepository) Destroy(scheme string) {
//
// Since: 2.0
func (r *FileRepository) Writer(u fyne.URI) (fyne.URIWriteCloser, error) {
return openFile(u, true)
return openFile(u, true, true)
}

// Appender implements repository.AppendableRepository.Appender
//
// Since: 2.6
func (r *FileRepository) Appender(u fyne.URI) (fyne.URIWriteCloser, error) {
return openFile(u, true, false)
}

// CanWrite implements repository.WritableRepository.CanWrite
Expand Down
10 changes: 9 additions & 1 deletion internal/repository/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,14 @@ func TestFileRepositoryWriter(t *testing.T) {
barWriter.Close()
bazWriter.Close()

bazAppender, err := storage.Appender(baz)
assert.Nil(t, err)
n, err = bazAppender.Write([]byte{1, 2, 3, 4, 5})
assert.Nil(t, err)
assert.Equal(t, 5, n)

bazAppender.Close()

// now make sure we can read the data back correctly
fooReader, err := storage.Reader(foo)
assert.Nil(t, err)
Expand All @@ -203,7 +211,7 @@ func TestFileRepositoryWriter(t *testing.T) {
bazReader, err := storage.Reader(baz)
assert.Nil(t, err)
bazData, err := io.ReadAll(bazReader)
assert.Equal(t, []byte{5, 4, 3, 2, 1, 0}, bazData)
assert.Equal(t, []byte{5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5}, bazData)
assert.Nil(t, err)

// close the readers, since Windows won't let us delete things with
Expand Down
13 changes: 13 additions & 0 deletions internal/repository/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var _ fyne.URIWriteCloser = (*nodeReaderWriter)(nil)
// declare conformance with repository types
var _ repository.Repository = (*InMemoryRepository)(nil)
var _ repository.WritableRepository = (*InMemoryRepository)(nil)
var _ repository.AppendableRepository = (*InMemoryRepository)(nil)
var _ repository.HierarchicalRepository = (*InMemoryRepository)(nil)
var _ repository.CopyableRepository = (*InMemoryRepository)(nil)
var _ repository.MovableRepository = (*InMemoryRepository)(nil)
Expand Down Expand Up @@ -210,6 +211,18 @@ func (m *InMemoryRepository) Writer(u fyne.URI) (fyne.URIWriteCloser, error) {
return &nodeReaderWriter{path: path, repo: m}, nil
}

// Appender implements repository.AppendableRepository.Appender
//
// Since: 2.6
func (m *InMemoryRepository) Appender(u fyne.URI) (fyne.URIWriteCloser, error) {
path := u.Path()
if path == "" {
return nil, fmt.Errorf("invalid path '%s'", path)
}

return &nodeReaderWriter{path: path, repo: m, writing: true, writeCursor: len(m.Data[path])}, nil
}

// CanWrite implements repository.WritableRepository.CanWrite
//
// Since: 2.0
Expand Down
10 changes: 9 additions & 1 deletion internal/repository/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,14 @@ func TestInMemoryRepositoryWriter(t *testing.T) {
barWriter.Close()
bazWriter.Close()

bazAppender, err := storage.Appender(baz)
assert.Nil(t, err)
n, err = bazAppender.Write([]byte{1, 2, 3, 4, 5})
assert.Nil(t, err)
assert.Equal(t, 5, n)

bazAppender.Close()

// now make sure we can read the data back correctly
fooReader, err := storage.Reader(foo)
assert.Nil(t, err)
Expand All @@ -210,7 +218,7 @@ func TestInMemoryRepositoryWriter(t *testing.T) {
bazReader, err := storage.Reader(baz)
assert.Nil(t, err)
bazData, err := io.ReadAll(bazReader)
assert.Equal(t, []byte{5, 4, 3, 2, 1, 0}, bazData)
assert.Equal(t, []byte{5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5}, bazData)
assert.Nil(t, err)

// now let's test deletion
Expand Down
14 changes: 14 additions & 0 deletions storage/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,20 @@ type WritableRepository interface {
Delete(u fyne.URI) error
}

// AppendableRepository is an extension of the WritableRepository interface which also
// supports opening a writer for URIs in append mode, without truncating their contents
//
// Since: 2.0
type AppendableRepository interface {
WritableRepository

// Appender will be used to call a Writer without truncating the
// file if it exists
//
// Since: 2.6
Appender(u fyne.URI) (fyne.URIWriteCloser, error)
}

// ListableRepository is an extension of the Repository interface which also
// supports obtaining directory listings (generally analogous to a directory
// listing) for URIs of the scheme it is registered to.
Expand Down
42 changes: 42 additions & 0 deletions storage/uri.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,48 @@ func Writer(u fyne.URI) (fyne.URIWriteCloser, error) {
return wrepo.Writer(u)
}

// Appender returns URIWriteCloser set up to write to the resource that the
// URI references without truncating it first
//
// Writing to a non-extant resource should create that resource if possible
// (and if not possible, this should be reflected in the return of CanWrite()).
// Writing to an extant resource should NOT overwrite it in-place.
//
// This method can fail in several ways:
//
// - Different permissions or credentials are required to write to the
// referenced resource.
//
// - This URI scheme could represent some resources that can be
// written, but this particular URI references a resources that is
// not something that can be written.
//
// - Attempting to set up the writer depended on a lower level
// operation such as a network or filesystem access that has failed
// in some way.
//
// - If the scheme of the given URI does not have a registered
// AppendableRepository instance, then this method will fail with a
// repository.ErrOperationNotSupported.
//
// Appender is backed by the repository system - this function calls into a
// scheme-specific implementation from a registered repository.
//
// Since: 2.6
func Appender(u fyne.URI) (fyne.URIWriteCloser, error) {
repo, err := repository.ForURI(u)
if err != nil {
return nil, err
}

wrepo, ok := repo.(repository.AppendableRepository)
if !ok {
return nil, repository.ErrOperationNotSupported
}

return wrepo.Appender(u)
}

// CanWrite determines if a given URI could be written to using the Writer()
// method. It is preferred to check if a URI is writable using this method
// before calling Writer(), because the underlying operations required to
Expand Down
10 changes: 9 additions & 1 deletion storage/uri_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ func TestWriteAndDelete(t *testing.T) {
barWriter.Close()
bazWriter.Close()

bazAppender, err := storage.Appender(baz)
assert.Nil(t, err)
n, err = bazAppender.Write([]byte{1, 2, 3, 4, 5})
assert.Nil(t, err)
assert.Equal(t, 5, n)

bazAppender.Close()

// now make sure we can read the data back correctly
fooReader, err := storage.Reader(foo)
assert.Nil(t, err)
Expand All @@ -286,7 +294,7 @@ func TestWriteAndDelete(t *testing.T) {
bazReader, err := storage.Reader(baz)
assert.Nil(t, err)
bazData, err := io.ReadAll(bazReader)
assert.Equal(t, []byte{5, 4, 3, 2, 1, 0}, bazData)
assert.Equal(t, []byte{5, 4, 3, 2, 1, 0, 1, 2, 3, 4, 5}, bazData)
assert.Nil(t, err)

// now let's test deletion
Expand Down