Skip to content

Commit

Permalink
Removed storageID and UIURL from config.
Browse files Browse the repository at this point in the history 
StorageID is auto-resolved in the gateway service, whereas
the UIURL's doc was misleading: we need to decide whether
to drop it entirely (surely it won't stay in the config).

En passant, this fixes cs3org#991 and includes other minor changes
concerning error handling and logging.
  • Loading branch information
@glpatcern
glpatcern committed Jul 29, 2020
1 parent 244e96d commit c64497c
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 66 deletions.
20 changes: 2 additions & 18 deletions docs/content/en/docs/config/grpc/services/appprovider/_index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,34 +9,18 @@ description: >
# _struct: config_

{{% dir name="iopsecret" type="string" default="" %}}
The iopsecret used to connect to the wopiserver. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L59)
The iopsecret used to connect to the wopiserver. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L57)
{{< highlight toml >}}
[grpc.services.appprovider]
iopsecret = ""
{{< /highlight >}}
{{% /dir %}}

{{% dir name="wopiurl" type="string" default="" %}}
The wopiserver's url. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L60)
The wopiserver's URL. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L58)
{{< highlight toml >}}
[grpc.services.appprovider]
wopiurl = ""
{{< /highlight >}}
{{% /dir %}}

{{% dir name="uirul" type="string" default="" %}}
URL to application (eg collabora) URL. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L61)
{{< highlight toml >}}
[grpc.services.appprovider]
uirul = ""
{{< /highlight >}}
{{% /dir %}}

{{% dir name="storageid" type="string" default="" %}}
The storage id used by the wopiserver to look up the file or storage id defaults to default by the wopiserver if empty. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L62)
{{< highlight toml >}}
[grpc.services.appprovider]
storageid = ""
{{< /highlight >}}
{{% /dir %}}

2 changes: 0 additions & 2 deletions examples/ocmd/ocmd-server-1.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ driver = "memory"
driver = "demo"
iopsecret = "testsecret"
wopiurl = "http://0.0.0.0:8880/"
storageid = ""
uiurl = ""

[grpc.services.appregistry]
driver = "static"
Expand Down
46 changes: 16 additions & 30 deletions internal/grpc/services/appprovider/appprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"io/ioutil"
"net/http"
"path"
"path/filepath"
"strconv"
"strings"
"time"
Expand All @@ -40,7 +39,6 @@ import (
"github.com/cs3org/reva/pkg/rhttp"
"github.com/cs3org/reva/pkg/user"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"google.golang.org/grpc"
)

Expand All @@ -57,9 +55,7 @@ type config struct {
Driver string `mapstructure:"driver"`
Demo map[string]interface{} `mapstructure:"demo"`
IopSecret string `mapstructure:"iopsecret" docs:";The iopsecret used to connect to the wopiserver."`
WopiURL string `mapstructure:"wopiurl" docs:";The wopiserver's url."`
UIURL string `mapstructure:"uirul" docs:";URL to application (eg collabora) URL."`
StorageID string `mapstructure:"storageid" docs:";The storage id used by the wopiserver to look up the file or storage id defaults to default by the wopiserver if empty."`
WopiURL string `mapstructure:"wopiurl" docs:";The wopiserver's URL."`
}

// New creates a new AppProviderService
Expand Down Expand Up @@ -102,6 +98,7 @@ func (s *service) UnprotectedEndpoints() []string {
func (s *service) Register(ss *grpc.Server) {
providerpb.RegisterProviderAPIServer(ss, s)
}

func getProvider(c *config) (app.Provider, error) {
switch c.Driver {
case "demo":
Expand All @@ -115,11 +112,6 @@ func (s *service) OpenFileInAppProvider(ctx context.Context, req *providerpb.Ope

log := appctx.GetLogger(ctx)

wopiurl := s.conf.WopiURL
iopsecret := s.conf.IopSecret
storageID := s.conf.StorageID
folderURL := s.conf.UIURL + filepath.Dir(req.Ref.GetPath())

httpClient := rhttp.GetHTTPClient(
rhttp.Context(ctx),
// TODO make insecure configurable
Expand All @@ -128,7 +120,9 @@ func (s *service) OpenFileInAppProvider(ctx context.Context, req *providerpb.Ope
rhttp.Timeout(time.Duration(24*int64(time.Hour))),
)

appsReq, err := rhttp.NewRequest(ctx, "GET", wopiurl+"wopi/cbox/endpoints", nil)
// TODO this query will eventually be served by Reva. For the time being it is a remnant of the CERNBox-specific WOPI server,
// which justifies the /cbox path in the URL.
appsReq, err := rhttp.NewRequest(ctx, "GET", s.conf.WopiURL+"wopi/cbox/endpoints", nil)
if err != nil {
return nil, err
}
Expand All @@ -154,50 +148,42 @@ func (s *service) OpenFileInAppProvider(ctx context.Context, req *providerpb.Ope
return nil, err
}

httpReq, err := rhttp.NewRequest(ctx, "GET", wopiurl+"wopi/iop/open", nil)
httpReq, err := rhttp.NewRequest(ctx, "GET", s.conf.WopiURL+"wopi/iop/open", nil)
if err != nil {
return nil, err
}

q := httpReq.URL.Query()
q.Add("filename", req.Ref.GetPath())
q.Add("endpoint", storageID)
q.Add("fileid", req.Ref.GetId().OpaqueId)
q.Add("endpoint", req.Ref.GetId().StorageId)
q.Add("viewmode", req.ViewMode.String())
q.Add("folderurl", folderURL)
// TODO the folder URL should be resolved as e.g. `'https://cernbox.cern.ch/index.php/apps/files/?dir=' + filepath.Dir(req.Ref.GetPath())`
// or should be deprecated/removed altogether, needs discussion and decision.
q.Add("folderurl", "undefined")
u, ok := user.ContextGetUser(ctx)

if ok {
q.Add("username", u.Username)
}
// else defaults to "Anonymous Guest"

httpReq.Header.Set("Authorization", "Bearer "+iopsecret)
httpReq.Header.Set("Authorization", "Bearer "+s.conf.IopSecret)
httpReq.Header.Set("TokenHeader", req.AccessToken)

httpReq.URL.RawQuery = q.Encode()

openRes, err := httpClient.Do(httpReq)

if err != nil {
log.Error().Err(err).Msg("error performing http request")
res := &providerpb.OpenFileInAppProviderResponse{
Status: status.NewInternal(ctx, err, "error performing http request"),
Status: status.NewInternal(ctx, err, "appprovider: error performing open request to WOPI"),
}
return res, nil
}
defer openRes.Body.Close()

if openRes.StatusCode != http.StatusOK {
log.Error().Err(err).Msg("error performing http request")
res := &providerpb.OpenFileInAppProviderResponse{
Status: status.NewInternal(ctx, err, "error performing http request, status code: "+strconv.Itoa(openRes.StatusCode)),
}
return res, nil
}

if err != nil {
err := errors.Wrap(err, "appprovidersvc: error calling GetIFrame")
res := &providerpb.OpenFileInAppProviderResponse{
Status: status.NewInternal(ctx, err, "error getting app's iframe"),
Status: status.NewInternal(ctx, err, "appprovider: error performing open request to WOPI, status code: "+strconv.Itoa(openRes.StatusCode)),
}
return res, nil
}
Expand Down Expand Up @@ -245,7 +231,7 @@ func (s *service) OpenFileInAppProvider(ctx context.Context, req *providerpb.Ope
providerURL += "?"
}

appProviderURL := fmt.Sprintf("App URL:\n%sWOPISrc=%s\n", providerURL, openResBody)
appProviderURL := fmt.Sprintf("%sWOPISrc=%s\n", providerURL, openResBody)

return &providerpb.OpenFileInAppProviderResponse{
Status: status.NewOK(ctx),
Expand Down
6 changes: 0 additions & 6 deletions internal/grpc/services/appprovider/appprovider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,12 @@ func Test_parseConfig(t *testing.T) {
"Demo": map[string]interface{}{"a": "b", "c": "d"},
"IopSecret": "very-secret",
"WopiURL": "https://my.wopi:9871",
"UIURL": "https://the.ui",
"StorageID": "123-455",
},
want: &config{
Driver: "demo",
Demo: map[string]interface{}{"a": "b", "c": "d"},
IopSecret: "very-secret",
WopiURL: "https://my.wopi:9871",
UIURL: "", // this field seems not to get parsed see https://github.com/cs3org/reva/issues/991
StorageID: "123-455",
},
wantErr: nil,
},
Expand All @@ -72,8 +68,6 @@ func Test_parseConfig(t *testing.T) {
Demo: map[string]interface{}(nil),
IopSecret: "",
WopiURL: "",
UIURL: "",
StorageID: "",
},
wantErr: nil,
},
Expand Down
16 changes: 6 additions & 10 deletions internal/grpc/services/gateway/appprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,13 @@ import (
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/pkg/appctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
"github.com/pkg/errors"
)

func (s *svc) OpenFileInAppProvider(ctx context.Context, req *providerpb.OpenFileInAppProviderRequest) (*providerpb.OpenFileInAppProviderResponse, error) {

log := appctx.GetLogger(ctx)

c, err := s.find(ctx, req.Ref)
if err != nil {
if _, ok := err.(errtypes.IsNotFound); ok {
Expand All @@ -56,17 +52,14 @@ func (s *svc) OpenFileInAppProvider(ctx context.Context, req *providerpb.OpenFil
statRes, err := c.Stat(ctx, statReq)

if err != nil {
log.Err(err).Msg("gateway: error calling Stat for the share resource path:" + req.Ref.GetPath())
return &providerpb.OpenFileInAppProviderResponse{
Status: status.NewInternal(ctx, err, "gateway: error calling Stat for the share resource id"),
Status: status.NewInternal(ctx, err, "gateway: error calling Stat on the resource path for the app provider: "+req.Ref.GetPath()),
}, nil
}

if statRes.Status.Code != rpc.Code_CODE_OK {
err := status.NewErrorFromCode(statRes.Status.GetCode(), "gateway")
log.Err(err).Msg("gateway: error calling Stat for the share resource id:" + req.Ref.GetPath())
return &providerpb.OpenFileInAppProviderResponse{
Status: status.NewInternal(ctx, err, "error updating received share"),
Status: status.NewInternal(ctx, err, "Stat failed on the resource path for the app provider: "+req.Ref.GetPath()),
}, nil
}

Expand All @@ -86,6 +79,9 @@ func (s *svc) OpenFileInAppProvider(ctx context.Context, req *providerpb.OpenFil
}, nil
}

// make sure req.Ref is filled in with the required storageid + opaque fileid, not with the path
req.Ref.Spec = &storageprovider.Reference_Id{Id: statRes.Info.Id}

appProviderClient, err := pool.GetAppProviderClient(provider.Address)
if err != nil {
err = errors.Wrap(err, "gateway: error calling GetAppProviderClient")
Expand All @@ -96,7 +92,7 @@ func (s *svc) OpenFileInAppProvider(ctx context.Context, req *providerpb.OpenFil

res, err := appProviderClient.OpenFileInAppProvider(ctx, req)
if err != nil {
return nil, errors.Wrap(err, "gateway: error calling c.Open")
return nil, errors.Wrap(err, "gateway: error calling OpenFileInAppProvider")
}

return res, nil
Expand Down

0 comments on commit c64497c

Please sign in to comment.