-
Notifications
You must be signed in to change notification settings - Fork 113
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
[WIP] Use tus for uploads (and support range requests for downloads?) #286
Conversation
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
} | ||
|
||
// infoPath returns the path to the .info file storing the file's info. | ||
func (store OwnCloudStore) infoPath(ctx context.Context, id string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func OwnCloudStore.infoPath
is unused (from unused
)
… could use upstream
tusd "github.com/tus/tusd/pkg/handler" | ||
) | ||
|
||
const UploadLengthDeferred = "1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exported const UploadLengthDeferred should have comment or be unexported
haveInvalidDeferHeader := uploadDeferLengthHeader != "" && uploadDeferLengthHeader != UploadLengthDeferred | ||
lengthIsDeferred := uploadDeferLengthHeader == UploadLengthDeferred | ||
|
||
if lengthIsDeferred && !handler.composer.UsesLengthDeferrer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ifElseChain: rewrite if-else to switch statement (from gocritic
)
// Directly finish the upload if the upload is empty (i.e. has a size of 0). | ||
// This statement is in an else-if block to avoid causing duplicate calls | ||
// to finishUploadIfComplete if an upload is empty and contains a chunk. | ||
handler.finishUploadIfComplete(ctx, upload, info, r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of handler.finishUploadIfComplete
is not checked (from errcheck
)
return | ||
} | ||
|
||
defer lock.Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of lock.Unlock
is not checked (from errcheck
)
return | ||
} | ||
|
||
defer lock.Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of lock.Unlock
is not checked (from errcheck
)
} | ||
|
||
handler.sendResp(w, r, http.StatusOK) | ||
io.Copy(w, src) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of io.Copy
is not checked (from errcheck
)
w.Header().Set("Content-Type", "text/plain; charset=utf-8") | ||
w.Header().Set("Content-Length", strconv.Itoa(len(reason))) | ||
w.WriteHeader(statusErr.StatusCode()) | ||
w.Write(reason) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of w.Write
is not checked (from errcheck
)
aside from the noise above what really now is missing is an async process to propagate the etag, mtime and size changes. I want to trigger that only after the file upload is finished AND a possible workflow has happened (eg antivirus scan). eos should magically work without this. but for the owncloud storage driver this piece is still missing.
|
} | ||
|
||
var ( | ||
ErrUnsupportedVersion = NewHTTPError(errors.New("unsupported version"), http.StatusPreconditionFailed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exported var ErrUnsupportedVersion should have comment or be unexported
LogEvent(h.logger, eventName, details...) | ||
} | ||
|
||
func LogEvent(logger *log.Logger, eventName string, details ...string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exported function LogEvent should have comment or be unexported
haveInvalidDeferHeader := uploadDeferLengthHeader != "" && uploadDeferLengthHeader != UploadLengthDeferred | ||
lengthIsDeferred := uploadDeferLengthHeader == UploadLengthDeferred | ||
|
||
if lengthIsDeferred && !handler.composer.UsesLengthDeferrer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ifElseChain: rewrite if-else to switch statement (from gocritic
)
// UploadProgress is used to send notifications about the progress of the | ||
// currently running uploads. For each open PATCH request, every second | ||
// a HookEvent instance will be send over this channel with the Offset field | ||
// being set to the number of bytes which have been transfered to the server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transfered
is a misspelling of transferred
(from misspell
)
// being set to the number of bytes which have been transfered to the server. |
// mimeInlineBrowserWhitelist is a map containing MIME types which should be | ||
// allowed to be rendered by browser inline, instead of being forced to be | ||
// downloadd. For example, HTML or SVG files are not allowed, since they may | ||
// contain malicious JavaScript. In a similiar fashion PDF is not on this list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similiar
is a misspelling of similar
(from misspell
)
// contain malicious JavaScript. In a similiar fashion PDF is not on this list |
// Directly finish the upload if the upload is empty (i.e. has a size of 0). | ||
// This statement is in an else-if block to avoid causing duplicate calls | ||
// to finishUploadIfComplete if an upload is empty and contains a chunk. | ||
handler.finishUploadIfComplete(ctx, upload, info, r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of handler.finishUploadIfComplete
is not checked (from errcheck
)
return | ||
} | ||
|
||
defer lock.Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of lock.Unlock
is not checked (from errcheck
)
return | ||
} | ||
|
||
defer lock.Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of lock.Unlock
is not checked (from errcheck
)
} | ||
|
||
handler.sendResp(w, r, http.StatusOK) | ||
io.Copy(w, src) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of io.Copy
is not checked (from errcheck
)
w.Header().Set("Content-Type", "text/plain; charset=utf-8") | ||
w.Header().Set("Content-Length", strconv.Itoa(len(reason))) | ||
w.WriteHeader(statusErr.StatusCode()) | ||
w.Write(reason) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of w.Write
is not checked (from errcheck
)
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
} | ||
|
||
upload.info.Offset += n | ||
upload.writeInfo() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error return value of upload.writeInfo
is not checked (from errcheck
)
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
replace datasvc with tusdsvc ... or teach datasvc some tus tricks ... fixes #284
currently needs tus/tusd#309