neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 221 forks source link

Migrate/Fix/Rethink `NodeIdentityConverterAspect` for 9.0 #5069

Open mhsdesign opened 4 months ago

mhsdesign commented 4 months ago

subissue of https://github.com/neos/neos-development-collection/issues/3216

I dont understand exactly what the NodeIdentityConverterAspect thing does or what it should do in 9.0, but as we dont implement any __contextNodePath logic in 9.0's EventSourcedFrontendNodeRoutePartHandler and seeing that the tests pass after its removal makes me think it has become obsolete.

If there is something i dont know, please let me know otherwise i think it can be removed?

introduced 2012 with:

adjusted:

whyyyy does Route::resolves even call convertObjectsToIdentityArrays? And why with raw node objects?

kitsunet commented 4 months ago

we rely on Neos\Flow\Persistence\AbstractPersistenceManager->convertObjectToIdentityArray in some low level places, think you have a node in a Scope("session") object or so. That method can only convert ORM objects though so the aspect wraps around and converts nodes itself into something that can later be unserialized. A serialized NodeAddress shoudl be fine I think.

mhsdesign commented 4 months ago

and converts nodes itself into something that can later be unserialized. A serialized NodeAddress shoudl be fine I think.

... but we dont support string nodeaddress to node in the long run? https://github.com/neos/neos-development-collection/issues/4873

mhsdesign commented 4 months ago

so it was for holding a Node in a session?

like

/**
 * @Flow\Scope("session")
 */
class MySessionThing {
        protected Node $myNode = null;

        public function setNode(Node $item) {
                $this->myNode = $item;
        }

        public function getMyNode() {
                return $this->items;
        }
}

in Neos 9 id say one must use:

/**
 * @Flow\Scope("session")
 */
class MySessionThing {
        protected NodeAddress $myNode = null;

        public function setNode(NodeAddress $item) {
                $this->myNode = $item;
        }

        public function getMyNode() {
                return $this->items;
        }
}
mhsdesign commented 4 months ago

well ... as we relaxed our decision over at https://github.com/neos/neos-development-collection/issues/4873#issuecomment-2131390989 and it looks like well keep a node property mapper, we should probably keep this hack as well ... and make sure that it works.

Imo for that we have to return the node address as string directly and NOT in an array as value of contextPath!

mhsdesign commented 4 months ago

Christian and me concluded that for routing nodes this is not necessarily required as we currently and will ever serialise the NodeAddress manually. If passed as instance we could confirm that __contextPath will be part of the uri. Also this logic is needed for forwarding requests and serialising for that all arguments.