增加微信支付通知处理器 #105

lianup · 2022-01-19T12:03:09Z

No description provided.

xy-peng · 2022-01-20T02:39:31Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/auth/WechatPayNotifyHandler.java

+            throw new IllegalArgumentException("serialNumber为空");
+        }
+        if (message == null || message.length == 0) {
+            throw new IllegalArgumentException("message为空");


这种都是实时的外部不可控输入，不应该用IllegalArgumentException这样的 RuntimeException 的，而是 ValidationException 的。切记不能因为外部输入导致程序直接异常呀。

另外，如果只能从 builder 构造，参数检查不是做过一遍了吗？很多参数都是自己可控的，难道你是想别人扩展NotifyHandler 吗

已修改参数校验

xy-peng · 2022-01-20T02:40:20Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/auth/WechatPayNotifyHandler.java

@@ -0,0 +1,75 @@
+package com.wechat.pay.contrib.apache.httpclient.auth;


感觉不应该放在 auth 下，只是 auth 的一个应用场景？

可以新建notify，放到notify下

xy-peng · 2022-01-20T02:42:07Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/auth/WechatPayNotifyHandler.java

+/**
+ * @author lianup
+ */
+public class WechatPayNotifyHandler {


名字要加 wechatpay 前缀吗，除了最外层的 builder 其他地方都没加呢

我觉得应该叫做 NotificationHandler

移动到notify，前缀可去掉

用名词notification ？

xy-peng · 2022-01-20T02:50:05Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/auth/WechatPayNotifyRequest.java

+        this.verifyMessage = verifyMessage;
+        this.body = body;
+        // 获取请求体需要信息
+        parseBody();


先验签再解析吧，只有是微信支付的回调，才有解析的意义

xy-peng · 2022-01-20T03:33:30Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/auth/WechatPayNotifyRequest.java

+/**
+ * @author lianup
+ */
+public class WechatPayNotifyRequest {


我担心这样直接暴露实现给开发者，不利于未来的扩展，就像图片上传一样。而是提供 interface，interface只包含验签所需要的字段，即

serialno

message

signature

而没有timestamp和nonce。

再针对接口提供实现会更好。

可以修改为：

增加Request接口，包括getSerialNumber()、getMessage()、getSignature()方法。

增加NotifyRequest类，实现Request接口。

1还可以增加getBody()方法，也属于request的一部分。

xy-peng

配套的readme呢？

xy-peng · 2022-01-20T08:35:31Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/NotificationHandler.java

+                            getAssociatedData, new String(bodyNonce), ciphertext);
+            throw new DecryptionException(errorMessage, e);
+        }
+        return data;


这里返回一个完整的回调结构体，而不仅仅是加密的部分吧。还有id/type/summary等等对商户可能有价值信息。

可以返回一个parseResult，包括id、create_time、event_type、data（解密后的数据）、summary。

xy-peng · 2022-01-20T08:59:02Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/Request.java

+package com.wechat.pay.contrib.apache.httpclient.notification;
+
+/**
+ * @author lianup


对外接口加一些注释？

lianup · 2022-01-20T09:01:15Z

配套的readme呢？

打算在bump version时添加

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/ParseResult.java

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/NotificationRequest.java

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/NotificationHandler.java

Eric-Lee-Handyman · 2022-01-21T07:01:24Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/Notification.java

+        }
+
+        @Override
+        public String toString() {


使用 ObjectMapper 来代替手写？

此处用到toString的地方为拼接错误信息，在错误中抛出，商户测试和使用时也可以直接用toString打印。

Eric-Lee-Handyman · 2022-01-21T07:01:30Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/Notification.java

+    private String decryptData;
+
+    @Override
+    public String toString() {


使用 ObjectMapper 来代替手写？

此处用到toString的地方为拼接错误信息，在错误中抛出，商户测试和使用时也可以直接用toString打印。

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/NotificationHandler.java

Eric-Lee-Handyman · 2022-01-21T07:25:36Z

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/NotificationHandler.java

+        try {
+            decryptData = aesUtil.decryptToString(associatedData, bodyNonce, ciphertext);
+        } catch (GeneralSecurityException e) {
+            String errorMessage = String.format("aes解密失败 apiV3Key[%s]， associatedData[%s] bodyNonce[%s] ciphertext[%s]",


Resource也有tostring方法，可以直接用？

已修改。

lianup requested review from EmmetZC, Eric-Lee-Handyman and xy-peng January 19, 2022 12:03

xy-peng requested changes Jan 20, 2022

View reviewed changes

lianup force-pushed the master branch 2 times, most recently from 569e513 to e502cdf Compare January 20, 2022 08:00

xy-peng requested changes Jan 20, 2022

View reviewed changes

lianup force-pushed the master branch 4 times, most recently from d2e41a5 to 04e2e82 Compare January 21, 2022 04:34

Eric-Lee-Handyman reviewed Jan 21, 2022

View reviewed changes

src/main/java/com/wechat/pay/contrib/apache/httpclient/notification/NotificationHandler.java Show resolved Hide resolved

lianup force-pushed the master branch from 1a4e91c to 15995ad Compare January 21, 2022 06:51

Eric-Lee-Handyman reviewed Jan 21, 2022

View reviewed changes

lianup force-pushed the master branch 2 times, most recently from 2fbcd18 to 72fbae1 Compare January 21, 2022 10:06

增加微信支付通知处理器

72fbae1

Eric-Lee-Handyman approved these changes Jan 21, 2022

View reviewed changes

EmmetZC approved these changes Jan 23, 2022

View reviewed changes

lianup linked an issue Jan 23, 2022 that may be closed by this pull request

建议直接在sdk中封装一个对微信支付请求的验签(直接封装进行就不用用户自己封装了) #99

Closed

xy-peng approved these changes Jan 23, 2022

View reviewed changes

lianup merged commit c5d89c7 into wechatpay-apiv3:master Jan 23, 2022

		@@ -0,0 +1,75 @@
		package com.wechat.pay.contrib.apache.httpclient.auth;

增加微信支付通知处理器 #105

增加微信支付通知处理器 #105

Uh oh!

Conversation

lianup commented Jan 19, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xy-peng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lianup commented Jan 20, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!