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

Email address causes Alice to look for a fixture file #780

Closed
simshaun opened this issue Aug 26, 2017 · 4 comments
Closed

Email address causes Alice to look for a fixture file #780

simshaun opened this issue Aug 26, 2017 · 4 comments

Comments

@simshaun
Copy link
Contributor

Odd problem. I was on 3.0.0-RC.1 and tried updating to dev-master, but it still bugs out.

My fixture file is as follows, somewhat matching the example provided in the docs:

AppBundle\Entity\User:
    user_disabled_1:
        enabled: false
        firstName: John
        lastName: Doe
        email: [email protected]
        plainPassword: 123456

and trying to load fixtures results in:

[Fidry\AliceDataFixtures\Exception\MaxPassReachedException]
 Loading files limit of 15 reached. Could not load the following files:
  /var/www/cms/backend/src/PlatformBundle/Resources/fixtures/orm/user_disabled.yml:
   - Nelmio\Alice\Throwable\Exception\Generator\Resolver\FixtureNotFoundException: Could not find the fixture "example.com". in /var/www/cms/backend/vendor/nelmio/alice/src/Throwable/Exception/Generator/Resolver/FixtureNotFoundExceptionFactory.php:23

Setting a breakpoint and stepping through, I see that the email property is being parsed into a ListValue containing 2 values:

0: "disabled1"
1: Nelmio\Alice\Definition\Value\FixtureReferenceValue:
    reference: "example.com"

Trying to figure out why it's doing that,

  1. UniqueValueDenormalizer::denormalize is called with [email protected]

  2. SimpleValueDenormalizer::denormalize is called with [email protected]

  3. SimpleValueDenormalizer::parseValue is called with [email protected]

  4. FunctionFixtureReferenceParser::parse is called with [email protected]

  5. StringMergerParser::parse is called with [email protected]

  6. SimpleParser::parse is called with [email protected]

  7. EmptyValueLexer::lex is called with [email protected]

  8. ReferenceEscaperLexer::lex is called with [email protected]

  9. GlobalPatternsLexer::lex is called with [email protected]

  10. FunctionLexer::lex is called with [email protected]

  11. SubPatternsLexer::lex is called with [email protected]

  12. !!!!! This lexer turns [email protected] into 3 tokens:

    0: disabled  [STRING_TYPE]
    1: 1  [STRING_TYPE]
    2: @example.com  [SIMPLE_REFERENCE_TYPE]
    

I think this is ultimately caused by the regex pattern in ReferenceEscaperLexer not matching against [email protected].

The current pattern is (\p{L})@ which matches against any letter, but does not include numbers. I think the pattern needs to be (\p{L}|\p{N})@.

See https://regex101.com/r/4yUIOk/1 for failing regex test.
See https://regex101.com/r/d8VSyt/1 for working regex test.

@theofidry
Copy link
Member

regexes for emails are difficult, I think the easiest way is either to check if we have non empty a STRING_TYPE token immediately followed by a SIMPLE_REFERENCE_TYPE, we could transform the reference into a string.

Alternatively just escape it: disabled1\@example.com

@simshaun
Copy link
Contributor Author

regexes for emails are difficult, I think the easiest way is either to check if we have non empty a STRING_TYPE token immediately followed by a SIMPLE_REFERENCE_TYPE, we could transform the reference into a string.

This would be a new lexer that comes after SubPatternsLexer, right, or would it just be a part of the SubPatternsLexer?

Alternatively just escape it: disabled1\@example.com

I'll have to try this tomorrow. Not sure I like having to escape the @ symbol, but if it works it works.

@theofidry
Copy link
Member

This would be a new lexer that comes after SubPatternsLexer, right, or would it just be a part of the SubPatternsLexer?

A simpler way I think would be to decorate the lexer and modify the list of tokens accordingly. IIRC this is already done somewhere to transform all the successive STRING_TYPE tokens into one. I don't remember exactly where, but that's within the lexer for sure

simshaun added a commit to simshaun/alice that referenced this issue Sep 7, 2017
@simshaun
Copy link
Contributor Author

simshaun commented Sep 7, 2017

I created a PR with a new lexer that does what I think you're saying.

Edit: As an alternative, I tried changing the reference escaper regex to include numbers like I showed in my OP, but it causes some integration tests to fail, so it's not as simple as it seems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants