{"action":"created","issue":{"url":"https://api.github.com/repos/nelmio/alice/issues/355","repository_url":"https://api.github.com/repos/nelmio/alice","labels_url":"https://api.github.com/repos/nelmio/alice/issues/355/labels{/name}","comments_url":"https://api.github.com/repos/nelmio/alice/issues/355/comments","events_url":"https://api.github.com/repos/nelmio/alice/issues/355/events","html_url":"https://github.com/nelmio/alice/pull/355","id":154878186,"number":355,"title":"Harden parameters and references","user":{"login":"theofidry","id":5175937,"avatar_url":"https://avatars.githubusercontent.com/u/5175937?v=3","gravatar_id":"","url":"https://api.github.com/users/theofidry","html_url":"https://github.com/theofidry","followers_url":"https://api.github.com/users/theofidry/followers","following_url":"https://api.github.com/users/theofidry/following{/other_user}","gists_url":"https://api.github.com/users/theofidry/gists{/gist_id}","starred_url":"https://api.github.com/users/theofidry/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/theofidry/subscriptions","organizations_url":"https://api.github.com/users/theofidry/orgs","repos_url":"https://api.github.com/users/theofidry/repos","events_url":"https://api.github.com/users/theofidry/events{/privacy}","received_events_url":"https://api.github.com/users/theofidry/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/nelmio/alice/labels/Needs%20review","name":"Needs review","color":"b60205"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2016-05-14T22:10:28Z","updated_at":"2016-05-19T07:48:24Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/nelmio/alice/pulls/355","html_url":"https://github.com/nelmio/alice/pull/355","diff_url":"https://github.com/nelmio/alice/pull/355.diff","patch_url":"https://github.com/nelmio/alice/pull/355.patch"},"body":"Closes #284\r\nCloses #358\r\n\r\nProposed changes:\r\n\r\n- Fixture references should be lowercase and only be composed of letters, digits, periods (`.`), underscores (`_`) and slashes (`/`). The current policy is to deprecate it, although I expect one case or two to no longer work; but those should be very weird edge cases that are more bugs than anything.\r\n- Rework `Parameterized` processor to process only `Processable`s which holds a parameter\r\n- Add **LOADS** of tests\r\n- Add some warning in the doc regarding fixture references names and parameters\r\n\r\n/cc @Soullivaneuh for #284."},"comment":{"url":"https://api.github.com/repos/nelmio/alice/issues/comments/220251833","html_url":"https://github.com/nelmio/alice/pull/355#issuecomment-220251833","issue_url":"https://api.github.com/repos/nelmio/alice/issues/355","id":220251833,"user":{"login":"matt-usurp","id":1379839,"avatar_url":"https://avatars.githubusercontent.com/u/1379839?v=3","gravatar_id":"","url":"https://api.github.com/users/matt-usurp","html_url":"https://github.com/matt-usurp","followers_url":"https://api.github.com/users/matt-usurp/followers","following_url":"https://api.github.com/users/matt-usurp/following{/other_user}","gists_url":"https://api.github.com/users/matt-usurp/gists{/gist_id}","starred_url":"https://api.github.com/users/matt-usurp/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/matt-usurp/subscriptions","organizations_url":"https://api.github.com/users/matt-usurp/orgs","repos_url":"https://api.github.com/users/matt-usurp/repos","events_url":"https://api.github.com/users/matt-usurp/events{/privacy}","received_events_url":"https://api.github.com/users/matt-usurp/received_events","type":"User","site_admin":false},"created_at":"2016-05-19T07:48:24Z","updated_at":"2016-05-19T07:48:24Z","body":"Your points are very valid around testing and I would understand the reasoning if you were moving to the `symfony/config` component but currently we are parsing custom `yaml` with custom parsers. So totally understand the want for \"standardising\" the references I just feel like removing the case of characters might be a little much. Removing special characters I totally agree with. \n\nYeah, the wonders of opening code up to people means you will get a lot of \"creative use cases\". Maybe this would be a feature but if it was possible that an objects content was generated before its reference then we could use variable names in the reference too. Essentially we need a way of targeting these specific instances as other fixtures rely heavily on targeting the correct data. An example being maybe a list of products and creating fixed amount of orders for these products would require us knowing the `@reference`. Our use case is that the fixture data isn't truly production data but we as close as possible. Working with numbers `{1..9}` is not except able for large datasets.\n\nBut like I raised in a previous ticket (on phone so cannot find it) the use of `current()` with the `identity()` format would solve most of my issues that I had with this change. The directory structure thing can be easily replaced with underscore and period format (that we use for service definitions anyway). \n\n> On 17 May 2016, at 10:47, Théo FIDRY <notifications@github.com> wrote:\n> \n> Thanks for sharing your thoughts @matt-usurp.\n> \n> My reasons for those changes:\n> \n> I find very error prone to have @user and @User that would give two different results\n> Special characters are problematic as some are already used: {, }, -, (, ), > and complicates a lot of other regex, which makes it harder to maintain and ensure they work correctly\n> Not every regex are case-sensitive, which means there is already potential bugs when relying on it\n> We have absolutely no test for such cases which means we can't guarantee the result\n> It's common for service declaration to have lowercase service names with a limited set of special characters (usually ., _ and -).\n> The rationale behind is that right now, there is not much checks done. This is cool as it offers great flexibility. The issue is that it as a lot, lot more testing. The current tests are good enough for most of the common use cases but they often miss dark edge cases. Having \"weird\" references is very far from being tested thoroughly if at all. Hence my intend to offer less flexibility on this side to ease maintenance and develop feature on our side and offer better reliability.\n> \n> That said, it's not set in stone. It sounds like a reasonable choice to me, but I'm happy to discuss of the problematic use cases that arise.\n> \n> Now for your problem of directory structure convention: I'm not sure how does it cause a problem? Unless you have 2 directories with the same name and one lower case, the other uppercase, it doesn't change anything on that side right? (also you could use / after that PR in the names to match the directory structure btw)\n> \n> We also have this case where its nice to list all the names in the fixtures that are canonical and generate slugs or alias based on those\n> \n> That's an interesting use case but it's one where I would say you are misusing alice flexibility and should be done another way. The first problem is that it's very easy to insert special characters in the list name that way, (ex: Alice's Cat) which can be problematic for the reasons mentioned before. The second is that this could be easily done by having a custom faker provider where you have a whole list of those possible values instead:\n> \n> Class:\n>   \"reference_{1..5}\":\n>     name (unique): <slugName()>\n>     alias: <(someUrlFriendlyFunction($name))>\n> Now the only drawback I see is you're not able to do anymore \"I know I have X values, I want a fixture for each of it\" in a dynamic manner. But in that case I would argue that you could have your list of fixed fixtures as a parameter and have your fixtures matching its size:\n> \n> parameters:\n>   slugs: [Alice, White Rabbit, Mad Hatter, 'Alice\\'s Cat']\n> \n> Class:\n>   reference_<size(slugs)>:\n>     name (unique): <slugName()>\n>     alias: <(someUrlFriendlyFunction($name))>\n> (It doesn't work right now), but my point is I would find that more flexible and robust, and would prefer to add this new feature (which would go well with #347), than relying on your current way which looks more hacky and much more problematic if you were to get the references of your generated fixtures for example.\n> \n> That said:\n> \n> We don't plan to remove it in v2 at least, only deprecating it. So if that's something working well for you now and changing it would make things hard, there is no need to upgrade right away.\n> V3 still have a few months before being out, you got time.\n> —\n> You are receiving this because you were mentioned.\n> Reply to this email directly or view it on GitHub\n> \n"}}