Fix Jackson exception handling due to 3.x release line migration

[reta]

Description

Fix Jackson exception handling due to 3.x release line migration

Related Issues

Temporary fix exception handling while working on #4783

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 77df780.

Path	Line	Severity	Description
common/src/main/java/org/opensearch/ml/common/MLCommonsClassLoader.java	29	high	Import of JsonParseException changed from the well-known 'com.fasterxml.jackson.core' package to 'org.opensearch.tools.jackson.core'. This substitutes a standard, widely-audited library class with one from an unfamiliar namespace. The same change is applied in two test files. Without a corresponding verified build dependency declaration, this could indicate a shaded or counterfeit class masquerading as a Jackson type. Maintainers must verify that 'org.opensearch.tools.jackson' is an official, trusted OpenSearch-published artifact and that the corresponding build files (pom.xml / build.gradle) pull it from a trusted registry.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 1 | Medium: 0 | Low: 0

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 870a012)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Migrate Jackson imports from com.fasterxml to org.opensearch.tools Relevant files: common/src/main/java/org/opensearch/ml/common/MLCommonsClassLoader.java common/src/test/java/org/opensearch/ml/common/MLCommonsClassLoaderTests.java common/src/test/java/org/opensearch/ml/common/model/MLDeployingSettingTests.java Sub-PR theme: Fix test mocks and assertions for Jackson 3.x API changes Relevant files: plugin/src/test/java/org/opensearch/ml/processor/MLInferenceSearchResponseTests.java plugin/src/test/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessorTests.java search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeSearchResponseTests.java search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/ext/GenerativeQAParametersTests.java ml-algorithms/src/test/java/org/opensearch/ml/engine/algorithms/agent/MLAgentExecutorTest.java

⚡ Recommended focus areas for review Debug Statement A System.out.println debug statement was added in the onFailure callback of a test. This should be removed before merging as it pollutes test output and is not appropriate for production test code. System.out.println("Message: " + e.getMessage()); Duplicate Import There are duplicate imports for any() — both org.mockito.ArgumentMatchers.any and org.mockito.Mockito.any are imported. This can cause ambiguity and compilation issues. The org.mockito.Mockito.any import should be removed in favor of org.mockito.ArgumentMatchers.any. import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.any; Removed Assertion The line verify(listener, timeout(5000)).onFailure(any()) was removed from test_ExecuteAgent_NonV2Agent_DoesNotRouteToExecuteV2Agent. This assertion verified that the listener's onFailure was called, which is an important behavioral check. Removing it may allow the test to pass even if the execution path is broken. verify(memory, never()).getStructuredMessages(any()); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 870a012
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Remove duplicate conflicting static import There is a duplicate import of any — once from org.mockito.ArgumentMatchers.any and once from org.mockito.Mockito.any. This creates an ambiguous static import that can cause compilation errors or unexpected behavior. Remove the org.mockito.Mockito.any import since org.mockito.ArgumentMatchers.any is the correct one to use alongside anyBoolean. plugin/src/test/java/org/opensearch/ml/processor/MLInferenceSearchResponseTests.java [7-11] import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; Suggestion importance[1-10]: 7 __ Why: The duplicate import of any from both org.mockito.ArgumentMatchers and org.mockito.Mockito creates an ambiguous static import that can cause compilation errors. Removing the org.mockito.Mockito.any import is the correct fix.	Medium
General	Remove debug print statement from test A debug System.out.println statement was left in the test code. This is not appropriate for production test code and should be removed before merging. plugin/src/test/java/org/opensearch/ml/processor/MLInferenceSearchResponseProcessorTests.java [2504] -System.out.println("Message: " + e.getMessage()); + Suggestion importance[1-10]: 5 __ Why: The System.out.println debug statement is not appropriate for production test code and should be removed before merging, though it doesn't affect test correctness.	Low

---

Previous suggestions

Suggestions up to commit dc20767

Category	Suggestion	Impact
General	Verify mock signature matches upgraded API The XContentBuilder constructor may invoke createGenerator with specific argument values. Using anyBoolean() is correct for the boolean parameter, but ensure the mock is set up before new XContentBuilder(xc, os) is called, as the constructor itself triggers createGenerator. This ordering looks correct, but verify that the new 4-argument createGenerator signature matches the actual API being used in the upgraded version to avoid UnnecessaryStubbingException or NullPointerException at runtime. search-processors/src/test/java/org/opensearch/searchpipelines/questionanswering/generative/GenerativeSearchResponseTests.java [73-74] +when(xc.createGenerator(any(), any(), any(), anyBoolean())).thenReturn(generator); +XContentBuilder builder = new XContentBuilder(xc, os); - Suggestion importance[1-10]: 2 __ Why: The existing_code and improved_code are identical, meaning no actual change is suggested. The suggestion only asks to verify the mock setup, which is already correctly implemented in the PR diff.	Low

[reta]

@rithinpullela could you please take a look at this pull request, it fixes tests and addresses recent changes in Jackson dependencies for main / 3.7.0, thank you

[rithinpullela]

Thanks for the PR @reta !

[github-actions]

Persistent review updated to latest commit 870a012

[reta]

Thanks for the PR @reta !

Thanks @rithinpullela could you please reapprove check runs (fixed tests) whenever you have time, thank you

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.41%. Comparing base (3b174ad) to head (870a012).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #4784   +/-   ##
=========================================
  Coverage     77.41%   77.41%           
+ Complexity    11900    11899    -1     
=========================================
  Files           963      963           
  Lines         53310    53310           
  Branches       6500     6500           
=========================================
+ Hits          41268    41271    +3     
- Misses         9287     9291    +4     
+ Partials       2755     2748    -7     

Flag	Coverage Δ
ml-commons	77.41% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[reta]

Regarding failing Windows build, the distribution publishing jobs fails to build Windows archives for quite some time [1], so the quite old build is picked up and it does not cut :(

[1] https://build.ci.opensearch.org/job/distribution-build-opensearch/11842

[dhrubo-os]

Regarding failing Windows build, the distribution publishing jobs fails to build Windows archives for quite some time [1], so the quite old build is picked up and it does not cut :(

[1] https://build.ci.opensearch.org/job/distribution-build-opensearch/11842

What is the path forward in this case?

[reta]

What is the path forward in this case?

Thanks @dhrubo-os, I see 2 options:

• merge this pull request (the only changes are in tests and all non-Windows one pass, sadly we have split brain for checks at the moment)
• wait for distribution build to be fixed (related to protobuf / protoc, have no insights on how long it is going to take)