Skip to content

Commit

Permalink
Merge pull request #25 from dew326/ezp-27439-improve-loading-time
Browse files Browse the repository at this point in the history
 EZP-27439: Improve Multi File Upload loading time
  • Loading branch information
lserwatka authored Jun 12, 2017
2 parents dc871b1 + fe55a41 commit e04ed8a
Show file tree
Hide file tree
Showing 13 changed files with 402 additions and 119 deletions.
6 changes: 2 additions & 4 deletions bundle/Controller/RestController.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,19 @@ public function __construct(
}

/**
* @param int $contentTypeId
* @param int $parentLocationId
* @param \Symfony\Component\HttpFoundation\Request $request
*
* @return \EzSystems\MultiFileUpload\API\Repository\Values\PermissionReport
*/
public function checkPermission($contentTypeId, $parentLocationId, Request $request)
public function checkPermission($parentLocationId, Request $request)
{
if (!$request->isXmlHttpRequest()) {
throw new BadRequestHttpException('The request is not an AJAX request');
}

$contentType = $this->contentTypeService->loadContentType($contentTypeId);
$parentLocation = $this->locationService->loadLocation($parentLocationId);

return $this->permissionReportService->canUserCreateContent($contentType, $parentLocation);
return $this->permissionReportService->canUserCreateContent($parentLocation);
}
}
3 changes: 1 addition & 2 deletions bundle/Resources/config/routing.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
ez_systems.multifile_upload.controller.rest.check_permission:
methods: [GET]
path: /multifileupload/v1/check-permission/{parentLocationId}/{contentTypeId}
path: /multifileupload/v1/check-permission/{parentLocationId}
requirements:
parentLocationId: '\d+'
contentTypeId: '\d+'
defaults:
_controller: ez_systems.multifile_upload.controller.rest:checkPermission
6 changes: 5 additions & 1 deletion bundle/Resources/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ services:
arguments:
- '@ez_systems.multifile_upload.service.permission_resolver'
- '@ezpublish.api.service.content'
- '@ezpublish.api.service.content_type'
- '@ezpublish.api.service.location'
- '%ez_systems.multifile_upload.location_mappings%'
- '%ez_systems.multifile_upload.default_mappings%'
- '%ez_systems.multifile_upload.fallback_content_type%'

ez_systems.multifile_upload.controller.rest:
class: '%ez_systems.multifile_upload.controller.rest.class%'
Expand All @@ -30,7 +34,7 @@ services:
- '@ezpublish.api.service.location'
- '@ez_systems.multifile_upload.service.permission_report'

# todo: remove once kernel bundle exposes PermissionResolver as a DI service
# todo: remove once kernel exposes PermissionResolver as a DI service
ez_systems.multifile_upload.service.permission_resolver:
class: '%ez_systems.multifile_upload.service.permission_resolver.class%'
factory: ['@ezpublish.api.inner_repository', 'getPermissionResolver']
Expand Down
38 changes: 5 additions & 33 deletions bundle/Resources/public/js/plugins/mfu-fileupload-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,51 +265,23 @@ YUI.add('mfu-fileupload-plugin', function (Y) {
* @param event.errorCallback {Function} an error callback
*/
_checkContentCreatePermissions: function (event) {
const defaultContentTypeIdentifier = this.get('defaultContentType').contentTypeIdentifier;
const locationIdentifier = this.get('host').get('contentType').get('identifier');
const locationMappings = this.get('contentTypeByLocationMappings');
const mappedLocation = locationMappings.find(item => item.contentTypeIdentifier === locationIdentifier);
const uniqueIdentifiers = mappedLocation ? [...new Set(mappedLocation.mappings.map(item => item.contentTypeIdentifier))] : [];
let promises = [];

if (uniqueIdentifiers.length) {
promises = uniqueIdentifiers.map(identifier => this._loadContentTypeByIdentifier(identifier));
} else {
promises = [this._loadContentTypeByIdentifier(defaultContentTypeIdentifier)];
}
const locationId = this.get('host').get('location').get('locationId');

Promise.all(promises)
.then(this._checkContentCreatePermissionByContentType.bind(this))
this._makeCheckPermissionsRequest(locationId)
.then(this._setContentCreatePermissionsState.bind(this, event.permissionsStateCallback))
.catch(event.errorCallback);
},

/**
* Checks permissions for creating content of specific content types
*
* @method _checkContentCreatePermissionByContentType
* @protected
* @param data {Array} a list of resolved promises
* @return {Promise}
*/
_checkContentCreatePermissionByContentType: function (data) {
const locationId = this.get('host').get('location').get('locationId');

return Promise.all(data.map(this._makeCheckPermissionsRequest.bind(this, locationId)))
},

/**
* Makes a request to check permissions for a selected location
*
* @method _makeCheckPermissionsRequest
* @protected
* @param locationId {Number} non-REST location id
* @param response {Object} load content type by identifier REST request response
* @return {Promise}
*/
_makeCheckPermissionsRequest: function (locationId, response) {
const contentTypeId = response.document.ContentTypeInfoList.ContentType[0].id;
const url = `api/ezp/v2/multifileupload/v1/check-permission/${locationId}/${contentTypeId}`;
_makeCheckPermissionsRequest: function (locationId) {
const url = `api/ezp/v2/multifileupload/v1/check-permission/${locationId}`;

return new Promise((resolve, reject) => {
this.get('host')
Expand All @@ -333,7 +305,7 @@ YUI.add('mfu-fileupload-plugin', function (Y) {
* @param permissions {Array} a list of resolved content type content create permission checks
*/
_setContentCreatePermissionsState: function (callback, permissions) {
const isContentCreateAllowed = permissions.some(item => item.document.PermissionReport.allowed);
const isContentCreateAllowed = !!permissions.document.PermissionReport.allowedContentTypes.length;

callback(isContentCreateAllowed);
},
Expand Down
18 changes: 18 additions & 0 deletions bundle/Resources/public/js/views/mfu-uploadform-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ YUI.add('mfu-uploadform-view', function (Y) {
return;
}

if (this.get('checkPermissionPrevented')) {
this._setFormActiveState(true);
return;
}

/**
* Checks for content create permissions for a logged in user.
* Listened by {{#crossLink "mfu.Plugin.FileUploadService"}}mfu.Plugin.FileUploadService{{/crossLink}}
Expand Down Expand Up @@ -360,6 +365,19 @@ YUI.add('mfu-uploadform-view', function (Y) {
valueFn: () => Y.eZ.trans('file.upload.permissions.check.error', {}, 'uploadform'),
readOnly: true,
},

/**
* Flag indicating if the permissons check should be ommited.
* If set to truthy value user is allowed to create content.
*
* @attribute checkPermissionPrevented
* @type {Boolean}
* @writeOnce 'initOnly'
*/
checkPermissionPrevented: {
value: false,
writeOnce: 'initOnly',
},
}
});
});
3 changes: 2 additions & 1 deletion bundle/Resources/public/js/views/mfu-uploadpopup-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ YUI.add('mfu-uploadpopup-view', function (Y) {
valueFn: function () {
return new Y.mfu.UploadFormView({
bubbleTargets: this,
onDropCallback: this._updateUploadedFilesList.bind(this)
onDropCallback: this._updateUploadedFilesList.bind(this),
checkPermissionPrevented: true
});
},
readOnly: true,
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
},
"extra": {
"branch-alias": {
"dev-master": "0.1.x-dev"
"dev-master": "0.2.x-dev"
}
}
}
3 changes: 1 addition & 2 deletions lib/API/Repository/PermissionReportServiceInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ interface PermissionReportServiceInterface
/**
* Returns PermissionReport regarding content:create permission for the contentType in location.
*
* @param \eZ\Publish\API\Repository\Values\ContentType\ContentType $contentType
* @param \eZ\Publish\API\Repository\Values\Content\Location $parentLocation
* @param \eZ\Publish\API\Repository\Values\User\UserReference $userReference
*
* @return \EzSystems\MultiFileUpload\API\Repository\Values\PermissionReport
*/
public function canUserCreateContent(ContentType $contentType, Location $parentLocation, UserReference $userReference = null);
public function canUserCreateContent(Location $parentLocation, UserReference $userReference = null);
}
7 changes: 2 additions & 5 deletions lib/API/Repository/Values/PermissionReport.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@

class PermissionReport extends ValueObject
{
/** @var int */
public $contentTypeId;

/** @var int */
public $parentLocationId;

Expand All @@ -21,6 +18,6 @@ class PermissionReport extends ValueObject
/** @var string */
public $function;

/** @var bool */
public $allowed;
/** @var string[] */
public $allowedContentTypes;
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,18 @@ public function visit(Visitor $visitor, Generator $generator, $data)
$generator->startValueElement('parentLocationId', $data->parentLocationId);
$generator->endValueElement('parentLocationId');

$generator->startValueElement('contentTypeId', $data->contentTypeId);
$generator->endValueElement('contentTypeId');

$generator->startValueElement('module', $data->module);
$generator->endValueElement('module');

$generator->startValueElement('function', $data->function);
$generator->endValueElement('function');

$generator->startValueElement('allowed', $data->allowed);
$generator->endValueElement('allowed');
$generator->startList('allowedContentTypes');
foreach ($data->allowedContentTypes as $contentTypeIdentifier) {
$generator->startValueElement('allowedContentTypes', $contentTypeIdentifier);
$generator->endValueElement('allowedContentTypes');
}
$generator->endList('allowedContentTypes');

$generator->endObjectElement('PermissionReport');
}
Expand Down
102 changes: 84 additions & 18 deletions lib/Core/Repository/PermissionReportService.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
namespace EzSystems\MultiFileUpload\Core\Repository;

use eZ\Publish\API\Repository\ContentService;
use eZ\Publish\API\Repository\ContentTypeService;
use eZ\Publish\API\Repository\LocationService;
use eZ\Publish\API\Repository\PermissionResolver;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\API\Repository\Values\ContentType\ContentType;
use eZ\Publish\API\Repository\Values\User\UserReference;
use EzSystems\MultiFileUpload\API\Repository\PermissionReportServiceInterface;
use EzSystems\MultiFileUpload\API\Repository\Values\PermissionReport;
Expand All @@ -25,59 +25,125 @@ class PermissionReportService implements PermissionReportServiceInterface
/** @var \eZ\Publish\API\Repository\ContentService */
protected $contentService;

/** @var \eZ\Publish\API\Repository\ContentTypeService */
protected $contentTypeService;

/** @var \eZ\Publish\API\Repository\LocationService */
protected $locationService;

/** @var array */
protected $locationMappings = [];

/** @var array */
protected $defaultMappings = [];

/** @var array */
protected $fallbackContentType = [];

/**
* @param \eZ\Publish\API\Repository\PermissionResolver $permissionResolver
* @param \eZ\Publish\API\Repository\ContentService $contentService
* @param \eZ\Publish\API\Repository\ContentTypeService $contentTypeService
* @param \eZ\Publish\API\Repository\LocationService $locationService
* @param array $locationMappings
* @param array $defaultMappings
* @param array $fallbackContentType
*/
public function __construct(
PermissionResolver $permissionResolver,
ContentService $contentService,
LocationService $locationService
ContentTypeService $contentTypeService,
LocationService $locationService,
array $locationMappings,
array $defaultMappings,
array $fallbackContentType
) {
$this->permissionResolver = $permissionResolver;
$this->contentService = $contentService;
$this->contentTypeService = $contentTypeService;
$this->locationService = $locationService;
$this->locationMappings = $locationMappings;
$this->defaultMappings = $defaultMappings;
$this->fallbackContentType = $fallbackContentType;
}

/**
* @param \eZ\Publish\API\Repository\Values\ContentType\ContentType $contentType
* @param \eZ\Publish\API\Repository\Values\Content\Location $parentLocation
* @param \eZ\Publish\API\Repository\Values\User\UserReference|null $userReference
*
* @return \EzSystems\MultiFileUpload\API\Repository\Values\PermissionReport
*/
public function canUserCreateContent(
ContentType $contentType,
Location $parentLocation,
UserReference $userReference = null
) {
$module = 'content';
$function = 'create';

if (null !== $userReference) {
$this->permissionResolver->setCurrentUserReference($userReference);
}

$contentCreateStruct = $this->contentService->newContentCreateStruct($contentType, $contentType->mainLanguageCode);
$locationCreateStruct = $this->locationService->newLocationCreateStruct($parentLocation->id);
$mappings = $this->getContentTypeIdentifierMappingsForLocation($parentLocation);
$allowedContentTypes = [];

$permissionReport = new PermissionReport([
'contentTypeId' => $contentType->id,
'parentLocationId' => $parentLocation->id,
'module' => $module,
'function' => $function,
'allowed' => $this->permissionResolver->canUser(
$module,
$function,
foreach ($mappings as $contentTypeIdentifier) {
$contentType = $this->contentTypeService->loadContentTypeByIdentifier(
$contentTypeIdentifier
);
$contentCreateStruct = $this->contentService->newContentCreateStruct(
$contentType,
$contentType->mainLanguageCode
);
$isAllowed = $this->permissionResolver->canUser(
'content',
'create',
$contentCreateStruct,
[$locationCreateStruct]
),
);

if ($isAllowed) {
$allowedContentTypes[] = $contentTypeIdentifier;
}
}

return new PermissionReport([
'parentLocationId' => $parentLocation->id,
'module' => 'content',
'function' => 'create',
'allowedContentTypes' => array_unique($allowedContentTypes),
]);
}

/**
* @param \eZ\Publish\API\Repository\Values\Content\Location $parentLocation
*
* @return array
*/
private function getContentTypeIdentifierMappingsForLocation(Location $parentLocation)
{
$locationContentType = $this->contentTypeService->loadContentType($parentLocation->getContentInfo()->contentTypeId);
$mappings = [];

foreach ($this->locationMappings as $locationMapping) {
if ($locationMapping['content_type_identifier'] === $locationContentType->identifier) {
$mappings = $locationMapping['mappings'];
break;
}
}

return $permissionReport;
$mappings = !empty($mappings)
? $mappings
: array_merge($this->defaultMappings, [$this->fallbackContentType]);

return $this->getUniqueContentTypeIdentifiersFromMappings($mappings);
}

/**
* @param array $mappings
*
* @return array
*/
private function getUniqueContentTypeIdentifiersFromMappings(array $mappings)
{
return array_unique(array_column($mappings, 'content_type_identifier'));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function testGetConfig()
'mappings' => [
[
'mime_types' => [
'application/msword'
'application/msword',
],
'content_type_identifier' => 'file',
'content_field_identifier' => 'file',
Expand All @@ -51,7 +51,7 @@ public function testGetConfig()
[
'mime_types' => [
'application/msword',
'application/vnd.openxmlformats-officedocument.wordprocessingml.document'
'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
],
'content_type_identifier' => 'file',
'content_field_identifier' => 'file',
Expand Down
Loading

0 comments on commit e04ed8a

Please sign in to comment.