Feature/add faker integration

[franziskahahn]

This PR adds faker integration to slimdump with two options:

• All "official" faker properties which need no arguments like lastname or word (see https://github.com/fzaninotto/Faker#basic-usage ) when prefix FAKER_ is used (e.g. FAKER_word)
• One additional (can be extended) option "FAKER_name" which returns firstname and lastname separated by space.

Please comment before merging because I would like to add user documentation if everything else was discussed!

Of cause it is still possible to replace column value with static content:

<?xml version="1.0" ?>
<slimdump>
    <table name="users" dump="full" keep-auto-increment="false" condition="`signup_date` >= '2017-01-20 00:00:00'">
        <column name="password" dump="replace" replacement="XXXXX" />
        <column name="username" dump="replace" replacement="FAKER_name" />
    </table>
</slimdump>

[coveralls]

Coverage decreased (-2.5%) to 64.015% when pulling 078263c on franziskahahn:feature/add-faker-integration into cb3553d on webfactory:master.

[franziskahahn]

currently the vuild is failing due to ".PHP Fatal error: Class 'Faker\Factory' not found in /home/travis/build/webfactory/slimdump/src/Webfactory/Slimdump/Config/FakerReplacer.php on line 36" error.
But faker was added to composer.json already. I am not sure how to handle this error in your build system - @mpdude Could you please check this? Thank you

[Matthimatiker]

@franziskahahn Thanks for your effort!

To speed up the build, the composer.lock has been put under version control.
Updating the lock file via

composer update fzaninotto/faker

and pushing it should fix the error.

[franziskahahn]

@Matthimatiker ah okay, I see.... 😅
I can commit the lock file on monday :-/

I will also check the decreased test coverage then!

[coveralls]

Coverage increased (+2.8%) to 69.388% when pulling 4c126de on franziskahahn:feature/add-faker-integration into cb3553d on webfactory:master.

[coveralls]

Coverage increased (+2.8%) to 69.388% when pulling 4c126de on franziskahahn:feature/add-faker-integration into cb3553d on webfactory:master.

[coveralls]

Coverage increased (+2.8%) to 69.388% when pulling 4c126de on franziskahahn:feature/add-faker-integration into cb3553d on webfactory:master.

[Matthimatiker]

@mpdude @MalteWunsch Any objections to merge? Looks good to me.

[mpdude]

Only quickly skimmed the code, so possibly dumb questions:

This only works for the default Faker formatters that require no arguments, and "FAKER_name" has special case handling/extra code?

What would we need to change to design this in a more open way that makes it possible to use/register custom Faker formatters as well?

I am asking because in the cases where we are running fixtures with Faker we came up with custom formatters pretty quickly.

[MalteWunsch]

My impulse was to shorten it to return strpos($replacement, self::PREFIX) === 0;, but that seems to be a matter of personal taste, so please go along.

[MalteWunsch]

Being a fan of static code analysis, I don't really like the mapping alias => function name, resulting in the IDE marking these functions as unused. If the functions are that small, one might want to consider inlining them to the mapping, if the functions get bigger, one might want to extract classes.

But as we have only one such function so far, this is not a real issue and nothing we could reasonably decide now. Additionally, this not-real-issue is contained in the FakerReplacer, which I'm perfectly happy to see as a black box. So, everything fine with me.

[MalteWunsch]

Nothing for this PR to care about, but I smell an emerging switch statement, which could be replaced in a more object oriented way. I haven't thought about it yet, but some kind of configurable Provider comes into mind. Depending on the solution, one might even get rid of the hard Faker dependency.

But again, this is more of a reminder for ourselves than a critique on the PR, which is fine with me. Thanks!

[MalteWunsch]

I've added some non-blocking suggestions/worries, but to be fair, they all relate to the architecture of slimdump and are clearly out of the scope of this PR.

So no objections and thanks a lot for your effort, @franziskahahn ! I'd be happy to see the merge as soon as possible.

@mpdude I suggest to separate our concerns from the PR and do other improvements on demand.

[franziskahahn]

@mpdude

This only works for the default Faker formatters that require no arguments, and "FAKER_name" has special case handling/extra code?

That's correct. _name is just one thing I would need and I can imagint that I am not alone with this issue. This is used as an example for the current way to extend the functionality.

What would we need to change to design this in a more open way that makes it possible to use/register custom Faker formatters as well?

I didn't think about it yet but I think it is a good idea. Maybe it is possible to add a new layer which makes extensions possible (maybe totally independent of faker).

[franziskahahn]

If we decide to merge this I could write documentation in a new PR or within this one (?)..

[MalteWunsch]

@franziskahahn Talked to @mpdude earlier, he agreed to postpone the worries and asked me to thank you, too.

It would be great if you could add the documentation in a separate PR, and after that I'd be glad to tag a new version.

[mpdude]

Yes - big thanks to you @franziskahahn for this addition!