Skip to content

Fix bugs, add missing test coverage, and clean up dead code in auto-exchange annotation framework - #4

Draft
juwencheng with Copilot wants to merge 3 commits into
mainfrom
copilot/review-auto-exchange-rate-code
Draft

Fix bugs, add missing test coverage, and clean up dead code in auto-exchange annotation framework#4
juwencheng with Copilot wants to merge 3 commits into
mainfrom
copilot/review-auto-exchange-rate-code

Conversation

Copilot AI commented Mar 16, 2026

Copy link
Copy Markdown

Code review of the auto-exchange rate annotation across three dimensions: bugs, unit test coverage, and functional completeness.

Bug Fixes

Annotation processor source version mismatch — both processors declared @SupportedSourceVersion(RELEASE_11) on a Java 17 project, causing 2 test failures via compiler warning.

// Before
@SupportedSourceVersion(SourceVersion.RELEASE_11)
// After
@SupportedSourceVersion(SourceVersion.RELEASE_17)

ExchangeManager.init() NPE on null provider response — unlike refreshRates(), init() had no null guard on fetchData() return. Added consistent null check that retains cache on null response.

AutoExchangeContext used wrong map typeHashMap (equals/hashCode) was used to key per-bean appended data. Two distinct instances with equal hashCode/equals would collide. Changed to IdentityHashMap.

ExchangeBeanSerializerModifier.hasAutoExchangeFieldInHierarchy() was a stub — hardcoded return true wrapped every bean in AppendingBeanSerializer regardless of whether it had @AutoExchangeField. Replaced with real class hierarchy inspection backed by a ConcurrentHashMap cache.

Removed deprecated strategy classes with wrong hardcoded valuesAppendApplyExchangeStrategy and InPlaceApplyExchangeStrategy had BigDecimal.valueOf(2) hardcoded as the exchange rate and "CNY" hardcoded as the target currency. Neither class was used. Removed along with AbstractApplyExchangeStrategyDeprecated.

Unit Tests Added

Previously untested paths now covered:

  • ExchangeManagerTestinit() null handling, refreshRates() null retention, cache updates, rate not found
  • AutoExchangeContextTest — identity-based key lookup, multiple fields per bean
  • MissingRateThrowExceptionTestTHROW_EXCEPTION strategy returns HTTP 500 with structured error via AutoExchangeExceptionHandler (0% coverage before)
  • MissingRateProtectiveTestPROTECTIVE strategy applies configured protective rate
  • MissingRateReturnNullTestRETURN_NULL strategy yields null price field
  • Added integration tests for HTTP header-based target currency, null @AutoExchangeField field values (should yield price=0, no NPE), and @AutoExchangeBaseCurrency runtime field resolution

Functional Completeness

  • ApplyExchangeMode enum was defined but never referenced anywhere in the framework. Marked @Deprecated. (ICurrencyExchangeService and DefaultCurrencyExchangeService were already deprecated.)
Original prompt

这个自动应用汇率的注解,请审核下代码,从 bug,ut,功能完备性三个方面审查


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI and others added 2 commits March 16, 2026 05:23
Co-authored-by: juwencheng <2663764+juwencheng@users.noreply.github.com>
…ference

Co-authored-by: juwencheng <2663764+juwencheng@users.noreply.github.com>
Copilot AI changed the title [WIP] Review code for automatic exchange rate feature Fix bugs, add missing test coverage, and clean up dead code in auto-exchange annotation framework Mar 16, 2026
Copilot AI requested a review from juwencheng March 16, 2026 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants