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.

Sign up for GitHub

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

Jump to bottom

tus for ocdav #674

Merged
merged 14 commits into from
Apr 30, 2020
Merged

tus for ocdav #674

merged 14 commits into from
Apr 30, 2020

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Apr 21, 2020 •
edited
Loading

This PR / branch will serve as the basis for further ocis-web development

includes #669
based on #661

@butonic butonic requested a review from labkode as a code owner April 21, 2020 21:10
@butonic
Copy link
Contributor Author

butonic commented Apr 21, 2020

cc @PVince81 @@LukasHirt pleaase try phoenix on top of this

@butonic butonic mentioned this pull request Apr 22, 2020
Merged
@PVince81 PVince81 mentioned this pull request Apr 22, 2020
Closed
@butonic
Copy link
Contributor Author

butonic commented Apr 22, 2020 •
edited
Loading

@labkode @PVince81 I think I addressed all comments or raised specific issues to track them.

butonic added 6 commits April 23, 2020 14:29
@butonic
minimal tus implmentation
03476b1 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
minimal tus implementation for ocdav
dbeabee 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
add CORS headers to ocdav
f557a7a 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
add CORS headers to middleware
3640502 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
also return Tus-Resumable headers
609442b 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
use data as default dataprovider prefix
5e0ade6 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic requested a review from labkode April 23, 2020 14:17
@PVince81
Copy link
Contributor

PVince81 commented Apr 23, 2020 •
edited
Loading

  • BUG: getting a 500 internal server error for non-TUS upload with PUT (works fine on reva master). Please note that non-TUS upload with PUT works fine on the butonic:tus-dataprovider branch.

@PVince81
Copy link
Contributor

  • BUG: on this branch I still get a CORS error:

image

it wasn't present on the butonic:tus-dataprovider branch and works fine there when testing with owncloud/web#3345 (had to hack it to bypass capability check)

@butonic butonic mentioned this pull request Apr 23, 2020
Closed
@butonic
leave CORS to tus, fix upload on eos
d7f1f36 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
Copy link
Contributor Author

butonic commented Apr 23, 2020

ok, the cors issues are fixed but require changes to the Makefile as well. but we now run into the funny namespacing again: owncloud/ocis#219

@butonic butonic mentioned this pull request Apr 23, 2020
Closed
butonic added 2 commits April 24, 2020 16:22
@butonic
fix upload handling
6f05a46 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
allow user layout in ocdav namespeces
9c4ff7c 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
Copy link
Contributor Author

butonic commented Apr 24, 2020

Ok, with the last commit I can now configure

  • remote.php/webdav to use /home and
  • remote.php/dav/files to use /eos/users/{{substr 0 1 .Username}}/ (instead of just /eos)

Which allows the home storage to return references to the /eos dataprovider (which is configured not to use user_layouts because it is NOT a home storage)

The remote.php/dav/files endpoint will as the first argument always get a username which we must 'layout' the same way as in eos. I am currently relying on the requests to always use the user that is logged in ... but we could change that to layout the first path parameter, becaluse we know in the dav/files/ endpoint always is the username.

Also ... it seems to become more clear that a global endpoint should get its own endpoint, eg. /dav/global

@butonic butonic mentioned this pull request Apr 24, 2020
Closed
@butonic butonic mentioned this pull request Apr 24, 2020
Closed
6 tasks
@PVince81 PVince81 mentioned this pull request Apr 27, 2020
Closed
@butonic
Copy link
Contributor Author

butonic commented Apr 27, 2020

@labkode please restart https://cloud.drone.io/cs3org/reva/1023/1/5

butonic added 5 commits April 29, 2020 18:10
@butonic
implement chmod based change propagation in owncloud driver
ced5ea3 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
also return Tus-Resumable on POST errors
488859d 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
fix propagation
f5afd81 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
implement creation-with-upload for owncloud
1ba0da2 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic
close body after forwarding request
c79ca8c 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
labkode
labkode reviewed Apr 30, 2020
View reviewed changes
internal/grpc/services/storageprovider/storageprovider.go
newRef, err := s.unwrap(ctx, req.Ref)
if err != nil {
return &provider.InitiateFileUploadResponse{
Status: status.NewInternal(ctx, err, "error unwrapping path"),
}, nil
}
url.Path = path.Join("/", url.Path, newRef.GetPath())
var uploadLength int64
if req.Opaque != nil && req.Opaque.Map != nil && req.Opaque.Map["Upload-Length"] != nil {
Copy link
Member

Choose a reason for hiding this comment

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

It should come from the CS3 Req

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tracked in cs3org/cs3apis#71

@labkode labkode merged commit 09edd2b into cs3org:master Apr 30, 2020
C0rby pushed a commit to owncloud/reva that referenced this pull request May 4, 2020
@butonic
tus support (cs3org#674)
8f1ebc3
@micbar micbar mentioned this pull request Jun 26, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PVince81 PVince81 PVince81 left review comments

@labkode labkode labkode approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@butonic @PVince81 @labkode