Skip to content

Warn about the old OCP\API::register construct for OCS #7917

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.

Sign up for GitHub

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

Merged
MorrisJobke merged 1 commit into from
Jan 24, 2018
Merged

Warn about the old OCP\API::register construct for OCS #7917

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ocs/v1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@
OC_App::loadApps();

OC::$server->getRouter()->match('/ocs'.\OC::$server->getRequest()->getRawPathInfo());

sleep(1);
Copy link
Member

@MorrisJobke MorrisJobke Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ... I guess we should then not do this (even if I proposed this), because the Android app for example uses it. On the other side: a good reason to migrate to v2 ;)

@tobiasKaminsky @marinofaggiana @mario @AndyScherzinger @camilasan @juliushaertl Opinions? cc'ing you for the clients

Copy link
Member Author

@rullzer rullzer Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also happens on v2.php ;). Basically this only happens if the old route matches. The match throws an exception if it can't be found etc.

Copy link
Member Author

@rullzer rullzer Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it only does the sleep if the request is one that matches an old OCP\API::register route

Copy link
Member

@tobiasKaminsky tobiasKaminsky Jan 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should migrate, yes. But please give us some time :-)

Copy link
Member Author

@rullzer rullzer Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like i said. This in no way enforces you to use v2. (that is a different PR maybe :P)

Copy link
Member

@MorrisJobke MorrisJobke Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. I checked also all apps in the appstore and this is the only usage:

server/apps/files_external/appinfo/routes.php

Lines 65 to 68 in 0215b00

\OCP\API::register('get',
'/apps/files_external/api/v1/mounts',
array('\OCA\Files_External\Lib\Api', 'getUserMounts'),
'files_external');

Copy link
Member

@MorrisJobke MorrisJobke Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry was wrong - survey_client, survey_server and ocdownloader also use it:

bildschirmfoto 2018-01-17 um 16 35 35

Copy link
Member

@MorrisJobke MorrisJobke Jan 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But anyways - fine by me.

Copy link
Member

@tobiasKaminsky tobiasKaminsky Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what are the ocs path that are old?
Then I can check them on android.

Copy link
Member Author

@rullzer rullzer Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobiasKaminsky nothing for you to do here. This is only the implentation that needs to be adjusted. The endpoints should stay the same

OC::$server->getLogger()->info('This uses an old OCP\API::register construct. This will be removed in a future version of Nextcloud. Please migrate to the OCSController');

return;
} catch (ResourceNotFoundException $e) {
// Fall through the not found
Expand Down