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.

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

Public API OC6 - what's missing #4863

Closed
18 of 27 tasks
DeepDiver1975 opened this issue Sep 16, 2013 · 53 comments
Closed
18 of 27 tasks

Public API OC6 - what's missing #4863

DeepDiver1975 opened this issue Sep 16, 2013 · 53 comments

Comments

@DeepDiver1975
Copy link
Member

This issue is to be used as a todo list for public apis which are missing as of today.
Please add a comment on what we miss and We will add this to the list below.

For each missing api a pull request is to be opened - once merged we can check the item on the todo list.

How to implement a public api?

  1. Create an interface in the public namespace within lib/public which acts as the entry point for accessing the functionality e.g. https://github.com/owncloud/core/blob/master/lib/public/ipreview.php#L39
  2. Add a getter to the IServerContainer - e.g. https://github.com/owncloud/core/blob/master/lib/public/iservercontainer.php#L64
  3. Implement the api within the private namespace (core/lib/private) e.g.https://github.com/owncloud/core/blob/master/lib/private/previewmanager.php
  4. Add the implementation of the getter to \OC\Server - please use the provided methods registerService() and query() because with this approach only those classes will be created which are going to be used
    return $this->query('PreviewManager');

    $this->registerService('PreviewManager', function($c){

Mainly important on all public classes, interfaces and methods: add proper PHPDoc comments

Missing apis

@DeepDiver1975
Copy link
Member Author

Regarding OC_FileProxy:

OC_FileProxy:

  • $enabled
    Needed when reading and writing to storage obtained with
    \OCP\Files::getStorage('appName') when encryption is enabled.
    Maybe this can be done in the getStorage() call?

Thanks @tanghus

@DeepDiver1975
Copy link
Member Author

@owncloud/core-developers One of the rare case where a notification to all seams legit 😉

@BernhardPosselt
Copy link
Contributor

OC_Appconfig is needed only partly i think. Most of the useful stuff is in OCP\Config

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

Session

Do we have a Session class? The PHP session is already available in Request.

(and kill session from IRequest)

Shouldn't that be separate from Request?

@DeepDiver1975
Copy link
Member Author

Do we have a Session class?

https://github.com/owncloud/core/blob/master/lib/session/session.php - we need to create an interface from it

Shouldn't that be separate from Request?

Exactly - $_SESSION is available via the Request object - but this global var should not be used (actually it doesn't work in conjunction with our own session implementation)

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

Ah, I was looking only in /lib, not the sub-folders.

Exactly - $_SESSION is available via the Request object - but this global var should not be used (actually it doesn't work in conjunction with our own session implementation)

Should it be removed from Request, changed to return an instance of ISession or...?

Edit: Now I read your previous comment again, and see that I misunderstood it. I thought you meant kill session as in destroy current session, so that answers my my own question ;)

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

I think dogfooding is the best approach to iron out what's missing/inconvenient, but I can't quite grasp how to implement it in an app. What do I need to do to port an app from OCA\AppFramework?
Maybe the differences are smaller than I think :)

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

More or less copy/paste from the ML:

Now in order to get the Files view you will have to do something like:

$root = $server->getRootFolder();
$files = $root->get('/files');

If so adding a getUserFolder() is a good idea. Maybe getRootFolder() should
return the view to /files, otherwise an app has access to other apps storage
area.
Then get app storage via getStorage()

Does Server know the current app? That would be very handy for e.g. getStorage.

@DeepDiver1975
Copy link
Member Author

What do I need to do to port an app from OCA\AppFramework?
Maybe the differences are smaller than I think :)

That's another open topic and we need to add this to the documentation.
I will try to come up with an example pretty soon in order to show what it will look like.

I think dogfooding is the best approach to iron out what's missing/inconvenient

Exactly

@icewind1991
Copy link
Contributor

If so adding a getUserFolder() is a good idea. Maybe getRootFolder() should
return the view to /files, otherwise an app has access to other apps storage
area.

getRootFolder should return the absolute root folder, there are more then enough valid use cases to access other apps' files.

Adding a getUserFolder is a good idea

@bartv2
Copy link
Contributor

bartv2 commented Sep 16, 2013

I'll take a stab at adding interfaces for the app calls

@DeepDiver1975
Copy link
Member Author

Adding a getUserFolder is a good idea

Awaiting the pull request 😉

@DeepDiver1975
Copy link
Member Author

I'll take a stab at adding interfaces for the app calls

thanks a lot!

@BernhardPosselt
Copy link
Contributor

ideally you implement ArrayAccess for the session object so nothing should change compared to using $_SESSION

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

Adding a getUserFolder is a good idea

Maybe:

  • getUserFolder(): returns the root if the file view as in user/files
  • getAppFolder(): returns a view specific to the app as in user/app. Basically an alias for OCP\Files::getStorage($app)

Awaiting the pull request 😉

I'm still befuddled by the filesystem, so I'm not sure of the proper way to do it.

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

I will try to come up with an example pretty soon in order to show what it will look like.

Fantabulous :)

@icewind1991
Copy link
Contributor

  • getUserFolder(): returns the root if the file view as in user/files
  • getAppFolder(): returns a view specific to the app as in user/app. Basically an alias for OCP\Files::getStorage($app)

👍

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

getUserFolder(): returns the root if the file view as in user/files

Or getFilesFolder()?

@bartv2
Copy link
Contributor

bartv2 commented Sep 16, 2013

@bartv2
Copy link
Contributor

bartv2 commented Sep 16, 2013

@DeepDiver1975 i'm also missing a todo for the nav menu, or am i missing something. (this should go into the template namespace BTW)

@DeepDiver1975
Copy link
Member Author

any comment on https://gist.github.com/bartv2/6586173

looks good

@DeepDiver1975
Copy link
Member Author

i'm also missing a todo for the nav menu, or am i missing something. (this should go into the template namespace BTW)

task added - thx

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

Shouldn't all Response subclasses be in the public namespace?

@DeepDiver1975
Copy link
Member Author

Shouldn't all Response subclasses be in the public namespace?

yes - thanks for spotting - I missed this

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

Shall I move them? What about the abstract controller class?

@DeepDiver1975
Copy link
Member Author

Shall I move them?

hmm - json and template response are already part of the public namespace
lib/public/appframework/http

@tanghus
Copy link
Contributor

tanghus commented Sep 16, 2013

Yes, only RedirectResponse and DownloadResponse missing. TextDownloadResponse has been killed? It also was a bit overkill to have a class just for a different mimetype.

@tanghus
Copy link
Contributor

tanghus commented Sep 18, 2013

I added the interface to the middleware but there is no way to register the middleware within the container.

I still think having an abstract class would be sensible there :)

I need to add that ...

That would be great

@BernhardPosselt
Copy link
Contributor

@tanghus the server container should not define any middleware at all. Actually i have no idea why its there

@tanghus
Copy link
Contributor

tanghus commented Sep 18, 2013

@Raydiation

the server container should not define any middleware at all.

No that is in the app container.

@MorrisJobke
Copy link
Contributor

OC::getVersionString() - maybe define an interface IUtil?
l10n

State of this?

I checked

template and related functions like nav menu - nav part in #4928
OC_Appconfig - is done with the config interface

because they are merged.

@karlitschek
Copy link
Contributor

we are running out of time. let's move this to oC 7

@jancborchardt
Copy link
Member

Did this make it into ownCloud 7 then, or do we need to triage? @karlitschek @DeepDiver1975

@jancborchardt jancborchardt removed this from the ownCloud 7 milestone Aug 19, 2014
@MorrisJobke
Copy link
Contributor

We nned to triage the above list again I think

@PVince81
Copy link
Contributor

Anything here that could make it into OC 8 ?

@DeepDiver1975
Copy link
Member Author

We need to review this once more and see how to continue - to some extent this also depends on requirements/requests from apps on which api they expect ...

@bboule
Copy link

bboule commented Nov 19, 2014

I think we would want to consider things like planned splunk integration... This could be leverage in terms of reporting user clean up etc

@MTRichards thoughts?

Sent from my iPhone

On Nov 18, 2014, at 4:25 PM, Vincent Petry [email protected] wrote:

Anything here that could make it into OC 8 ?


Reply to this email directly or view it on GitHub.

@DeepDiver1975
Copy link
Member Author

Time's up on this - please everybody: open individual issue for missing APIs - THX

@MorrisJobke MorrisJobke removed this from the backlog milestone Jun 26, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests