Sylius / SyliusResourceBundle

Simpler CRUD for Symfony applications
https://sylius.com
MIT License
219 stars 155 forks source link

Remove FosRestBundle and BazingaHateoasBundle dependency. #170

Closed videni closed 3 years ago

videni commented 4 years ago

Reason 1

The two bundles are very usefull to build RESTfull API via HAL, I also like to support JSONAPI, so I suggest move they into a new package, maybe named as HAL extension for SyliusResourceBundle.

Reason 2

I'd like transform my entity to json using fractal instead of jmsserializer.

even though jmsserializer is flexiable and powerful , but it makes my API not stable enough, because they share the same metadata, for example, someone adds a 'roles' property to my user class within 'Default' groups.

Acme\Component\User\Model\User:
    exclusion_policy: ALL
    xml_root_name: xiaolvmo_user
    properties:
        id:
            expose: true
            type: integer
            groups: [Default]
        username:
            expose: true
            type: string
            groups: [Default]
        roles:
            expose: true
            type: ArrayCollection<Acme\Component\User\Model\Role>
            groups: [Default]

if Order also has user relation, then it will output roles, which may be not what need. serializing the roles property may trigger a doctrine/orm lazy loading, which hurt performance. so the API is very fragile because of jmsserializer.

loic425 commented 4 years ago

@pamil @lchrusciel Use something like that on ResourceController could be cool

    use FOS\RestBundle\View;

    public function showAction(Request $request): Response
    {
        $configuration = $this->requestConfigurationFactory->create($this->metadata, $request);

        $this->isGrantedOr403($configuration, ResourceActions::SHOW);
        $resource = $this->findOr404($configuration);

        $this->eventDispatcher->dispatch(ResourceActions::SHOW, $configuration, $resource);

        if ($configuration->isHtmlRequest()) {
            return $this->render($configuration->getTemplate(ResourceActions::SHOW . '.html'), [
                'configuration' => $configuration,
                'metadata' => $this->metadata,
                'resource' => $resource,
                $this->metadata->getName() => $resource,
            ]);
        }

        if (!$this->has('fos_rest') {
            throw new \LogicException('You can not use the "non-html" request if Fos Rest Bundle is not available. Try running "composer require fos/rest-bundle".');      
        }

        $view = View::create($resource);

        return $this->viewHandler->handle($configuration, $view);
    }

    public static function getSubscribedServices()
    {
        return array_merge(
            parent::getSubscribedServices(),
            [
                'fos_rest' => '?'.View::class
            ]
        );
    }
loic425 commented 4 years ago

@pamil @lchrusciel I opened a pull request with this work too.

lchrusciel commented 4 years ago

I've checked the PR and looks ok, however I would like someone to take another look. This is a quite important part of Sylius

loic425 commented 4 years ago

Yes and this is not enough (see my comments there).