Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

EZP-27271: Unable to upload large binary file using REST / Platform UI #953

Merged
merged 4 commits into from
Mar 19, 2018

Conversation

kmadejski
Copy link
Member

@kmadejski kmadejski commented Mar 1, 2018

JIRA issue: https://jira.ez.no/browse/EZP-27271

Description

Currently, this PR contains only the first step related to the backend: try to DRY and reuse once parsed and created RestContentCreateStruct object.

Related changes

The related changes in the kernel are here ezsystems/ezpublish-kernel#2268

@kmadejski kmadejski requested review from adamwojs and andrerom March 1, 2018 15:16
@kmadejski kmadejski changed the base branch from master to 1.7 March 1, 2018 15:48
Copy link
Member

@adamwojs adamwojs left a comment

Choose a reason for hiding this comment

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

Let's do this in another way:

On the kernel side: from the \eZ\Publish\Core\REST\Server\Controller\Content::createContent method let's extract:

try {
    $content = $this->repository->getContentService()->createContent(
        $contentCreate->contentCreateStruct,
        array($contentCreate->locationCreateStruct)
    );
} catch (ContentValidationException $e) {
    throw new BadRequestException($e->getMessage());
} catch (ContentFieldValidationException $e) {
    throw new RESTContentFieldValidationException($e);
}

$contentValue = null;
$contentType = null;
$relations = null;
if ($this->getMediaType($request) === 'application/vnd.ez.api.content') {
    $contentValue = $content;
    $contentType = $this->repository->getContentTypeService()->loadContentType(
        $content->getVersionInfo()->getContentInfo()->contentTypeId
    );
    $relations = $this->repository->getContentService()->loadRelations($contentValue->getVersionInfo());
}

return new Values\CreatedContent(
    array(
        'content' => new Values\RestContent(
            $content->contentInfo,
            null,
            $contentValue,
            $contentType,
            $relations
        ),
    )
);

to the new protected method \eZ\Publish\Core\REST\Server\Controller\Content::doCreateContent(ContentCreate $contentCreate) (name could be better).

On the PlatformUI let's change \EzSystems\PlatformUIBundle\Controller\Rest\ContentController::createContent in this way:

public function createContent(Request $request)
{
   $contentCreate = $this->inputDispatcher->parse(
        new Message(
            array('Content-Type' => $request->headers->get('Content-Type')),
            $request->getContent()
        )
    );

    if (!$this->isUserCreateRequest($request, $contentCreate)) {
        return parent::doCreateContent($contentCreate);
    }

    $createdUser = $this->repository->getUserService()->createUser(
        $this->mapRequestToUserCreateStruct($request),
        $this->mapRequestToParentGroups($request)
    );

    return $this->mapUserToCreatedContent($createdUser);
}

@kmadejski kmadejski force-pushed the ezp_27271_upload branch 2 times, most recently from 78466f7 to d1f8fd2 Compare March 5, 2018 14:51
@kmadejski
Copy link
Member Author

ping @adamwojs, I've applied the changes which you've proposed, thanks!

@kmadejski kmadejski force-pushed the ezp_27271_upload branch 2 times, most recently from 6c93b86 to b273fc9 Compare March 6, 2018 10:20
@kmadejski kmadejski changed the title [POC] EZP-27271: Unable to upload large binary file using REST / Platform UI EZP-27271: Unable to upload large binary file using REST / Platform UI Mar 6, 2018
@kmadejski
Copy link
Member Author

kmadejski commented Mar 14, 2018

@andrerom PR changed as requested in ezsystems/ezpublish-kernel#2268 (comment) 🙂

ping @adamwojs, @andrerom do the review once again, please.

Update: adapted accordingly to the changes in kernel.

@kmadejski
Copy link
Member Author

kmadejski commented Mar 19, 2018

All tests passed for a4c9c64 (https://travis-ci.org/ezsystems/PlatformUIBundle/builds/354367290). These ones will probably fail due to changed kernel requirements. Besides that, I think that this PR is ready for merge. @andrerom could you review, please?

composer.json Outdated
@@ -10,7 +10,7 @@
"require": {
"ezsystems/platform-ui-assets-bundle": "^3.1.0",
"ezsystems/repository-forms": "^1.2",
"ezsystems/ezpublish-kernel": "^6.6",
"ezsystems/ezpublish-kernel": "~6.7.8 || ^6.13.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

you can set it to ~6.7.8@dev || ^6.13.2@dev, we'll ideally have to cleanup after release, but otherwise this will break travis on merge

@andrerom andrerom merged commit e405e70 into ezsystems:1.7 Mar 19, 2018
andrerom added a commit that referenced this pull request Mar 19, 2018
Since there where several cases using this, adjust the naming in kernel as well: ezsystems/ezpublish-kernel@35f4073
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants