Skip to content

HDDS-15808. Replace json-simple with Jackson in httpfsgateway#10710

Merged
adoroszlai merged 9 commits into
apache:masterfrom
smengcl:HDDS-15808
Jul 10, 2026
Merged

HDDS-15808. Replace json-simple with Jackson in httpfsgateway#10710
adoroszlai merged 9 commits into
apache:masterfrom
smengcl:HDDS-15808

Conversation

@smengcl

@smengcl smengcl commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Generated-by: Claude Code (Opus 4.8)

What changes were proposed in this pull request?

The hadoop-ozone/httpfsgateway module was the only remaining consumer of com.googlecode.json-simple:json-simple (1.1.1), a library that has been unmaintained since 2012. Jackson (jackson-databind) is already a compile dependency of the module, so json-simple was redundant and carried needless maintenance and supply chain risk.

This PR migrates all JSON serialization in httpfsgateway from json-simple to Jackson, then removes the dependency:

  • JSONMapProvider now serializes the response Map with a shared ObjectMapper. AUTO_CLOSE_TARGET is disabled so the underlying response stream stays open for the trailing newline and container managed close, matching the previous behavior.
  • JSONProvider, whose only purpose was serializing json-simple JSONStreamAware entities, is removed. JAX-RS providers are registered by Jersey package scanning via jersey.config.server.provider.packages, so deleting the class is sufficient to unregister it.
  • In InstrumentationService, the Timer, VariableHolder, and Sampler snapshot types no longer implement JSONAware/JSONStreamAware. Their getJSON() methods return a LinkedHashMap annotated with @JsonValue, so Jackson serializes each object as that map and produces the same JSON.
  • FSOperations and HttpFSServer use Map/List (LinkedHashMap/ArrayList) in place of JSONObject/JSONArray, and reuse the module local JsonUtil.toJsonString helper.
  • The dependency is removed from the module pom, and the version property plus the dependencyManagement entry are removed from the root pom. The com.googlecode.json-simple:json-simple entries in the distribution bin/LICENSE.txt and the generated jar-report.txt inventory are removed accordingly. The module also declares jackson-core and jackson-annotations explicitly, since the code now references JsonGenerator and @JsonValue directly rather than only transitively through jackson-databind.

The wire format is preserved. Both libraries emit standard JSON, AtomicLong serializes as a bare number in both, and switching from HashMap to LinkedHashMap only makes key order deterministic, which WebHDFS clients (which parse by key) are unaffected by. The replaceAll("\\\\", "") call in FSCreateSnapshot, originally there to undo json-simple's forward slash escaping (\/), is removed. Jackson does not escape slashes, and stripping all backslashes could corrupt legitimately escaped values.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15808

How was this patch tested?

Two focused unit tests were added to the module (which previously had almost no coverage for these paths):

  • TestInstrumentationSerialization verifies the @JsonValue serialization of Timer, VariableHolder, and Sampler produces the expected JSON keys and values.
  • TestJSONMapProvider verifies the provider emits compact JSON followed by a trailing newline and does not close the underlying stream.

Verified locally: the module compiles, all module unit tests pass (2 existing + 5 new), checkstyle.sh is clean, and rat.sh reports 0 unknown licenses.

The httpfsgateway module was the only remaining consumer of the
unmaintained com.googlecode.json-simple:json-simple library. Jackson
(jackson-databind) is already a compile dependency, so this migrates all
JSON serialization to Jackson and drops json-simple from the build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 10, 2026 07:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the hadoop-ozone/httpfsgateway module off the unmaintained json-simple library by switching JSON serialization to Jackson (already present as a dependency), and then removing json-simple from the build and distribution inventories.

Changes:

  • Replaced json-simple-based JAX-RS JSON serialization with Jackson in providers and response-building code paths.
  • Updated httpfsgateway JSON-producing server operations to build Map/List structures instead of JSONObject/JSONArray.
  • Added focused unit tests for the new Jackson serialization behavior and removed json-simple from module and distro license inventories.

Reviewed changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
hadoop-ozone/httpfsgateway/src/test/java/org/apache/ozone/lib/wsrs/TestJSONMapProvider.java Adds tests validating JSON framing (newline) and stream-close behavior for JSONMapProvider.
hadoop-ozone/httpfsgateway/src/test/java/org/apache/ozone/lib/service/instrumentation/TestInstrumentationSerialization.java Adds tests validating @JsonValue-driven Jackson serialization shape for instrumentation snapshots.
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/lib/wsrs/JSONProvider.java Removes the json-simple JSONStreamAware writer provider.
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/lib/wsrs/JSONMapProvider.java Switches map serialization from json-simple to Jackson with stream-close behavior preserved.
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/lib/service/instrumentation/InstrumentationService.java Removes json-simple interfaces and uses @JsonValue to serialize snapshot types as maps.
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/fs/http/server/HttpFSServer.java Updates endpoints to return Map-backed JSON payloads instead of JSONObject.
hadoop-ozone/httpfsgateway/src/main/java/org/apache/ozone/fs/http/server/FSOperations.java Replaces JSONObject/JSONArray construction with Map/List and uses JsonUtil for serialization.
hadoop-ozone/httpfsgateway/pom.xml Removes the json-simple dependency from the httpfsgateway module.
hadoop-ozone/dist/src/main/license/jar-report.txt Removes json-simple.jar from the distribution jar inventory.
hadoop-ozone/dist/src/main/license/bin/LICENSE.txt Removes com.googlecode.json-simple:json-simple from the distribution LICENSE inventory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PMD's UnusedPrivateMethod flagged the three @JsonValue-annotated
getJSON() methods (Timer, VariableHolder, Sampler) as unused because it
does not see Jackson's reflective invocation. Reducing them to
package-private takes them out of the rule's scope while keeping them
effectively internal; Jackson serializes @jsonvalue methods at any
visibility.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 10, 2026 07:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.

@adoroszlai

adoroszlai commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Thanks @smengcl for the patch. There are some PMD failures:

PMD Failure: org.apache.ozone.lib.service.instrumentation.Timer:251 Rule:UnusedPrivateMethod Priority:3 Avoid unused private methods such as 'getJSON()'..
PMD Failure: org.apache.ozone.lib.service.instrumentation.VariableHolder:298 Rule:UnusedPrivateMethod Priority:3 Avoid unused private methods such as 'getJSON()'..
PMD Failure: org.apache.ozone.lib.service.instrumentation.Sampler:342 Rule:UnusedPrivateMethod Priority:3 Avoid unused private methods such as 'getJSON()'..

Oh, I see already fixed.

…ale comment

FSCreateSnapshot no longer strips all backslashes from the serialized
JSON. That strip existed only to undo json-simple's forward-slash
escaping (\/); Jackson does not escape forward slashes, so the strip was
a no-op for normal paths and would corrupt any value Jackson legitimately
escapes (\", \\, \n). Return the Jackson output as-is.

Also reword the misspelled, stale VisibilityModifier comment on
VariableHolder.var to describe the actual reason for the suppression.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 10, 2026 07:56
…onAware

The toJSON Javadoc still described the value as a json-simple JsonAware
instance. Update it to reflect Jackson serialization.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 4 comments.

Copilot AI review requested due to automatic review settings July 10, 2026 08:02
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

hadoop-ozone/httpfsgateway/pom.xml:52

  • The PR description says the json-simple version property and dependencyManagement entry were removed from the root pom.xml, but they are still present (eg pom.xml:121 and pom.xml:459-463). If json-simple is truly no longer used anywhere, please remove those root-pom entries as well (or update the PR description/dist license inventory accordingly) so the dependency is fully eliminated.
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-databind</artifactId>
    </dependency>
    <dependency>
      <groupId>jakarta.ws.rs</groupId>
      <artifactId>jakarta.ws.rs-api</artifactId>
    </dependency>

Copilot AI review requested due to automatic review settings July 10, 2026 08:07
…ypes

Tighten the generic types of the JSON array locals introduced by the
migration (previously json-simple JSONArray) to their actual element
types: List<Map<String, Object>> for status/policy lists and List<String>
for ACL entries and storage type names. No change to serialized JSON.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

hadoop-ozone/httpfsgateway/pom.xml:52

  • The module dependency on json-simple is removed here, but the repository root pom.xml still defines <json-simple.version> and a dependencyManagement entry for com.googlecode.json-simple:json-simple (eg pom.xml:121 and pom.xml:459-463). This contradicts the PR description’s claim that the root pom cleanup is included, and it leaves stale dependency metadata behind. Please remove those root pom entries (and any other remaining references) to complete the migration/removal.
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-databind</artifactId>
    </dependency>
    <dependency>
      <groupId>jakarta.ws.rs</groupId>
      <artifactId>jakarta.ws.rs-api</artifactId>
    </dependency>

Copilot AI review requested due to automatic review settings July 10, 2026 08:14
The root pom still declared the json-simple version property and a
dependencyManagement entry after the module dependency was dropped. No
module or source references json-simple anymore, so remove both to
complete the removal.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

Narrow the JSON array introduced by the migration to
List<Map<String, Object>> and parameterize the raw Map locals and return
type in xAttrsToJSON. The unchecked/rawtypes suppression is no longer
needed. No change to serialized JSON.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 10, 2026 08:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

…eway

The migration references jackson-core (JsonGenerator.Feature in
JSONMapProvider) and jackson-annotations (@jsonvalue in
InstrumentationService) directly, but the module only pulled them in
transitively via jackson-databind. maven-dependency-plugin analyze-only
flagged them as used-undeclared. Declare both explicitly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 10, 2026 08:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.

@adoroszlai adoroszlai marked this pull request as ready for review July 10, 2026 10:31
@adoroszlai adoroszlai added dependencies Pull requests that update a dependency file code-cleanup Changes that aim to make code better, without changing functionality. labels Jul 10, 2026
@adoroszlai adoroszlai merged commit 3c8f936 into apache:master Jul 10, 2026
189 of 193 checks passed
@adoroszlai

Copy link
Copy Markdown
Contributor

Thanks @smengcl for the patch.

@smengcl

smengcl commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @adoroszlai for the review and merging this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-cleanup Changes that aim to make code better, without changing functionality. dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants