Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

[WIP] move, rename delete resource of editable repository #31

Closed

Conversation

ElectricMaxxx
Copy link
Member

This i the WIP PR to test an try the EditableRepository interface of both repositories in the component.

@ElectricMaxxx
Copy link
Member Author

ElectricMaxxx commented May 27, 2016

@dantleech got an other issue with the factories of the resource-bundle while testing:

[RuntimeException]                                    
  Repository factory "composite" has already been set. 

For some reasons CmfResourceBundle::build() is called twice when calling the api with WebTests. So the factory collection inside the extension class still has the factories and throws an exception.

@@ -1,6 +1,6 @@
# Symfony CMF Resource REST API Bundle

[![Build Status](https://secure.travis-ci.org/symfony-cmf/ResourceRestBundle.png?branch=master)](http://travis-ci.org/symfony-cmf/ResourceRestBundle)
[![Build Status](https://travis-ci.org/symfony-cmf/resource-rest-bundle.svg?branch=master)](https://travis-ci.org/symfony-cmf/resource-rest-bundle)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the build icon change all other bundles still have.

@ElectricMaxxx ElectricMaxxx force-pushed the add_more_rest_methods branch from 5211e6b to 324a451 Compare May 28, 2016 23:28
@ElectricMaxxx ElectricMaxxx force-pushed the add_more_rest_methods branch from 324a451 to e6de622 Compare May 30, 2016 13:36
}

/**
* There are two different changes that can currently be done on a cmf resource:
Copy link
Member

Choose a reason for hiding this comment

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

the styleci complains because the first line of a comment is the short description.

we should say something like "Change an existing resource." and then add a new line.

@wouterj
Copy link
Member

wouterj commented Jun 3, 2016

For some reason, using this PR in the CMF sandbox always results in the following error:

Resources are not supported in serialized data. Path: Monolog\Handler\StreamHandler -> Symfony\Bridge\Monolog\Logger -> Symfony\Component\HttpKernel\Debug\TraceableEventDispatcher -> Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager -> appDevDebugProjectContainer -> Doctrine\Bundle\PHPCRBundle\ManagerRegistry -> DTL\Glob\Finder\PhpcrOdmTraversalFinder -> Symfony\Cmf\Component\Resource\Repository\PhpcrOdmRepository -> Symfony\Cmf\Component\Resource\Repository\Resource\PhpcrOdmResource

Which is quite strange, as it involves a lot of things that shouldn't really need to be serialized.

Without this PR, but with the resource and resourcebundle move/remove PRs, things work without errors.

@ElectricMaxxx
Copy link
Member Author

@wouterj yea i think we need some exclusions on the documents and the resource itself, i think got the same issue on my on try on the sandbox :-)

@wouterj
Copy link
Member

wouterj commented Jun 3, 2016

But the strange thing is that things work perfectly without this PR. Unless I'm wrong, you don't change anything about the get action here, do you?

@ElectricMaxxx
Copy link
Member Author

@wouterj yes, you are right. That's really strange.

@wouterj
Copy link
Member

wouterj commented Jun 3, 2016

Hmm, I think I have a clue about why this issue is happening and how to solve it. Let's see...

@ElectricMaxxx
Copy link
Member Author

I would pass an other problem: started testing patch, created request (test) and the body is empty inside the controller.

@wouterj
Copy link
Member

wouterj commented Jun 3, 2016

You have to revert all PuliResource to Resource renamings. The new interface is called PuliResource (https://github.com/puli/repository/commits/1.0/src/Api/Resource/PuliResource.php).

Because this is now wrong, the serializer handlers aren't executed and the JMS serializer simply serializes everything in the Document, including the doctrine manager and full container.

@ElectricMaxxx
Copy link
Member Author

uhh you are right, did that on my bundle PR :-)

_controller: cmf_resource_rest.controller.resource:patchResourceAction
_format: json

_get_cmf_resource:
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to put the vendor in front: _cmf_get_resource

@wouterj
Copy link
Member

wouterj commented Jun 5, 2016

Closing in favor of #36

@wouterj wouterj closed this Jun 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants