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

Added the ability to override RamlNavigatorFactory constants from a child class. #76

Conversation

estelsmith
Copy link
Contributor

It's nice to have the ability to override where the api.raml file is located. By using late static binding, users can extend the RamlNavigatorFactory and update constants, pointing api.raml to a bundle-specific configuration rather than forcing users to place the file in the app/config directory.

Example:

namespace ApiBundle\Factory;

use GoIntegro\Bundle\HateoasBundle\DependencyInjection\Factory\RamlNavigatorFactory as BaseNavigatorFactory;

class RamlNavigatorFactory extends BaseNavigatorFactory
{
    const RAML_DOC_PATH = '/../src/ApiBundle/Resources/config/api.raml';
    const ERROR_PARAM_TYPE = 'The "api.raml" file was not found!';
}

Ideally, the path to the api.raml file would be configurable via config.yml, rather than having to add compiler passes that change service definitions to custom implementations.

@skqr
Copy link
Contributor

skqr commented Apr 15, 2015

Hi, @estelsmith! Thanks for taking the time to contribute.

I'm kinda thinking that that's already possible by overriding the factory service, although I might be wrong.

@acmenna is this possible already, or am I tripping balls?

@estelsmith
Copy link
Contributor Author

That's what I'm doing is overriding the factory service. However, currently I need to override the service and not only update the constant, but also override __construct() and createNavigator() methods using the exact same functionality provided in the base. An example of what I'm doing is https://github.com/estelsmith/hateoas-test/blob/master/src/ApiBundle/Factory/RamlNavigatorFactory.php.

None of this overridden code is required if RamlNavigatorFactory uses late static binding instead of self when referencing the constants.

@estelsmith
Copy link
Contributor Author

@skqr I think I just realized a potential source of confusion. I just want to clarify that I was not looking for the ability to override the service, which already exists. I was looking for the ability to change the constants in the overridden service, then have the original code utilize the updated constants without having to also override the class methods as well.

@estelsmith
Copy link
Contributor Author

@skqr @acmenna Can this pull request be accepted?

@acmenna
Copy link
Contributor

acmenna commented Apr 21, 2015

@estelsmith thank you.

acmenna added a commit that referenced this pull request Apr 21, 2015
…ry_constants

Added the ability to override RamlNavigatorFactory constants from a child class.
@acmenna acmenna merged commit eb51cc7 into GoIntegro:master Apr 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants