Copilot commented on code in PR #10710:
URL: https://github.com/apache/ozone/pull/10710#discussion_r3557243707
##########
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/fs/http/server/FSOperations.java:
##########
@@ -1793,9 +1784,9 @@ public FSCreateSnapshot(String path, String snapshotName)
{
@Override
public String execute(FileSystem fs) throws IOException {
Path snapshotPath = fs.createSnapshot(path, snapshotName);
- JSONObject json = toJSON(HttpFSConstants.HOME_DIR_JSON,
+ Map<String, Object> json = toJSON(HttpFSConstants.HOME_DIR_JSON,
snapshotPath.toString());
- return json.toJSONString().replaceAll("\\\\", "");
+ return JsonUtil.toJsonString(json).replaceAll("\\\\", "");
Review Comment:
`FSCreateSnapshot` still strips all backslashes from the JSON output via
`replaceAll("\\\\", "")`. With Jackson this is no longer needed for `\/`
escaping, and it can corrupt/invalidates JSON if the snapshot path ever
contains characters that require escaping (eg a literal backslash).
##########
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/lib/service/instrumentation/InstrumentationService.java:
##########
@@ -295,7 +281,7 @@ public void addCron(String group, String name,
Instrumentation.Cron cron) {
timer.addCron((Cron) cron);
}
- static class VariableHolder<E> implements JSONAware, JSONStreamAware {
+ static class VariableHolder<E> {
// Supressed, because it is only used in this class or in test files,
// but the tests will be removed later.
Review Comment:
The comment above `VariableHolder.var` has a spelling issue ("Supressed")
and states the tests will be removed later, which is no longer accurate now
that serialization tests were added.
##########
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/fs/http/server/FSOperations.java:
##########
@@ -374,17 +373,16 @@ private static Map<String, Object>
quotaUsageToMap(QuotaUsage quotaUsage) {
*
* @return the JSON representation of the key-value pair.
*/
- @SuppressWarnings("unchecked")
- private static JSONObject toJSON(String name, Object value) {
- JSONObject json = new JSONObject();
+ private static Map<String, Object> toJSON(String name, Object value) {
Review Comment:
The Javadoc for `toJSON` still refers to a `JsonAware` type from the removed
json-simple implementation, which is now misleading after the Jackson migration.
--
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]
