Copilot commented on code in PR #8067:
URL: https://github.com/apache/incubator-seata/pull/8067#discussion_r3158397243
##########
seata-spring-autoconfigure/seata-spring-autoconfigure-client/src/main/java/org/apache/seata/spring/boot/autoconfigure/properties/SeataJsonProperties.java:
##########
@@ -31,13 +31,24 @@
@ConfigurationProperties(prefix = JSON_PREFIX)
public class SeataJsonProperties {
Review Comment:
`serializerType` was added as a new config property but currently has no
Javadoc/description, while `allowlist` is well documented. Please add a brief
explanation (what it controls, expected values like
fastjson/fastjson2/jackson/gson, and how it maps to the underlying config key)
so users know how to configure JSON serialization consistently.
```suggestion
/**
* JSON serializer implementation to use for Seata JSON
serialization/deserialization.
* Supported values include fastjson, fastjson2, jackson, and gson.
* This property is bound from the Spring Boot configuration key {@code
seata.json.serializer-type}
* (derived from {@code JSON_PREFIX}).
*/
```
##########
common/src/main/java/org/apache/seata/common/DefaultValues.java:
##########
@@ -341,11 +341,16 @@ public interface DefaultValues {
*/
String DEFAULT_SAGA_JSON_PARSER = "fastjson";
+ /**
+ * The default global JSON serializer.
+ */
+ String BUSINESS_ACTION_CONTEXT_JSON_PARSER = "jackson";
Review Comment:
The default JSON serializer is being changed here to `jackson` (previously
the default for this setting was `fastjson`). This is a behavior change that
can affect wire format/performance/compatibility for users who rely on
defaults; please confirm this is intended for 2.7.0 and, if so, ensure there is
an explicit migration note/release note (or keep the old default and only
change the sample configs).
```suggestion
String BUSINESS_ACTION_CONTEXT_JSON_PARSER = "fastjson";
```
##########
json-common/json-common-core/src/main/java/org/apache/seata/common/json/JsonUtil.java:
##########
@@ -29,14 +33,32 @@
*/
public final class JsonUtil {
- private static final String CONFIG_JSON_PARSER_NAME =
ConfigurationFactory.getInstance()
- .getConfig(
-
ConfigurationKeys.TCC_BUSINESS_ACTION_CONTEXT_JSON_PARSER_NAME,
-
DefaultValues.DEFAULT_TCC_BUSINESS_ACTION_CONTEXT_JSON_PARSER);
+ private static final Logger LOGGER =
LoggerFactory.getLogger(JsonUtil.class);
+
+ private static final String CONFIG_JSON_PARSER_NAME =
resolveJsonSerializerName(ConfigurationFactory.getInstance());
private static final JsonSerializer DEFAULT_SERIALIZER =
JsonSerializerFactory.getSerializer(CONFIG_JSON_PARSER_NAME);
Review Comment:
`CONFIG_JSON_PARSER_NAME` / `jsonParseName` naming still refers to "parser"
while the new configuration key and method name use "serializer". Consider
aligning these names (and possibly the constant name) to avoid confusion
between parser/serializer terminology now that the setting is
`json.serializerType`.
##########
seata-spring-autoconfigure/seata-spring-autoconfigure-client/src/main/java/org/apache/seata/spring/boot/autoconfigure/properties/SeataTccProperties.java:
##########
@@ -24,12 +24,15 @@
@Component
@ConfigurationProperties(prefix = TCC_PREFIX)
public class SeataTccProperties {
+ @Deprecated
private String contextJsonParserType;
+ @Deprecated
public String getContextJsonParserType() {
return contextJsonParserType;
}
Review Comment:
`contextJsonParserType` is now marked `@Deprecated`, but there is no
indication of the replacement property. Add an `@deprecated` Javadoc (or a
short inline comment) pointing users to `seata.json.serializer-type` /
`json.serializerType` so IDEs and generated docs provide a clear migration path.
```suggestion
public class SeataTccProperties {
/**
* @deprecated Use {@code seata.json.serializer-type} / {@code
json.serializerType} instead.
*/
@Deprecated
private String contextJsonParserType;
/**
* @deprecated Use {@code seata.json.serializer-type} / {@code
json.serializerType} instead.
*/
@Deprecated
public String getContextJsonParserType() {
return contextJsonParserType;
}
/**
* @deprecated Use {@code seata.json.serializer-type} / {@code
json.serializerType} instead.
*/
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
