name of received share appears as 'home' once accepted

[michielbdejong]

instead of .e.g. '/asdf'

[michielbdejong]

This stop gap makes it work: https://github.com/cs3org/reva/commit/c466de2b1f41a093f6a1ba82bc3b2509f730d7ce @yasharpm can you see if you can find a way to achieve the same by doing some operation on info.Path?

[yasharpm]

This is a sample of the share object that the provider sends to the receiver via OCM

'share' => 
  array (
    'shareWith' => 'marie@https://nc2.docker',
    'shareType' => 'user',
    'name' => 'inner2',
    'resourceType' => 'file',
    'description' => '',
    'providerId' => 11,
    'owner' => 'einstein@https://nc1.docker/',
    'ownerDisplayName' => 'einstein',
    'sharedBy' => 'einstein@https://nc1.docker/',
    'sharedByDisplayName' => 'einstein',
    'protocol' => 
    array (
      'name' => 'webdav',
      'options' => 
      array (
        'sharedSecret' => '8DlbywvAGmPz00I',
        'permissions' => '{http://open-cloud-mesh.org/ns}share-permissions',
      ),
    ),
  ),
)

This shows that path is never needed to be sent for file sharing via the OCM. So I am going to get rid of the path entirely and only send the file name.

[yasharpm]

Turns out filepath.Base actually returns the last element of path. The only part that the code got wrong was that it was trying to get the file name from the path. So passing req.ResourceId.OpaqueId as the parameter for Base fixed the issue.

[yasharpm]

@michielbdejong I am not authorized to push into the sciencemesh-dev branch.

Change line 269 in file internal/grpc/services/ocmshareprovider/ocmshareprovider.go into

Name:          filepath.Base(req.ResourceId.OpaqueId),

and this issue will be resolved.

[yasharpm]

ah and don't forget to add the dependency first in line 25: "path/filepath"

[glpatcern]

A couple of observations here for @yasharpm as you're getting into OCM: 1) in the OCM v1.1 - which we're aiming for in this implementation! - you do need to pass the full path. As a matter of fact, the legacy way of passing an OCM share is incomplete and navigating a sub-path is still unclear. 2) req.ResourceId.OpaqueId is opaque and its content shouldn't be assumed to contain file name. It turns out that with the localfs storage driver - the one we all use for demos, but the one that nobody will ever use in production - the opaqueId looks like fileid-<path>, but again this is an implementation detail.

[gmgigi96]

This shows that path is never needed to be sent for file sharing via the OCM. So I am going to get rid of the path entirely and only send the file name.

Saying that, as already said by @glpatcern, the localfs storage driver (particularly the localhome version) is bugged, this is already the case in reva master. We are sending only the file name, as you can see in the code https://github.com/cs3org/reva/blob/507e96e4346c68230b48879b28c231e4d8e68fc3/internal/grpc/services/ocmshareprovider/ocmshareprovider.go#L268

[yasharpm]

in the OCM v1.1 - which we're aiming for in this implementation! - you do need to pass the full path.

I don't think this is a realistic goal to achieve since you would need to get pull requests into both NC and OC. Considering our time limitations and the fact that OCM v1.1 is not even merged yet let alone being implemented by someone for OC and NC and then the time consuming process of getting the pull requests accepted I don't think this can be done in any time window smaller than at least a couple of months.

req.ResourceId.OpaqueId is opaque and its content shouldn't be assumed to contain file name.

As far as I understand filepath.Base(info.Path) uses info which comes from statRes which is set by only passing req.ResourceId to a function called Stat here. So everything is coming from req.ResourceId anyways. Unless there is another field with a non-opaque path in ResourceId I think info.Path doesn't give us any extra information. I also tried to find the implementation of the Stat function. The best I could get was here which is referring to the OpaqueId. I have no idea if I am looking at the right place to be honest. Reva code is hard to read!

We are sending only the file name

I think info.Path only contains the path and not the file name itself. And so the Base function resolves to the latest element in the path which is home which is undesired and the subject of this issue.

@glpatcern @gmgigi96 what do you think?

[glpatcern]

I don't think this is a realistic goal to achieve since you would need to get pull requests into both NC and OC

I realize I was not clear myself: I meant we're aiming to OCM v1.1 in Reva, and in any Reva-to-Reva communication. Of course NC and OC are out of scope, they will hopefully implement OCM v1.1 whenever it matches their work plan. OCM v1.1 will be merged soon anyway... and Reva is already 99% compliant.

For the rest, Gianmaria is also writing a comment - I acknowledge browsing the Reva code is hard (it is for me as well), anyway the decomposedfs is not implemented in Reva master and the localfs is incomplete/buggy. The "real" storage provider drivers are the nextcloud one and the eos one.

[gmgigi96]

We are sending only the file name

I think info.Path only contains the path and not the file name itself. And so the Base function resolves to the latest element in the path which is home which is undesired and the subject of this issue.

info.Path is supposed (according to the CS3APIs) to have the path of the resource, that also contains as last element the filename. As already said in my previous comment, localfs + localhome (used in general for tests) might have some bugs, so that the path could not be correctly resolved in some cases. Because the name in the OCM share is meant to be just a tag to the share for the end user in a web UI, this can have whatever value. I know it's annoying seeing home as name, but this is just a bug in the 2 storage drivers. Saying that, I think you can forget this problem for now.

[yasharpm]

Of course NC and OC are out of scope

Do you mean that we are not going to use NC or OC in our Reva presentation? If we are, I am not sure that the OCM end points of NC or OC can handle a path for a filename in their input. If we are not, then the work that PonderSource is doing is irrelevant for the upcoming deadline and you didn't need to wait for us to complete anything. @michielbdejong are we out of scope? :-)

[yasharpm]

Saying that, I think you can forget this problem for now.

Let me quickly double check what the receiver side gets from Reva. The issue to my understanding is that you share a file named for example welcome.txt and it shows up as home on the receiving end. If it is the case, we can not ignore it.

[glpatcern]

@yasharpm the work you / PonderSource are doing is not at all out of scope. Let's have a call whenever you wish, it does not help to write further comments as I see I'm giving for granted a lot of context and as you're relatively new you don't know what has happened.

[gmgigi96]

Saying that, I think you can forget this problem for now.

Let me quickly double check what the receiver side gets from Reva. The issue to my understanding is that you share a file named for example welcome.txt and it shows up as home on the receiving end. If it is the case, we can not ignore it.

I was doing some tests with the configurations in examples/ocmd, and the name is correctly the filename of the resource. You can also check it out in the reva doc https://reva.link/docs/tutorials/share-tutorial/#5-2-2-access-the-list-of-received-shares.

{
   "id":{
      "opaqueId":"ef05c999-8ae2-41af-ba0d-a886b061011f"
   },
   "name":"my-folder",
   "resourceId":{
      "opaqueId":"123e4567-e89b-12d3-a456-426655440000:fileid-einstein%2Fmy-folder"
   },
   "grantee":{
      "type":"GRANTEE_TYPE_USER",
      "userId":{
         "idp":"cesnet.cz",
         "opaqueId":"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c",
         "type":"USER_TYPE_PRIMARY"
      }
   },
[..]

1. Are you running the latest reva on master?
2. Which storage drivers are you using?

[yasharpm]

@michielbdejong I am dropping this issue at this point. Going with the logic that existed i.e. filepath.Base(info.Path), I created folders outer/inner/ on nc1 and shared it with nc2 via Reva. nc2 controller still receives home as name.

If info.Path contains the full path, we would expect inner as the value for name. @glpatcern believes we should be receiving outer/inner which is not the case for now. What I believe we should aim for is inner. What your current commit req.ResourceId.OpaqueId[len("fileid-/home"):] returns is outer/inner which @glpatcern and @gmgigi96 don't like the approach.

I would try to log the actual value for info.Path and ResourceId next but as I have other tasks planned for today, I am not going to chase this any longer.

[gmgigi96]

I can help you if you answer my questions above :) Also, which is the configuration you are using? I cannot reproduce your issue. For me the path is correct, and hence the name in the share.

[michielbdejong]

Ah, right! info.Path comes from info := statRes.Info which comes from statRes, err := s.gateway.Stat(ctx, &providerpb.StatRequest{ ... so since we are using pkg/storage/fs/nextcloud as our storage driver, we can fix this in https://github.com/pondersource/nc-sciencemesh/blob/main/lib/Controller/RevaController.php#L200 And then revert the stop gap entirely. That should be the desired course of action, I think.

Sorry for the whole discussion and confusion about OCM versions in Reva and NC/OC-10!