Improve json marshalling of share protobuf messages

.github/config.yml

@@ -9,6 +9,9 @@ updateDocsWhiteList:
  - tests-only
  - Tests-Only
  - Build-deps
+ - docs-only
+ - Docs-only
+ - Docs-Only
 
 updateDocsTargetFiles:
  - changelog/unreleased/

changelog/unreleased/improve-share-json-marshal.md

@@ -0,0 +1,9 @@
+Enhancement: Improve json marshalling of share protobuf messages
+
+Protobuf oneof fields cannot be properly handled by the native json marshaller,
+and the protojson package can only handle proto messages. Previously, we were
+using a workaround of storing these oneof fields separately, which made the code
+inelegant. Now we marshal these messages as strings before marshalling them via
+the native json package.
+
+https://github.com/cs3org/reva/pull/1655

examples/ocmd/ocmd-server-1.toml

@@ -1,16 +1,16 @@
 [shared]
 gatewaysvc = "localhost:19000"
 
-[registry]
-driver = "static"
-
-[registry.static]
-services = ["authprovider","userprovider"]
-
-[registry.static.authprovider]
-bearer = ["localhost:0123"]
-basic = ["localhost:1234"]
-publiclink = ["localhost:9876"]
+# [registry]
+# driver = "static"
+#
+# [registry.static]
+# services = ["authprovider","userprovider"]
+#
+# [registry.static.authprovider]
+# bearer = ["localhost:0123"]
+# basic = ["localhost:1234"]
+# publiclink = ["localhost:9876"]
 
 [grpc]
 address = "0.0.0.0:19000"

internal/grpc/services/gateway/gateway.go

@@ -82,9 +82,6 @@ func (c *config) init() {
 
  // if services address are not specified we used the shared conf
  // for the gatewaysvc to have dev setups very quickly.
-
- // we're commenting this line to showcase the fact that now we don't want to point to an ip address but rather
- // resolve an ip address from a name.
  c.AuthRegistryEndpoint = sharedconf.GetGatewaySVC(c.AuthRegistryEndpoint)
  c.StorageRegistryEndpoint = sharedconf.GetGatewaySVC(c.StorageRegistryEndpoint)
  c.AppRegistryEndpoint = sharedconf.GetGatewaySVC(c.AppRegistryEndpoint)

pkg/cbox/user/rest/rest.go

@@ -123,7 +123,7 @@ func New(m map[string]interface{}) (user.Manager, error) {
 
 func (m *manager) getUserByParam(ctx context.Context, param, val string) (map[string]interface{}, error) {
  url := fmt.Sprintf("%s/Identity?filter=%s:%s&field=upn&field=primaryAccountEmail&field=displayName&field=uid&field=gid&field=type",
- m.conf.APIBaseURL, param, val)
+ m.conf.APIBaseURL, param, url.QueryEscape(val))
  responseData, err := m.apiTokenManager.SendAPIGetRequest(ctx, url, false)
  if err != nil {
  return nil, err
@@ -225,7 +225,6 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User
 }
 
 func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*userpb.User, error) {
- value = url.QueryEscape(value)
  opaqueID, err := m.fetchCachedParam(claim, value)
  if err == nil {
  return m.GetUser(ctx, &userpb.UserId{OpaqueId: opaqueID})

pkg/share/manager/json/json.go

@@ -26,8 +26,6 @@ import (
  "sync"
  "time"
 
- grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
- userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
  collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
  provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
  typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
@@ -91,23 +89,20 @@ func loadOrCreate(file string) (*shareModel, error) {
  return nil, err
  }
 
- m := &shareModel{}
- if err := json.Unmarshal(data, m); err != nil {
- err = errors.Wrap(err, "error decoding data to json")
+ j := &jsonEncoding{}
+ if err := json.Unmarshal(data, j); err != nil {
+ err = errors.Wrap(err, "error decoding data from json")
  return nil, err
  }
 
- // There are discrepancies in the marshalling of oneof fields, so these need
- // to be stored separately
- for i := range m.Grantees {
- id := m.Grantees[i].(map[string]interface{})
- if m.Shares[i].Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER {
- m.Shares[i].Grantee.Id = &provider.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: id["opaque_id"].(string), Idp: id["idp"].(string)}}
- } else if m.Shares[i].Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP {
- m.Shares[i].Grantee.Id = &provider.Grantee_GroupId{GroupId: &grouppb.GroupId{OpaqueId: id["opaque_id"].(string), Idp: id["idp"].(string)}}
+ m := &shareModel{State: j.State}
+ for _, s := range j.Shares {
+ var decShare collaboration.Share
+ if err = utils.UnmarshalJSONToProtoV1([]byte(s), &decShare); err != nil {
+ return nil, errors.Wrap(err, "error decoding share from json")
  }
+ m.Shares = append(m.Shares, &decShare)
  }
- m.Grantees = nil
 
  if m.State == nil {
  m.State = map[string]map[string]collaboration.ShareState{}
@@ -118,28 +113,27 @@ func loadOrCreate(file string) (*shareModel, error) {
 }
 
 type shareModel struct {
- file string
- State map[string]map[string]collaboration.ShareState `json:"state"` // map[username]map[share_id]ShareState
- Shares []*collaboration.Share `json:"shares"`
- Grantees []interface{} `json:"grantees"`
+ file string
+ State map[string]map[string]collaboration.ShareState `json:"state"` // map[username]map[share_id]ShareState
+ Shares []*collaboration.Share `json:"shares"`
+}
+
+type jsonEncoding struct {
+ State map[string]map[string]collaboration.ShareState `json:"state"` // map[username]map[share_id]ShareState
+ Shares []string `json:"shares"`
 }
 
 func (m *shareModel) Save() error {
- temp := *m
- temp.Grantees = []interface{}{}
- temp.Shares = []*collaboration.Share{}
- for i := range m.Shares {
- s := *m.Shares[i]
- if s.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER {
- temp.Grantees = append(temp.Grantees, s.Grantee.GetUserId())
- } else if s.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP {
- temp.Grantees = append(temp.Grantees, s.Grantee.GetGroupId())
+ j := &jsonEncoding{State: m.State}
+ for _, s := range m.Shares {
+ encShare, err := utils.MarshalProtoV1ToJSON(s)
+ if err != nil {
+ return errors.Wrap(err, "error encoding to json")
  }
- s.Grantee = &provider.Grantee{Type: s.Grantee.Type}
- temp.Shares = append(temp.Shares, &s)
+ j.Shares = append(j.Shares, string(encShare))
  }
 
- data, err := json.Marshal(temp)
+ data, err := json.Marshal(j)
  if err != nil {
  err = errors.Wrap(err, "error encoding to json")
  return err

tools/check-changelog/main.go

@@ -32,7 +32,7 @@ import (
 )
 
 // Case-insensitive list of PRs for which changelog enforcement needs to be skipped
-var skipTags = []string{"[tests-only]", "[build-deps]"}
+var skipTags = []string{"[tests-only]", "[build-deps]", "[docs-only]"}
 
 func skipPR(prID int) bool {
  ctx := context.Background()