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

[discuss] delete the Seata integration servlet. HandlerInterceptor #3151

Closed
slievrly opened this issue Feb 15, 2023 · 6 comments
Closed

[discuss] delete the Seata integration servlet. HandlerInterceptor #3151

slievrly opened this issue Feb 15, 2023 · 6 comments
Labels

Comments

@slievrly
Copy link
Member

slievrly commented Feb 15, 2023

Background

Seata has integrated well with SCA in the past, but some issues have been exposed. Today, we will mainly discuss whether SeataHandlerInterceptor needs to be delete in SCA, and extend the boundary of Seata and SCA integration.

Current

There are 2 implements for interface org.springframework.web.servlet.HandlerInterceptor

spring-cloud-alibaba-seata-starter depend on seata-spring-boot-starter

The function of this code is to process the transaction context of Seata at different stages of the lifecycle after the servlet receives the request. There is a clear duplication in functionality.

Based on the current situation, the issues are:

  • Repeated execution, resulting in an execution interceptor order problem. At the same time, it creates confusion for developers.
  • High maintenance costs. It is possible that the two sides could make changes to the class out of sync (such as different release times) leading to accuracy issues.

Solution

delete SeataHandlerInterceptor.java in SCA.

There are other reasons besides solving the issues:

  • HandlerInterceptor is not only suitable for Spring Cloud Web projects, but also for Spring MVC. Therefore, the HandlerInterceptor is larger than the Spring Cloud in scope. The advantage of this is that if a user is just developing a Spring MVC project and wants to use it with Seata, they don't have to Import complex Spring Cloud dependencies.

Extension

Seata has integrated well with SCA in the past. Mainly include: AutoConfiguration, restTemplate, openFeign (including LoadBalancer, Hytrix, Sentinel)

Previously, Seata and SCA each maintained some unique configurations and were not unified. In addition, Seata's configuration is relatively complex. In order to solve this problem Seata oneself maintainseata-spring-boot-starter module to used autoConfiguration. SCA changes from the previous dependency of seata-all to seata-spring-boot-starter, directly using the Seata project's own autoConfiguration feature.

welcome to discuss this issue and extend the responsibilities of Seata and SCA integration boundary, and look forward to more suggestions and comments.


背景

在过去Seata与SCA做了很好的集成,但是在目前暴露出了一些问题。今天我们主要讨论在SCA中SeataHandlerInterceptor是否需要删除问题,并以此延伸Seata与SCA集成中的代码边界问题。

现状与问题

这里有2个接口实现了 org.springframework.web.servlet.HandlerInterceptor

spring-cloud-alibaba-seata-starter 依赖 seata-spring-boot-starter

这段代码的功能是在servlet收到请求后的在不同的生命周期阶段对Seata的事务上下文进行处理。从代码上看在SCA与Seata代码中各自维护了一份,存在着功能的重复。

基于现状,产生的问题有:

  • 重复执行,产生执行拦截器顺序问题。同时也给开发者造成了困惑。
  • 维护成本高。对这个类的修改可能两边不同步(比如发版时间不同)导致产生正确性问题。

方案

删除 SCA中的 SeataHandlerInterceptor.java

除了能解决上述问题,这么做的理由是:

  • HandlerInterceptor 不仅仅适应于Spring Cloud Web项目,同时也适应于Spring MVC。所以,从作用范围上HandlerInterceptor 要大于Spring Cloud。这样做的好处是如果用户仅仅开发一个Spring MVC项目想结合Seata使用,不必引入复杂的Spring Cloud依赖。

延伸

Seata 与 SCA 过去做了很好的集成主要包括: autoConfiguration, restTemplate, openFeign (including LoadBalancer, Hytrix, Sentinel)

在之前 Seata 和 SCA 各自维护了一些特有的配置并且不统一。另外,Seata的配置相对复杂。为了解决这个问题Seata 自己维护了 seata-spring-boot-starter 模块用于 autoConfiguration。SCA 由之前的依赖 seata-all 更改为 seata-spring-boot-starter,直接使用 Seata 自身的 AutoConfiguration 功能。

期待大家对此问题和Seata与SCA的集成边界职责展开更多的讨论和意见输入。

@steverao steverao added the kind/discussion Mark as discussion issues/pr label Feb 15, 2023
@steverao
Copy link
Collaborator

There are some historical reasons here. As far as the current situation, there is really no need to repeatedly intercept. Spring Cloud Alibaba only needs to deal with some clients designed to remote procedure calls. This related interceptor for the Web can be considered to be deleted from Spring Cloud Alibaba in the future.

@l81893521
Copy link

I agree with delete one of them from the Spring Cloud Alibaba. Cause the org.springframework.web.servlet.HandlerInterceptor has different order in preHandle, postHandle, afterCompletion and if intercept repeatedly may bring some unimaginable case.

@funky-eyes
Copy link
Contributor

funky-eyes commented Feb 16, 2023

I think that 'Seata' interceptor should be replaced with filter so that it will be more generic under non-spring, the 'seata-http' module should remove the dependency on spring, enable filter in the 'spring-boot-starter' module, and no longer maintain interceptor and other functions for receiving transaction information on the SCA side
我认为Seata的Interceptor应该换成filter这样将在非spring下更加通用,seata-http模块应该移除对spring的依赖,在spring-boot-starter模块中进行启用filter,sca侧不再维护interceptor等用于事务信息接收的功能

@Bughue
Copy link

Bughue commented Feb 16, 2023

I can see the disadvantages of simply removing the interceptor from either SCA or seata-http

  1. If removed from SCA: xid-send and xid-receive are not maintained in one place, so if this feature changes there will be some trouble.
  2. If removed from seata-http: the user has to introduce the SCA or implement the interceptor once by itself.

From the development point of view, feign/http-client has similar problems, except that it does not have such problems in use. I think seata-http should focus on the following 2 things, so is it possible to split it into two more modules?

  1. seata-http-core: depends only on javax.servlet, reads/writes http header
  2. seata-http: depends on seata-http-core, depends on springmvc/apache-http, provides default http capabilities, he can be replaced by SCA

Copy link

This issue has been open 30 days with no activity. This will be closed in 7 days.

@github-actions github-actions bot added the stale label Mar 17, 2024
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity.If you think this should still be open, or the problem still persists, just pop a reply in the comments and one of the maintainers will (try!) to follow up. Thank you for your interest and contribution to the Sping Cloud Alibaba Community.

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

No branches or pull requests

5 participants