[
https://issues.apache.org/jira/browse/FINERACT-2723?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Foo Developer updated FINERACT-2723:
------------------------------------
Description:
{{StandingInstructionData}} declares several fields of type {{EnumOptionData}}
({{fromAccountType}}, {{toAccountType}}, {{transferType}}, {{instructionType}},
{{recurrenceType}}, {{recurrenceFrequency}}). The class also declares
same-named getter methods that return a different, internal domain-enum type
instead, e.g.:
public PortfolioAccountType getFromAccountType() {
return Optional.ofNullable(this.fromAccountType).map(e ->
PortfolioAccountType.fromInt(e.getId().intValue())).orElse(null);
}
This is confusing to read and maintain: a field and its same-named "getter"
disagree on type, and a future refactor or a different serialization path could
easily start relying on the wrong one. In this specific class, the API response
is unaffected today - {{StandingInstructionApiResource}} serializes via
{{DefaultToApiJsonSerializer}}, which is Gson-based and reads the raw
{{EnumOptionData}} field directly regardless of these methods - but the naming
collision is still a latent correctness hazard worth removing.
Proposed fix (pure internal rename, no behavior or API change): rename the six
domain-enum helper methods to a distinct name ({{getFromAccountTypeEnum()}},
{{getToAccountTypeEnum()}}, {{getTransferTypeEnum()}},
{{getInstructionTypeEnum()}}, {{getRecurrenceTypeEnum()}},
{{getRecurrenceFrequencyEnum()}}) so they no longer shadow the field name, and
add an explicit getter on each underlying field for clarity/consistency with
the rest of the class. Internal callers of the old getter names are updated to
use the renamed methods, with added null-safety since the domain-enum helpers
can return {{null}} when the underlying {{EnumOptionData}} field is {{null}}.
was:
{{StandingInstructionData}} declares several fields of type {{EnumOptionData}}
({{fromAccountType}}, {{toAccountType}}, {{transferType}}, {{instructionType}},
{{recurrenceType}}, {{recurrenceFrequency}}), each intended to serialize as an
{{EnumOptionData}} object ({{{id, code, value}}}) in the API response.
However, the class also declares same-named getter methods that return a
different, internal domain-enum type instead, e.g.:
public PortfolioAccountType getFromAccountType() {
return Optional.ofNullable(this.fromAccountType).map(e ->
PortfolioAccountType.fromInt(e.getId().intValue())).orElse(null);
}
Since none of the affected fields have their own explicit getter, Jackson's
bean-property introspection resolves the property "fromAccountType" (and the
other five) to this method alone, and serializes the internal domain enum's own
representation instead of the intended {{EnumOptionData}} object. This produces
incorrect/confusing data for these fields wherever a Standing Instruction is
returned by the API (e.g. the Standing Instructions view/list endpoints),
including account type, transfer type, instruction type, and recurrence
type/frequency.
Proposed fix: rename the six domain-enum helper methods to a distinct name
({{getFromAccountTypeEnum()}}, {{getToAccountTypeEnum()}},
{{getTransferTypeEnum()}}, {{getInstructionTypeEnum()}},
{{getRecurrenceTypeEnum()}}, {{getRecurrenceFrequencyEnum()}}) so they no
longer shadow the field-backed Jackson property, and add an explicit getter on
each underlying field so the correct {{EnumOptionData}} object is serialized.
Internal callers of the old getter names are updated to use the renamed
methods, with added null-safety since the domain-enum helpers can return
{{null}} when the underlying {{EnumOptionData}} field is {{null}}.
Summary: StandingInstructionData: rename ambiguous domain-enum getters
to avoid confusion with EnumOptionData fields (was: StandingInstructionData
getters shadow EnumOptionData fields causing incorrect JSON serialization)
> StandingInstructionData: rename ambiguous domain-enum getters to avoid
> confusion with EnumOptionData fields
> -----------------------------------------------------------------------------------------------------------
>
> Key: FINERACT-2723
> URL: https://issues.apache.org/jira/browse/FINERACT-2723
> Project: Apache Fineract
> Issue Type: Bug
> Reporter: Foo Developer
> Assignee: Foo Developer
> Priority: Major
> Labels: java
>
> {{StandingInstructionData}} declares several fields of type
> {{EnumOptionData}} ({{fromAccountType}}, {{toAccountType}}, {{transferType}},
> {{instructionType}}, {{recurrenceType}}, {{recurrenceFrequency}}). The class
> also declares same-named getter methods that return a different, internal
> domain-enum type instead, e.g.:
> public PortfolioAccountType getFromAccountType() {
> return Optional.ofNullable(this.fromAccountType).map(e ->
> PortfolioAccountType.fromInt(e.getId().intValue())).orElse(null);
> }
> This is confusing to read and maintain: a field and its same-named "getter"
> disagree on type, and a future refactor or a different serialization path
> could easily start relying on the wrong one. In this specific class, the API
> response is unaffected today - {{StandingInstructionApiResource}} serializes
> via {{DefaultToApiJsonSerializer}}, which is Gson-based and reads the raw
> {{EnumOptionData}} field directly regardless of these methods - but the
> naming collision is still a latent correctness hazard worth removing.
> Proposed fix (pure internal rename, no behavior or API change): rename the
> six domain-enum helper methods to a distinct name
> ({{getFromAccountTypeEnum()}}, {{getToAccountTypeEnum()}},
> {{getTransferTypeEnum()}}, {{getInstructionTypeEnum()}},
> {{getRecurrenceTypeEnum()}}, {{getRecurrenceFrequencyEnum()}}) so they no
> longer shadow the field name, and add an explicit getter on each underlying
> field for clarity/consistency with the rest of the class. Internal callers of
> the old getter names are updated to use the renamed methods, with added
> null-safety since the domain-enum helpers can return {{null}} when the
> underlying {{EnumOptionData}} field is {{null}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
