-
Notifications
You must be signed in to change notification settings - Fork 18
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
add restful API #458
base: master
Are you sure you want to change the base?
add restful API #458
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,91 @@ | ||||||||||||||||||||||||||||||
<?php | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
declare(strict_types=1); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||
* @license GNU AGPL version 3 or any later version | ||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||
* This program is free software: you can redistribute it and/or modify | ||||||||||||||||||||||||||||||
* it under the terms of the GNU Affero General Public License as | ||||||||||||||||||||||||||||||
* published by the Free Software Foundation, either version 3 of the | ||||||||||||||||||||||||||||||
* License, or (at your option) any later version. | ||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||
* This program is distributed in the hope that it will be useful, | ||||||||||||||||||||||||||||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||||||||||||||||||||||||||||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||||||||||||||||||||||||||||||
* GNU Affero General Public License for more details. | ||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||
* You should have received a copy of the GNU Affero General Public License | ||||||||||||||||||||||||||||||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | ||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
namespace OCA\Recommendations\Controller; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
use OCA\Recommendations\AppInfo\Application; | ||||||||||||||||||||||||||||||
use OCA\Recommendations\Service\RecommendationService; | ||||||||||||||||||||||||||||||
use OCP\AppFramework\ApiController; | ||||||||||||||||||||||||||||||
use OCP\AppFramework\Http\DataResponse; | ||||||||||||||||||||||||||||||
use OCP\IConfig; | ||||||||||||||||||||||||||||||
use OCP\IRequest; | ||||||||||||||||||||||||||||||
use OCP\IUserManager; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
class RecommendationApiController extends ApiController { | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
/** @var IConfig */ | ||||||||||||||||||||||||||||||
private $config; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
/** @var IUserManager */ | ||||||||||||||||||||||||||||||
private $userManager; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
/** @var RecommendationService */ | ||||||||||||||||||||||||||||||
private $service; | ||||||||||||||||||||||||||||||
Comment on lines
+34
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
public function __construct($appName, IRequest $request, IConfig $config, | ||||||||||||||||||||||||||||||
IUserManager $userManager, | ||||||||||||||||||||||||||||||
RecommendationService $service, $userId) { | ||||||||||||||||||||||||||||||
Comment on lines
+44
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||
parent::__construct($appName, $request, 'GET'); | ||||||||||||||||||||||||||||||
$this->config = $config; | ||||||||||||||||||||||||||||||
$this->service = $service; | ||||||||||||||||||||||||||||||
$this->userManager = $userManager; | ||||||||||||||||||||||||||||||
$this->userId = $userId; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||
* @CORS | ||||||||||||||||||||||||||||||
* @NoCSRFRequired | ||||||||||||||||||||||||||||||
* @NoAdminRequired | ||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||
public function index(): DataResponse { | ||||||||||||||||||||||||||||||
$user = $this->userManager->get($this->userId); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
$response = []; | ||||||||||||||||||||||||||||||
$response['enabled'] = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'enabled', 'true') === 'true'; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if ($response['enabled']) { | ||||||||||||||||||||||||||||||
$response['recommendations'] = $this->service->getRecommendations($user); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
return new DataResponse( | ||||||||||||||||||||||||||||||
$response | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||
* @CORS | ||||||||||||||||||||||||||||||
* @NoCSRFRequired | ||||||||||||||||||||||||||||||
* @NoAdminRequired | ||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||
public function always(): DataResponse { | ||||||||||||||||||||||||||||||
$user = $this->userManager->get($this->userId); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
$response = [ | ||||||||||||||||||||||||||||||
'enabled' => $this->config->getUserValue($user->getUID(), Application::APP_ID, 'enabled', 'true') === 'true', | ||||||||||||||||||||||||||||||
'recommendations' => $this->service->getRecommendations($user), | ||||||||||||||||||||||||||||||
]; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
return new DataResponse( | ||||||||||||||||||||||||||||||
$response | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} |
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.
I think this should be using OCSController instead: https://docs.nextcloud.com/server/latest/developer_manual/basics/controllers.html#ocs
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.
The first sentence in the info box is saying: "This is purely for compatibility reasons. If you are planning to offer an external API, go for a REST APIs instead.".
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.
hmm yeah, not completely sure. I remember seeing some new stuff using OCSController. It's probably better to confirm with someone involved in Nextcloud since a longer time :)
@ChristophWurst do you know what is the recommended way?
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.
I'm not very experienced with external API. @nickvergessen is the expert here :)
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.
Definitely should be an OCS controller, so all our clients can hook it up and use it in the future
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.
Well then what is this API needed for?
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.
As per #457 they want to use it in the Outlook plugin, so yeah that is exactly why we have OCS apis and it should be used in this case.
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.
I transformed it to an OCS controller, but I don't know who will maintain it in the future 😉
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.
@sualko please document the API at https://docs.nextcloud.com/server/latest/developer_manual/client_apis/OCS/index.html.
We'll allow the API but there are no guarantees for future compatibility, hence please flag as experimental.
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.
Also @sualko can you ping us or @ChristophWurst respectively when this is fully integrated and "stable". Reason is we would then merge it when the docs are in but wouldn't publish a new release yet, so in case you need to do some more changes during integration that could still be done and there wouldn't be any scenario that would require migration logic for app upgrades, since this wasn't published yet, which would safe everybody some time.
If this is already finished and works fine from your perspective, please let us know.