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

Correct payment sandbox endpoint and add a method to get sandbox sign key #666

Merged
merged 14 commits into from
Apr 27, 2017

Conversation

skyred
Copy link
Contributor

@skyred skyred commented Apr 26, 2017

Following #665, this PR implements the 2nd approach.

@mingyoung
Copy link
Collaborator

Incomplete kit!

@skyred
Copy link
Contributor Author

skyred commented Apr 26, 2017

@mingyoung 请阅读方案2: #665 (comment), 这个PR是实现这个方案。我目前测试成功。如果你不同意这个方案,或者认为还缺少哪里,请给出理由。

@overtrue
Copy link
Collaborator

请参考: https://github.com/overtrue/wechat/blob/master/src/Core/AccessToken.php#L99 正确使用缓存类,按你的写法就是写死了只能使用文件系统缓存了。

@skyred
Copy link
Contributor Author

skyred commented Apr 27, 2017

不要使用Else而用Return? 第一次听说。但是为什么 codacy/pr 只对这一个else作出警告呢?

} else {
throw new Exception($result->return_msg);
}
} catch (Exception $e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这样里面的 throw 没有意义吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里的throw目的是抓住$result->return_msg

Copy link
Collaborator

Choose a reason for hiding this comment

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

抓住了也没处理啊?感觉不需要外层的 try 了吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

throw new Exception($result->return_msg); 这个是为了抓住微信业务上的错误信息,在这里throw,直接到了 } catch (Exception $e) { 如果没有这个throw new Exception($result->return_msg);, 当微信返回 sandbox_signkey错误的时候,应用就收不到相对因的错误信息,并且继续 $this->sandboxSignKey 还是为空,然后就再次$request, 那么就会死循环。

这里的try catch可以省略

Copy link
Collaborator

Choose a reason for hiding this comment

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

对啊,就是因为你外面 catch 了又啥事儿没做,所以我才说里面的是没有必要的嘛

@overtrue
Copy link
Collaborator

@mingyoung 👍

@@ -76,11 +93,13 @@ class API extends AbstractAPI
/**
* API constructor.
*
* @param \EasyWeChat\Payment\Merchant $merchant
* @param \EasyWeChat\Payment\Merchant $merchant
* @param \EasyWeChat\Payment\Cache|null $cache
Copy link
Collaborator

Choose a reason for hiding this comment

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

EasyWeChat\Payment\Cache 忘记 use 了吧

} else {
throw new Exception($result->return_msg);
}
} catch (Exception $e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

抓住了也没处理啊?感觉不需要外层的 try 了吧

@overtrue overtrue merged commit 4c072dd into w7corp:master Apr 27, 2017
overtrue added a commit that referenced this pull request Apr 27, 2017
@overtrue
Copy link
Collaborator

@skyred 方法名称为 getXXX 就应该是有返回值的,否则你应该将它命名为操作形名称:setXXX 。我已经修复了该问题。

overtrue added a commit that referenced this pull request Apr 27, 2017
@skyred skyred deleted the patch-2 branch April 27, 2017 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants