Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to specify which entity manager to use for superclass entities #236

Closed
drgomesp opened this issue Jul 26, 2013 · 30 comments
Closed
Labels
Feature New feature proposals. RFC Discussions about potential changes or new features.

Comments

@drgomesp
Copy link

Currently I have multiple entity managers, on which I have superclasses inside both of them that extend some Sylius entities.

My issue is Sylius always uses the default entity manager, so whatever entity is mapped under a different manager doesn't get loaded.

Can you guys provide a solution for this? Or maybe an alternative?

@marcospassos
Copy link
Contributor

I think that a config option for entity managers would be good, but for now you can achieve this easily overriding the container service definition. So, just add the service definition on your bundle DI file or set in runtime through the bundle's extension file.

<service id="sylius.manager.BUNDLE" alias="your.entity.manager" />

Please, let me know whether this solution solves your problem.

@drgomesp
Copy link
Author

It solves half of the problem, since I need to use both of the entity managers and not just one of them.

It would be good if I could inject the correct manager at runtime somehow.

EDIT 1: I changed the service definition and I am still getting an error:

The class 'Sylius\Bundle\ProductBundle\Model\Product' was not found in the chain configured namespaces Flexy\Ftwo\Store\Entity

@marcospassos
Copy link
Contributor

You cannot have a model using two different managers, you have to use one entity manager per bundler. Anyway, you are getting this error because the model is not being covered by the defined manager. You have to specify it in your config file.

In fact, I found a better and appropriate solution to solve this issue, just changing a bit the DIC declaration:

<service id="sylius.model.MODEL_NAME" factory-service="doctrine" factory-method="getManagerForClass" class="Doctrine\Common\Persistence\ObjectManager">
    <argument>%sylius.model.MODEL_NAME.class%</argument>
</service>

This solution automatically get the proper manager for your model (based on your setting defined in the config file) and is storage agnostic.

@drgomesp
Copy link
Author

I don't want two different managers for one model. I want a set of entities to be loaded by one specific manager. This should be an option of the library, no doubt.

I didn't quite understand what you are saying. What service is this? Am I going to have to overwrite each service manually?

@marcospassos
Copy link
Contributor

With this solution, we don't need an option. The bundle is able to auto dicover the proper manager based in your model, transferring to you the responsibility for mapping the correct manager using the config file.

This change could be used by Sylius to solve this issue in all bundles (in fact there's an initiative to replace the hard coded service definition to a most flexible runtime generation) . For now, you can check if it solves your issue overriding each service that you are facing troubles. Once you confirm it, we can discuss about applying this solution widely.

@drgomesp
Copy link
Author

The thing is my entities are not inside any bundle structure, and the auto mapping is set to false, as you can see in my config file:

default:
    auto_mapping: false
    connection: default
    mappings:
        # An array of mappings, which may be a bundle name or something else
        entities:
            mapping: true
            type: xml
            dir: %kernel.root_dir%/../src/Flexy/Ftwo/Platform/Resources/doctrine
                alias: Entity
                prefix: Flexy\Ftwo\Platform\Entity
                is_bundle: false
store:
    auto_mapping: false
    connection: default
    mappings:
        # An array of mappings, which may be a bundle name or something else
        entities:
            mapping: true
            type: xml
            dir: %kernel.root_dir%/../src/Flexy/Ftwo/Store/Resources/doctrine
            alias: Entity
            prefix: Flexy\Ftwo\Store\Entity
            is_bundle: false

The Store namespace is where all Sylius extension classes are placed.

Is overwriting the service definition still going to be enough in this case?

@marcospassos
Copy link
Contributor

I think so, because when you are setting where your entities are, you are making Doctrine aware about where your entities are located. Then, the Registry can discover which is the correct manager for your entity. Can you try?

@drgomesp
Copy link
Author

@marcospassos I'm having some trouble on identifying this service I need to overwrite. Where is the original? What does model mean in that context?

@marcospassos
Copy link
Contributor

Suppose you want to use a different entity manager for the product, then you need to add to your DIC file (it must be loaded after SyliusProductBundle):

<service id="sylius.manager.product" factory-service="doctrine" factory-method="getManagerForClass" class="Doctrine\Common\Persistence\ObjectManager">
    <argument>%sylius.model.product.class%</argument>
</service>

<service id="sylius.repository.product" class="%sylius.repository.product.class%">
    <argument type="service" id="sylius.manager.product" />
    <argument type="service">
        <service
                factory-service="sylius.manager.product"
                factory-method="getClassMetadata"
                class="Doctrine\ORM\Mapping\ClassMetadata"
                public="false"
        >
            <argument>%sylius.model.product.class%</argument>
        </service>
    </argument>
</service>

@marcospassos
Copy link
Contributor

@marcospassos
Copy link
Contributor

No, because sylius.manager.product knows that DOCTRINE.ORM.STORE_ENTITY_MANAGER is the manager of %sylius.model.product.class%. Is just as I've wrote.

@drgomesp
Copy link
Author

It didn't work, as I'm still getting the output:

The class 'Sylius\Bundle\ProductBundle\Model\Product' was not found in the chain configured namespaces Flexy\Ftwo\Store\Entity

@marcospassos
Copy link
Contributor

Are you setting the model that you are using in the Sylius configuration?

You must have something like this in your config file:

sylius_product:
    classes:
        product: Flexy\Ftwo\Store\Entity\Product

@drgomesp
Copy link
Author

It's exactly what I have:

# Sylius Configuration
sylius_product:
    driver: doctrine/orm
    classes:
        product:
            model: Flexy\Ftwo\Store\Entity\Product
        property:
            model: Flexy\Ftwo\Store\Entity\Property
        prototype:
            model: Flexy\Ftwo\Store\Entity\Prototype

That's why I think there might be a bug occurring...

@marcospassos
Copy link
Contributor

Can you publish a repository that reproduces the problem you are facing? I'll clone it to see what is happening.

@marcospassos
Copy link
Contributor

Note: seems like you haven't overwritten what I mentioned previously. If you put it in a bundle that is loaded before SyliusProductBundle, it'll be overwritten by what you're trying to override.

@drgomesp
Copy link
Author

Actually I cannot =/. Can we try and find alternatives?

I did overwrite, and the bundle where I did is actually being loaded after the Sylius bundles.

@marcospassos
Copy link
Contributor

What I need is a fork of Symfony standard that reproduce your problem without expose your real code.

@pjedrzejewski
Copy link
Member

What we need is configurable object managers, like in FOSUserBundle. I think it should be relatively simple to implement. Do we want it configurable per model or per bundle?

@marcospassos
Copy link
Contributor

@pjedrzejewski looks to my snipped above, it does not require a manager configuration and is storage agnostic (unless you want to implement your own manager, but I don't think that is the case).

@drgomesp
Copy link
Author

@pjedrzejewski I think the configuration should be per model, since it covers all cases.

@drgomesp
Copy link
Author

@marcospassos your solution doesn't work, like I said earlier.

Any other tips?

@marcospassos
Copy link
Contributor

@drgomesp if you provide a repository that reproduces the problem will be a pleasure help you.

@drgomesp
Copy link
Author

@marcospassos I tried hardcoding the service definition you suggested directly on the SyliusProductBundle. The repository it tries to load is the ProductRepository, and nothing seems to work.

My workaround for now was to keep the superclass entities of sylius entities inside the default entity manager, and move all other stuff (such as the superclass of the FOSUserBundle user entity) to a different non-default manager.

Since FOSUserBundle supports configurable object managers, everything worked. But please, understand that this is a demand for the Sylius project and not something I need to do (as a workaround) in my project.

I'm willing to contribute if someone gives me a hint on how to start on that.

@marcospassos
Copy link
Contributor

Yes, I agree with you. I think this is something that we need to implement, I just asked you to publish a fork reproducing the problem for I work on. But it's ok, I'll try reproduce your scenario and I'll back with suggestions as soon as I can.

@FundCoders
Copy link

Looking forward to this enhancement? Take a look at how you can help Devs and get your fix sooner on FundCoders.com

@jjanvier
Copy link
Contributor

jjanvier commented Oct 2, 2013

ouch... I hope this is not the beginning of spam on Github...

@FundCoders
Copy link

No, not at all... We choose carefully which issues make sense for our tool. If it's considered spam by any of the repo administrators, our apologies and feel free to remove our comments. FundCoders team.

@pjedrzejewski
Copy link
Member

Marked as must-have for the beta. It's not that hard to achieve, but I just wonder if we can avoid a whole bunch of code duplicated across every bundle. (for configuration of this and handling)

@pjedrzejewski
Copy link
Member

Added in #1920.

pamil pushed a commit to pamil/Sylius that referenced this issue Mar 21, 2016
Fix mistype for xml config in extra_tools.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature proposals. RFC Discussions about potential changes or new features.
Projects
None yet
Development

No branches or pull requests

5 participants