Skip to content

Core controller cleanup #1172

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 3 commits into from
Aug 30, 2016
Merged

Core controller cleanup #1172

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0ed865
UserController does not require Defaults
rullzer Aug 29, 2016
f6423f7
Minor cleanup in core Controllers
rullzer Aug 29, 2016
b1a090f
AvatarController use proper JSONResponse
rullzer Aug 29, 2016
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
Prev Previous commit
AvatarController use proper JSONResponse 
* Do not rely on DataResponse magic. We want JSON so use JSON
* Fix tests
  • Loading branch information
@rullzer
rullzer committed Aug 29, 2016
commit b1a090f3576895f526cd102c43aad56789e6e889
75 changes: 33 additions & 42 deletions core/Controller/AvatarController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@

use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\JSONResponse;
use OCP\Files\File;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
Expand Down Expand Up @@ -111,7 +111,7 @@ public function __construct($appName,
*
* @param string $userId
* @param int $size
* @return DataResponse|DataDisplayResponse
* @return JSONResponse|DataDisplayResponse
*/
public function getAvatar($userId, $size) {
if ($size > 2048) {
Expand All @@ -128,13 +128,13 @@ public function getAvatar($userId, $size) {
$resp->setETag($avatar->getEtag());
} catch (NotFoundException $e) {
$user = $this->userManager->get($userId);
$resp = new DataResponse([
$resp = new JSONResponse([
'data' => [
'displayname' => $user->getDisplayName(),
],
]);
} catch (\Exception $e) {
$resp = new DataResponse([
$resp = new JSONResponse([
'data' => [
'displayname' => '',
],
Expand All @@ -152,25 +152,22 @@ public function getAvatar($userId, $size) {
* @NoAdminRequired
*
* @param string $path
* @return DataResponse
* @return JSONResponse
*/
public function postAvatar($path) {
$files = $this->request->getUploadedFile('files');

$headers = [];

if (isset($path)) {
$path = stripslashes($path);
$userFolder = $this->rootFolder->getUserFolder($this->userId);
$node = $userFolder->get($path);
if (!($node instanceof File)) {
return new DataResponse(['data' => ['message' => $this->l->t('Please select a file.')]], Http::STATUS_OK, $headers);
return new JSONResponse(['data' => ['message' => $this->l->t('Please select a file.')]]);
}
if ($node->getSize() > 20*1024*1024) {
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('File is too big')]],
Http::STATUS_BAD_REQUEST,
$headers
Http::STATUS_BAD_REQUEST
);
}
$content = $node->getContent();
Expand All @@ -181,28 +178,25 @@ public function postAvatar($path) {
!\OC\Files\Filesystem::isFileBlacklisted($files['tmp_name'][0])
) {
if ($files['size'][0] > 20*1024*1024) {
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('File is too big')]],
Http::STATUS_BAD_REQUEST,
$headers
Http::STATUS_BAD_REQUEST
);
}
$this->cache->set('avatar_upload', file_get_contents($files['tmp_name'][0]), 7200);
$content = $this->cache->get('avatar_upload');
unlink($files['tmp_name'][0]);
} else {
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('Invalid file provided')]],
Http::STATUS_BAD_REQUEST,
$headers
Http::STATUS_BAD_REQUEST
);
}
} else {
//Add imgfile
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('No image or file provided')]],
Http::STATUS_BAD_REQUEST,
$headers
Http::STATUS_BAD_REQUEST
);
}

Expand All @@ -214,57 +208,54 @@ public function postAvatar($path) {
if ($image->valid()) {
$mimeType = $image->mimeType();
if ($mimeType !== 'image/jpeg' && $mimeType !== 'image/png') {
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('Unknown filetype')]],
Http::STATUS_OK,
$headers
Http::STATUS_OK
);
}

$this->cache->set('tmpAvatar', $image->data(), 7200);
return new DataResponse(
return new JSONResponse(
['data' => 'notsquare'],
Http::STATUS_OK,
$headers
Http::STATUS_OK
);
} else {
return new DataResponse(
return new JSONResponse(
['data' => ['message' => $this->l->t('Invalid image')]],
Http::STATUS_OK,
$headers
Http::STATUS_OK
);
}
} catch (\Exception $e) {
$this->logger->logException($e, ['app' => 'core']);
return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_OK, $headers);
return new JSONResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_OK);
}
}

/**
* @NoAdminRequired
*
* @return DataResponse
* @return JSONResponse
*/
public function deleteAvatar() {
try {
$avatar = $this->avatarManager->getAvatar($this->userId);
$avatar->remove();
return new DataResponse();
return new JSONResponse();
} catch (\Exception $e) {
$this->logger->logException($e, ['app' => 'core']);
return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
return new JSONResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
}
}

/**
* @NoAdminRequired
*
* @return DataResponse|DataDisplayResponse
* @return JSONResponse|DataDisplayResponse
*/
public function getTmpAvatar() {
$tmpAvatar = $this->cache->get('tmpAvatar');
if (is_null($tmpAvatar)) {
return new DataResponse(['data' => [
return new JSONResponse(['data' => [
'message' => $this->l->t("No temporary profile picture available, try again")
]],
Http::STATUS_NOT_FOUND);
Expand All @@ -286,22 +277,22 @@ public function getTmpAvatar() {
* @NoAdminRequired
*
* @param array $crop
* @return DataResponse
* @return JSONResponse
*/
public function postCroppedAvatar($crop) {
if (is_null($crop)) {
return new DataResponse(['data' => ['message' => $this->l->t("No crop data provided")]],
return new JSONResponse(['data' => ['message' => $this->l->t("No crop data provided")]],
Http::STATUS_BAD_REQUEST);
}

if (!isset($crop['x'], $crop['y'], $crop['w'], $crop['h'])) {
return new DataResponse(['data' => ['message' => $this->l->t("No valid crop data provided")]],
return new JSONResponse(['data' => ['message' => $this->l->t("No valid crop data provided")]],
Http::STATUS_BAD_REQUEST);
}

$tmpAvatar = $this->cache->get('tmpAvatar');
if (is_null($tmpAvatar)) {
return new DataResponse(['data' => [
return new JSONResponse(['data' => [
'message' => $this->l->t("No temporary profile picture available, try again")
]],
Http::STATUS_BAD_REQUEST);
Expand All @@ -314,13 +305,13 @@ public function postCroppedAvatar($crop) {
$avatar->set($image);
// Clean up
$this->cache->remove('tmpAvatar');
return new DataResponse(['status' => 'success']);
return new JSONResponse(['status' => 'success']);
} catch (\OC\NotSquareException $e) {
return new DataResponse(['data' => ['message' => $this->l->t('Crop is not square')]],
return new JSONResponse(['data' => ['message' => $this->l->t('Crop is not square')]],
Http::STATUS_BAD_REQUEST);
} catch (\Exception $e) {
$this->logger->logException($e, ['app' => 'core']);
return new DataResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
return new JSONResponse(['data' => ['message' => $this->l->t('An error occurred. Please contact your admin.')]], Http::STATUS_BAD_REQUEST);
}
}
}
6 changes: 3 additions & 3 deletions tests/Core/Controller/AvatarControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public function testDeleteAvatarException() {
$this->logger->expects($this->once())
->method('logException')
->with(new \Exception("foo"));
$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
$this->assertEquals($expectedResponse, $this->avatarController->deleteAvatar());
}

Expand Down Expand Up @@ -377,7 +377,7 @@ public function testPostAvatarException() {
$this->logger->expects($this->once())
->method('logException')
->with(new \Exception("foo"));
$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_OK);
$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_OK);
$this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg'));
}

Expand Down Expand Up @@ -437,7 +437,7 @@ public function testPostCroppedAvatarException() {
$this->logger->expects($this->once())
->method('logException')
->with(new \Exception('foo'));
$expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
$expectedResponse = new Http\JSONResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST);
$this->assertEquals($expectedResponse, $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 11]));
}

Expand Down