webUI to OCS Share API #17143

rullzer · 2015-06-24T14:14:38Z

Currently the web interface uses the ajax/share.php endpoint to do all its sharing magic. Of course this should also use the OCS Share API so there is only 1 code path to maintain.

This PR aims to move the webUI to the OCS Share API. Most stuff is done

Done:

Create share
Unshare
Modify share
Link sharing stuff

Todo:

Port remaining calls to share.php to OCSShare (retrival of share list
Fix unit tests to also return token
Extend unit tests (check for correct url etc)
Less code duplication in ocs-share.js

Currently I created a new file ocs-share.js to make share.js a bit less messy. If desired this can of coursed trivially be merged with share.js

Lets see what jenkins has to say at the moment.

scrutinizer-notifier · 2015-06-24T14:14:55Z

A new inspection was created.

ghost · 2015-06-24T15:20:35Z

🚀 Test PASSed.🚀

DeepDiver1975 · 2015-06-24T15:47:07Z

core/js/ocs-share.js

+					} else {
+						var msg = t('core', 'Error');
+					}
+					OC.dialogs.alert(msg, t('core', 'Error'));


no dialog - please use oc.notifications

Ah yes, this is just old copied code... but you are right. Added to the TODO

karlitschek · 2015-06-24T21:35:55Z

Very nice! 👍
@schiesbn @PVince81 FYI

schiessle · 2015-06-25T08:40:50Z

Nice idea. But keep in mind the as of today the OCS API is a files sharing api, located in the files_sharing app while the share dialog is in core and should also work without the files_sharing app, e.g. for calendar, contacts, etc.

The first step would be to transform the OCS files sharing api to a general OCS sharing API in core if we want to use it as universal API for sharing. So probably the best way to deal with it is to implement it while we clean up the sharing code in core.

PVince81 · 2015-06-25T08:59:28Z

Hmmm, it sounds rather that we should focus on the share panel integration in the right sidebar: #16308

To proceed, we could either rewrite the dialog from scratch or reuse the OC.Share code from core/js/share.js and refactor it into a new class/file. But leave the original one in place.

This means that the new dialog/panel would use the OCS Share API already while the old dialog is left to use the ajax calls, until we can remove it (once calendar/contacts and co are adapted).

@rullzer so what you could do now is make a copy of your changes and move them into a new class/file.

DeepDiver1975 · 2015-06-25T10:05:41Z

This means that the new dialog/panel would use the OCS Share API already while the old dialog is left to use the ajax calls, until we can remove it (once calendar/contacts and co are adapted).

Well - with the idea in mind (as discussed) to have a file-only sharing code in core we have to remove this old code pieces AND allocate time to help out on the apps to fix it with the app devs together.

No need to hold back old crappy code - we basically already agreed on breaking this API the hard way in 8.2 @karlitschek FYI

schiessle · 2015-06-25T10:38:12Z

I agree with @DeepDiver1975 let's hold this back until we know how to move on with sharing in general.

Just as a side note: I'm not sure if it is a good idea to have files-only sharing code in core. Currently I think more in the direction of what we have done with encryption 2.0. Have a general share API in core which provides the minimal share functionality which is the same for all share types and let apps register their "share provider" which gets then called from core and can handle all the share type specific parts. But that's something we need to discuss in detail before we move forward. Let's sit together during the ownCloud conference (if this fits our overall timetable for sharing) and come up with some first ideas, create a github issue with it and then move forward.

DeepDiver1975 · 2015-06-25T11:15:54Z

Just to not forget this: caldav comes with thier own sharing mechanisms - might be worth to have a look at this ...

http://sabre.io/dav/caldav-sharing/

PVince81 · 2015-07-15T17:22:46Z

Another problem: if we want to rewrite the share dialog inside the new sidebar (#17665), should the new one still use the old share APIs ? Or is it an opportunity to make it use the OCS share APIs already ?

PVince81 · 2015-07-15T17:40:23Z

@DeepDiver1975 I had the feeling that the share info was already scheduled for 8.2 to be in the sidebar, so not sure if there will be time to discuss API stuff until then.

Since the OCS Share API (file specific) is already there and deprecating it will take 3 years anyway, it might still be worth using it now instead of waiting for the Share API 2 redesign.

@rullzer it just came to me that your PR will break calendar and contact sharing.
But we should be able to reuse pieces of it when doing the file specific share section in the right sidebar: #17665

rullzer · 2015-07-15T20:22:09Z

@PVince81 yeah I know my change breaks calendar and contact sharing.

What I think we should do in this case abstract away. So it is getting future proof. We should provide an OCS API class in core OCP.OCS, which allows basic OCS operations in core. This would allow proper parsing of return stuff etc.

Then files sharing could just implement sharing for files in its own functions. Using OCP.OCS. (So calendar and contacts sharing use the old stuff).

This leaves the interapp dependency issue. Since either we have specific sharing code in core or each app needs to implent sharing themself...

On a related note if we break sharing and introduce sharing 2.0 this will hopefully be more generic. So then implementing sharing in your app would be a mater of picking the right share type ('file', 'contact' etc). and a uniq id (e.g. 'path'). But that is all to be discussed in Berlin.

rullzer · 2015-07-15T20:26:06Z

Ok that was a little less of a clear story then I would have liked :p

@PVince81 in short I would say yes we should reuse parts. Since we officially do not allow inter app dependencies. So if we have an OCP.OCS in core we could just implement the files sharing js in the files_sharing app (which is better separation anyway). That way files sharing can use the OCS API. While leaving calender and contacts sharing working.

labkode · 2015-07-29T20:12:47Z

As discussed today with @cmonteroluque , @MTRichards ,@DeepDiver1975 ,@karlitschek and @dragotin the server should be API-oriented and both APIs must converge into one, ant this one has to be the OCS Share API.

The main problem of the internal Share API is the inefficiency of the operations because they are file ID based and not share ID based like the OCS API.

See this example of deleting a share:

This example illustrates the differences:

curl 'https://xxxx.cern.ch/index.php/core/ajax/share.php' --data 'action=unshare&itemType=file&itemSource=4625167871377408&shareType=3&shareWith='

OCS Share

curl -k -i -X DELETE 'https://xxxx.cern.ch/ocs/v1.php/apps/files_sharing/api/v1/shares/7783' -u guest

When retrieving information about a share both APIs offer the same result:

Internal share

{  
   "id":4821,
   "item_type":"folder",
   "item_source":"2476478",
   "item_target":"\/2476478",
   "parent":null,
   "share_type":0,
   "share_with":"gonzaleh",
   "uid_owner":"lmascett",
   "file_source":2476478,
   "file_target":"\/shared_with_hugo",
   "permissions":15,
   "stime":1433406815,
   "expiration":null,
   "token":null,
   "mail_send":"0",
   "path":"files\/shared_with_hugo",
   "storage":49,
   "share_with_displayname":"gonzaleh",
   "displayname_owner":"lmascett"
}

OCS API

  <element>
   <id>7867</id>
   <item_type>folder</item_type>
   <item_source>4082629</item_source>
   <parent/>
   <share_type>0</share_type>
   <share_with>gonzaleh</share_with>
   <file_source>4082629</file_source>
   <file_target>/Projects (#4082629)</file_target>
   <permissions>15</permissions>
   <stime>1438154313</stime>
   <expiration/>
   <token/>
   <mail_send>0</mail_send>
   <uid_owner>ourense</uid_owner>
   <path>/Projects</path>
   <storage>5</storage>
   <share_with_displayname>gonzaleh</share_with_displayname>
   <displayname_owner>ourense</displayname_owner>
  </element>

and both suffer the same problem.

The storage field is retrieved along with the share information after doing a JOIN over the oc_filecache.
If the storage ID would be used that info would make sense, but actual OC server design does not consider the direct access to a particular storage using the storage ID, so this added field is useless and expensive.

OwnCloud suffers the problem of being relative-path based to the common virtual filesystem, so every operation that use a path, internally must resolve this path to the corresponding storage.

I will clarify this design aspect in the draft we are working to present to you as we agreed.

Cheers

PVince81 · 2015-09-01T13:45:26Z

Blocked by #18185

rullzer · 2015-10-28T08:33:16Z

Let me close this PR. As we should do it all properly in the item model now anyway.

rullzer added 7 commits June 23, 2015 15:16

Deleting of existing shares is done using OCS Share API

bfb73e2

Webfrontend can now also add shares using OCS

ec5d9f0

Sharing (and unsharing) by link now also uses OCS

fe98312

Setting public upload works via OCS

adcdce1

password and permissions are now also set via OCS

6abc90f

Setting expiration date is now also done via OCS

39ac5b6

Fix unit tests

c3f43c3

rullzer added enhancement 2 - Developing feature:sharing labels Jun 24, 2015

rullzer added this to the 8.2-next milestone Jun 24, 2015

DeepDiver1975 reviewed Jun 24, 2015
View reviewed changes

PVince81 mentioned this pull request Jul 3, 2015

Properly send expiration date with all share.php calls #11674

Closed

1 task

rullzer added 0 - Backlog and removed 2 - Developing labels Jul 15, 2015

PVince81 mentioned this pull request Jul 15, 2015

Move share dropdown into the right sidebar in the files app #17665

Closed

8 tasks

DeepDiver1975 mentioned this pull request Aug 10, 2015

Share dialog files sidebar #18185

Merged

5 tasks

rullzer modified the milestones: 9.0-next, 8.2-current Sep 14, 2015

rullzer closed this Oct 28, 2015

rullzer deleted the webshare2ocs branch October 28, 2015 08:33

rullzer removed this from the 9.0-current milestone Oct 28, 2015

lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

webUI to OCS Share API #17143

webUI to OCS Share API #17143

rullzer commented Jun 24, 2015

scrutinizer-notifier commented Jun 24, 2015

ghost commented Jun 24, 2015

DeepDiver1975 Jun 24, 2015

rullzer Jun 24, 2015

karlitschek commented Jun 24, 2015

schiessle commented Jun 25, 2015

PVince81 commented Jun 25, 2015

DeepDiver1975 commented Jun 25, 2015

schiessle commented Jun 25, 2015

DeepDiver1975 commented Jun 25, 2015

PVince81 commented Jul 15, 2015

PVince81 commented Jul 15, 2015

rullzer commented Jul 15, 2015

rullzer commented Jul 15, 2015

labkode commented Jul 29, 2015

PVince81 commented Sep 1, 2015

rullzer commented Oct 28, 2015

webUI to OCS Share API #17143

webUI to OCS Share API #17143

Conversation

rullzer commented Jun 24, 2015

scrutinizer-notifier commented Jun 24, 2015

ghost commented Jun 24, 2015

DeepDiver1975 Jun 24, 2015

Choose a reason for hiding this comment

rullzer Jun 24, 2015

Choose a reason for hiding this comment

karlitschek commented Jun 24, 2015

schiessle commented Jun 25, 2015

PVince81 commented Jun 25, 2015

DeepDiver1975 commented Jun 25, 2015

schiessle commented Jun 25, 2015

DeepDiver1975 commented Jun 25, 2015

PVince81 commented Jul 15, 2015

PVince81 commented Jul 15, 2015

rullzer commented Jul 15, 2015

rullzer commented Jul 15, 2015

labkode commented Jul 29, 2015

PVince81 commented Sep 1, 2015

rullzer commented Oct 28, 2015