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

FIX for apparently random API failures while using array types #16137

Conversation

phoenix128
Copy link
Contributor

@phoenix128 phoenix128 commented Jun 14, 2018

Description

Magento2 web-api is based on the Interface doctype, but if the developer is using CR+LF end-of-line, the \Magento\Framework\Reflection\TypeProcessor class is not able to determine the class name on array types.

This was caused by a wrong regex non including \r in \Magento\Framework\Reflection\TypeProcessor::getParamType.

Additional note: Magento 2.2-develop seems to be not affected by this.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

If the webapi Interface class was using CR+LF end-of-line, TypeProcessor class was not able to determine the class name
when an array was used.
@magento-engcom-team
Copy link
Contributor

Hi @phoenix128. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on Pull Request changes
  • @magento-engcom-team give me new test instance - deploy NEW test instance based on Pull Request changes
  • @magento-engcom-team give me {$VERSION} instance - deploy Vanilla Magento instance for Issue or Pull Request

For more details, please, review the Magento Contributor Assistant documentation

@hostep
Copy link
Contributor

hostep commented Jun 14, 2018

Hi @phoenix128

Additional note: Magento 2.2-develop seems to be not affected by this.

I was triggered by this notice, so I started digging somewhat, and found this PR: #12324 which completely removes those regexes.

It seems this was only applied to the 2.2-develop branch and not to the 2.3-develop branch.

Maybe it would be better if that PR is forward ported to 2.3?

(I have no idea what all the code in these PR's is about, so if what I say makes no sense, then ignore me ;))

@phoenix128
Copy link
Contributor Author

Hello @hostep ,
this makes sense. Probably the best option is to forwardport that PR. I do not know why it was not made actually.

Copy link
Member

@larsroettig larsroettig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@magento-engcom-team
Copy link
Contributor

Hi @larsroettig, thank you for the review.
ENGCOM-1998 has been created to process this Pull Request

Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phoenix128 please provide scenario for a supported operating system when this situation may happen. Note that absence of Windows line-endings is enforced by phpcs.

Looks like affected developer forgot to configure git properly with core.autocrlf = input (see https://help.github.com/articles/dealing-with-line-endings/#global-settings-for-line-endings).

@magento-engcom-team
Copy link
Contributor

Hi @phoenix128. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.3.0 release.

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

Successfully merging this pull request may close these issues.

6 participants