symfony-cmf / routing-auto

RoutingAuto component
https://cmf.symfony.com
Other
6 stars 11 forks source link

New document does not get Route added #99

Closed silverbackdan closed 7 years ago

silverbackdan commented 7 years ago

When I add a new document, I need to refresh it before I can call getRoutes (otherwise it returns an empty array) - this is if I implement RouteReferrersReadInterface which does not include an addRoute method See a conversation here with examples: https://github.com/symfony-cmf/routing-bundle/issues/394

I followed some docs here about the CMF bundle which uses the routing-auto-bundle, which uses this bundle https://symfony.com/doc/current/cmf/tutorial/getting-started.html

I'm not sure what I'd need to put in the addRoute method if I implement that interface with my setup. I'd like the route to be automatically generated based on my config. Is just refreshing the doc the best way to do this?

dbu commented 7 years ago

you should implement the RouteReferrersInterface instead. in addRoute you need to add the route to the route referrers collection on your document. see https://github.com/symfony-cmf/content-bundle/blob/1cdaaeedf74d7f6ac6804711f2eb1c94bb118f13/src/Model/StaticContent.php#L267-L270 and https://github.com/symfony-cmf/content-bundle/blob/1cdaaeedf74d7f6ac6804711f2eb1c94bb118f13/src/Model/StaticContent.php#L100 => for new documents you need to initialize the collection in the constructor. once the document is loaded from doctrine, it has some tracked collection on it.

can you try if implementing this pattern actually works? if so, its mainly a problem of the tutorial suggesting to use the read only interface.

silverbackdan commented 7 years ago

I was hopeful with this, but it doesn't seem to have worked. So I use a BaseContentTrait and added this method for constructor:

public function __construct() {
    $this->routes = new ArrayCollection();
}

I also changed definition of $routes

/**
 * @var RouteObjectInterface[]
 * @PHPCR\Referrers(
 *     referringDocument="Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Phpcr\Route",
 *     referencedBy="content"
 * )
 */
protected $routes;

I changed the document classes to implement Symfony\Cmf\Component\Routing\RouteReferrersInterface - to make this work I added the methods as shown:

    /**
     * {@inheritdoc}
     */
    public function addRoute($route)
    {
        $this->routes->add($route);
    }

    /**
     * {@inheritdoc}
     */
    public function removeRoute($route)
    {
        $this->routes->removeElement($route);
    }

Before I refresh, because there's no type hinting I'm able to see the method exists because I can call addRoute with an arbitrary string, then getRoutes will return an array with that string.

Without doing that, I get the same result as before. The getRoutes method is an empty array - will this be to do with using the @PHPCR\Referrers annotation? Or possibly that I'm implementing 2 interfaces on my document class NewsPost implements RouteReferrersInterface, NodeInterface

I've also checked that my other trait has no constructor method in it and have tried moving the constructor into the class itself.

dantleech commented 7 years ago

Out of interest - why do you have addRoute if the document is managed by routing auto?

silverbackdan commented 7 years ago

@dantleech I didn't originally. I thought that's what @dbu has asked for me to implement to make this work without refreshing the doc with the doctrine document manager - I did just have RouteReferrersReadInterface implemented before.

dantleech commented 7 years ago

You should not need to have any setters on the document in order for the RA system to work.

As far as I recall this is a limitation of the PHPCR-ODM adapter (caused by limitation in PHPCR-ODM). We create the route directly at the PHPCR level, the PHPCR-ODM does not know about this, hence the requirement for a manual refresh.

Normally this is fine (typically you would be "done" after persisting your document). But indeed in some usecases it can be confusing.

silverbackdan commented 7 years ago

Cool, if refreshing is the way to go then that is fine for me. Thanks for both of your time. Happy for me to close, or do you think it's worth putting this in documentation somewhere?

dantleech commented 7 years ago

Wouldn't hurt to add a ::note somewhere, maybe in the main bundle documentation?

dbu commented 7 years ago

ftr: i don't consider it a limitation of phpcr-odm that database changes are not synchronized immediately. taking doctrine orm as reference, this is the expected behaviour. orm will also not automatically update the inverse relation of an entity: http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/unitofwork-associations.html

in my opinion, https://github.com/symfony-cmf/routing-auto-bundle/blob/c30ce6897c7de4a24b69e9fe85da2438db582372/src/Adapter/PhpcrOdmAdapter.php#L160 and https://github.com/symfony-cmf/routing-auto-bundle/blob/c30ce6897c7de4a24b69e9fe85da2438db582372/src/Adapter/PhpcrOdmAdapter.php#L272 (or the AutoRoute::setContent method) should check if the content has the addRoute method and if so add the route to the content. then the in-memory model representation of the relation would be immediately up to date.

i will try a pull request.

dbu commented 7 years ago

see https://github.com/symfony-cmf/routing-auto-bundle/pull/207

dbu commented 7 years ago

this is now merged. though you best wait for the stable release before updating your code.

silverbackdan commented 7 years ago

Thanks so much for finding the time to work on this one @dbu - no problem to wait.