8p / EightPointsGuzzleBundle

⛽️ Integrates Guzzle 6.x, a PHP HTTP Client, into Symfony
MIT License
440 stars 71 forks source link

Add plugin system and remove dependency on wsse package #129

Closed gregurco closed 7 years ago

gregurco commented 7 years ago

Fixed tickets: [#73, #79, #90] License: MIT

Info:

This pull request is a concept, how to make wsse plugin optional in guzzle bundle. Linked PR 1: https://github.com/8p/guzzle-wsse-middleware/pull/13 Linked PR 2: https://github.com/gregurco/GuzzleBundleWssePlugin/pull/1 Inspired from: https://matthiasnoback.nl/2015/07/a-plugin-system-for-bundles/

Important! It's not finished, it's a concept. I want to see any feedback, suggestion, criticism or questions :)

Usage:

All plugins will be activated/connected through GuzzleBundle constructor in AppKernel, like this:

new EightPoints\Bundle\GuzzleBundle\GuzzleBundle([
    new Gregurco\Bundle\GuzzleBundleWssePlugin\GuzzleBundleWssePlugin(),
])

How to help with testing:

Add in composer.json:

{
    "repositories": [
        {
            "type": "vcs",
            "url":  "git@github.com:gregurco/GuzzleBundle.git"
        },
        {
            "type": "vcs",
            "url":  "git@github.com:gregurco/GuzzleBundleWssePlugin.git"
        }
    ],
    "require": {
        ...
        "eightpoints/guzzle-bundle": "dev-wsse-plugin",
        "gregurco/guzzle-bundle-wsse-plugin": "dev-init"
    }
}

And run: composer update

florianpreusner commented 7 years ago

First of all: Thank you so much for your time and ideas, @gregurco! I like the idea a lot. This gives us more flexibility in the future and I think more plugins could come so we can really put new features into plugins instead of overloading this bundle! 👍

florianpreusner commented 7 years ago

Why are we not using the bundle by Matthias Noback? https://github.com/matthiasnoback/symfony-bundle-plugins If more bundles would follow this concept I think his library would be a nice base. So every bundle is following the same rules (interfaces, ...).

p5vgregurco commented 7 years ago

Thanks, I'm pleased to see positive feedback.

There are some pros and cons of usage matthiasnoback/symfony-bundle-plugins package. First of all I'm not sure that this package was developed with scope to be used in real packages. I thought it was created more as an example (for article). We can partially copy/paste interfaces to us. Finally we need only one interface: for bundle from plugin packages (this one: https://github.com/matthiasnoback/symfony-bundle-plugins/blob/master/src/BundlePlugin.php). What do you think? Any ideas or suggestions?

florianpreusner commented 7 years ago

Yeah, when reading the article it seems to be an example. But when I'm going directly to the bundle the README.md tells me that it can be used. There are also some tests included. Looks like a solid library. And if there is no con I would love to get this bundle integrated. I hope that this plugin bundle will be seen and used as a standard when it comes to symfony bundles and plugin.

gregurco commented 7 years ago

Fine. Later I will integrate it and do additional commit :+1:

gregurco commented 7 years ago

@florianpreusner today I tried to implement symfony-bundle-plugins here and I met a big problem. This package works only with next configuration structure:

bundle_name:
    first_plugin_name:
        ...
    second_plugin_name:
        ...

But in our case we have a little bit another structure. And I can't rewrite, because:

  1. all methods and classes are final
  2. if I will manage somehow to rewrite then the main logic will be rewritten and we will use only interfaces...

So, my suggestion is not to use symfony-bundle-plugins package and to create our interfaces. I better understood the implementation of Matthias Noback today and I think I can make some changes and it will look good :) So, what's your opinion?

florianpreusner commented 7 years ago

Alright, let's do this! 👍

gregurco commented 7 years ago

@florianpreusner so, I think it's ready for review. Could you please take a look in this PR and in https://github.com/gregurco/GuzzleBundleWssePlugin/pull/1 ? I added tests, rewrote and optimized code. Probably I will add more tests. Write if something is wrong or could be written better :) Above I wrote how to install new version of bundle and wsse plugin.

florianpreusner commented 7 years ago

Very nice! Thanks a lot, @gregurco

gregurco commented 7 years ago

You are welcome :) I think #73, #79, #90 can be closed after release.