-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 for OC6+ (includes AppFramework) #4459
Conversation
@Raydiation have a look 😉 |
@publicpage - No user logon is expected @NoAdminRequired - the login user requires no admin rights @NoCSRFRequired - the incoming request will not check for CSRF token
* Should you find yourself in need for more methods, simply inherit from this | ||
* class and add your methods | ||
*/ | ||
class API { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wont this be unneeded once we have the proper DI container to access to oc apis? since this is basically a wrapper around the static apis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wont this be unneeded once we have the proper DI container to access to oc apis?
for sure - I'm not yet finally where I want to be - but thanks for keeping a close eye
Will this make it into oC6? |
thats why its named ' ... for oc6+' 😉 |
I figured that much, but now that we have official release dates, priorities Will you make a post about what needs to be changed besides the namespace once |
The fact that OC_ should not be used is pretty old. Actually it gas been announced more that a year ago as we introduced the name space OCP. With oc6 it will be enforced to not use OC_ Anything which is in OCP today will stay there. Any new api will be defined as interfaces and a container/factory is to be used to retrieve the implementation. What comes together with this pr is the core portion of the app framework as developed by Bernhard in the past month. Regarding oc6: among other pieces this pr is the reason for pushing out oc6 |
Thanks for the pointer - I'm currently uncertain if we shall allow public access to the middleware interfaces |
Yes, the current implementation allows you to remove all security checks. Otherwise I find it very handy to be able to add your own middleware. |
Middleware should defenitely be publicly available and changeable. We just have to make sure that people use it right by documenting it properly |
…service hosted within the server container
Thinking about it I might had been able to get the error from the body of the ajax response. Printing an error page when in debug mode is a really stupid idea, when most of the requests are ajaxy. |
OC_VCategories => OC\Tags. Public interface and getter in server container
Test passed. |
Test passed. |
@DeepDiver1975 have you had a chance to look at the I will look into making the server container testable. Hopefully I'll push it later today. |
|
@tanghus can I see the code? Is it pushed to the contacts repo? |
@DeepDiver1975 When registering middleware in https://github.com/owncloud/contacts/blob/oc_appframework/lib/dispatcher.php#L36 Still WiP while adjusting to this PR. And this bt is more correct I think
|
Nice finding! I have to double check where this parameter is coming from |
There it is - looks like wrong initialization order |
registerMiddleware not lazy enough |
@tanghus can you please retest? THX |
Now with a warning too ;)
And still not lazy enough ;)
|
👍 |
Test passed. |
We decided to merge this now into master in order to get this going regarding OC6 feature freeze. |
Public API for OC6+ (includes AppFramework)
Public API
The basic concept of the public api is based on following criteria:
The public API requires further enhancement - which is documented in #4863
AppFramework
This pull request contains an adopted sub set of the AppFramework wrapped with an public interface.
At this point in time the public access is very limited and will be extended in the upcoming days.
FYI: @karlitschek @butonic @Raydiation @bartv2 @icewind1991