-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update to Gradle 8.9 #7554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Update to Gradle 8.9 #7554
Conversation
The recommendation is to switch to the UniformRandomProvider. This would however add a new dependency that's only used in the unit tests. [1] https://issues.apache.org/jira/browse/LANG-1604
…tAsLong The buyerSecurityDepositAsLong field was superseded by the buyerSecurityDepositAsPercent field.
It's too risky to refactor the code because it's used in the account age verification logic.
WalkthroughThis PR adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java (1)
88-100: Deprecation suppression requires a migration plan—deprecated proto APIs are not stable long-term.The
@SuppressWarnings("deprecation")suppresses warnings but does not address the underlying risk. Per protobuf's deprecation policy, non-experimental deprecated Java APIs are commonly removed ~2 years after deprecation and may be removed in major or minor releases. Simply suppressing the warnings without a documented migration plan or timeline creates technical debt and risks build failures in future protobuf upgrades.Either establish a tracking issue with a migration timeline to replace deprecated proto method calls, or document why the current approach is acceptable (e.g., if protoc/runtime versions are pinned indefinitely).
🧹 Nitpick comments (11)
desktop/src/test/java/bisq/desktop/ComponentsDemo.java (1)
52-123: Consider fixing the root cause instead of suppressing the warning.The
uncheckedwarning originates from the raw type usage ofObservableListon line 121. Rather than suppressing the warning, parameterize the list to eliminate it at the source.Apply this diff to fix the root cause:
- ObservableList list = FXCollections.observableArrayList(); + ObservableList<String> list = FXCollections.observableArrayList();Then remove the suppression annotation:
- @SuppressWarnings("unchecked") @Override public void start(Stage primaryStage) throws Exception {core/src/main/java/bisq/core/offer/OpenOfferManager.java (1)
959-960: Appropriate suppression for migration code.The method-level scope is correct since
maybeUpdatePersistedOffers()intentionally accesses deprecatedOfferPayloadfields (e.g.,getArbitratorNodeAddresses(),isCurrencyForMakerFeeBtc()) to migrate legacy persisted offers.Consider adding a brief comment documenting which deprecated APIs trigger this warning, making future cleanup easier:
+ // Suppress deprecation: accesses legacy OfferPayload fields for migration (e.g., arbitratorNodeAddresses, isCurrencyForMakerFeeBtc) @SuppressWarnings("deprecation") private void maybeUpdatePersistedOffers() {desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java (1)
93-104: Consider using ArrayList to avoid the unchecked cast.The
@SuppressWarnings("unchecked")annotation is appropriate for this Gradle 8.9 upgrade, and the cast at line 104 is safe sincebufferonly containsTelements. However, you can eliminate the warning entirely by using:action.accept(new ArrayList<>(buffer).stream());This approach creates the same snapshot behavior without requiring a cast or suppression annotation.
desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java (1)
126-141: LGTM - Both suppressions are justified.The
@SuppressWarnings({"rawtypes", "unchecked"})annotations are appropriate here:
rawtypes: TheViewLoader.load()API returns a rawViewtypeunchecked: The cast toTuple2on line 134 is necessarily unchecked sincedataisObjectMinor improvement: You could use a wildcard type for the
instanceofcheck to be slightly more explicit about the raw type usage:if (view instanceof BsqSendView) { toggleGroup.selectToggle(send); - if (data instanceof Tuple2) { - ((BsqSendView) view).fillFromTradeData((Tuple2) data); + if (data instanceof Tuple2<?, ?>) { + ((BsqSendView) view).fillFromTradeData((Tuple2<?, ?>) data); }This would still require
uncheckedbut would eliminate therawtypeswarning.desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java (1)
189-242: Local deprecation suppression forclonedOfferPayloadis acceptable, but consider future migrationThe suppression is tightly scoped to the deprecated
OfferPayloadconstructor usage and keeps the rest of the method warning-free, which is fine for this Gradle upgrade pass. Longer term, consider refactoring to whatever non-deprecated factory/builder replaces this constructor to gradually reduce reliance on deprecated APIs.core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java (1)
114-133: Targeted suppression keeps this deserializer quiet under Gradle 8.9The new @SuppressWarnings("deprecation") on fromProto is appropriately scoped and preserves existing behavior constructing CashDepositAccountPayload.
If you touch this area again, you could consider tightening the return type to
CashDepositAccountPayloadfor symmetry with other payloads, but that’s not required for this Gradle-focused change.core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java (1)
95-104: Method-level deprecation suppression is consistent and low-riskfromProto is now the only place in this class with deprecation suppressed, which is appropriate given it directly reads deprecated proto fields.
Longer term, once the proto APIs are fully migrated, this annotation should be easy to remove since it’s confined to a single factory method.
core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java (1)
102-121: Deprecation suppression on fromProto matches the project-wide patternThis keeps the deprecation handling localized to the deserialization path and avoids affecting compile-time checks elsewhere in the class.
If more methods in this class start relying on deprecated proto APIs, you might later switch to a class-level suppression to avoid repetition, but the current method-level scope is preferable for now.
p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java (1)
47-51: Unchecked warning suppression in test fake is acceptable (minor nit)Using
@SuppressWarnings("unchecked")on the constructor is fine for this in-memory test helper. If you ever want to avoid method-level suppression, you could instead givePersistenceManageran explicit generic type in the mock to satisfy the compiler, but that’s purely cosmetic here.p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java (1)
32-35: Unchecked suppression is fine for this test fake; generics could be a later cleanupAdding
@SuppressWarnings("unchecked")on the constructor is a pragmatic way to silence the raw‑type/generics warning foraddService(new MapStoreServiceFake())in test code. If you want to tighten things later, you could parameterizeAppendOnlyDataStoreServiceFakeandMapStoreServiceFakewith concrete type arguments instead of relying on a raw base type, but that’s not required for this PR.core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java (1)
196-244: Reasonable deprecation suppression on legacy republish path
maybeRepublishTradeStatisticsstill needs to call deprecatedTradeStatistics2/3.from(...)factories for backward compatibility, so suppressing deprecation warnings at the method level is a sensible, contained choice. No functional changes here; any follow‑up to remove the deprecated types can happen independently of this Gradle upgrade.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (77)
apitest/src/test/java/bisq/apitest/scenario/TradeTest.java(2 hunks)core/src/main/java/bisq/core/offer/OpenOfferManager.java(1 hunks)core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PixAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java(1 hunks)core/src/main/java/bisq/core/proto/CoreProtoResolver.java(2 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java(3 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatisticsManager.java(1 hunks)core/src/main/java/bisq/core/user/PreferencesPayload.java(0 hunks)desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java(1 hunks)desktop/src/main/java/bisq/desktop/components/MenuItem.java(1 hunks)desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java(1 hunks)desktop/src/main/java/bisq/desktop/main/dao/wallet/BsqWalletView.java(1 hunks)desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java(1 hunks)desktop/src/main/java/bisq/desktop/main/portfolio/cloneoffer/CloneOfferDataModel.java(1 hunks)desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java(1 hunks)desktop/src/main/java/bisq/desktop/util/MovingAverageUtils.java(1 hunks)desktop/src/test/java/bisq/desktop/ComponentsDemo.java(1 hunks)gradle/verification-metadata.xml(38 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/mocks/AppendOnlyDataStoreServiceFake.java(1 hunks)p2p/src/test/java/bisq/network/p2p/storage/mocks/MapStoreServiceFake.java(1 hunks)proto/src/main/proto/pb.proto(1 hunks)
💤 Files with no reviewable changes (1)
- core/src/main/java/bisq/core/user/PreferencesPayload.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Java 11, windows-latest
- GitHub Check: Test Java 11, macOS-latest
- GitHub Check: Test Java 11, ubuntu-latest
🔇 Additional comments (60)
gradle/wrapper/gradle-wrapper.properties (1)
3-3: Verify gradle-wrapper.jar was also updated.Gradle 8.9 is a stable release that improves error and warning reporting for variant issues, exposes structural details of Java compilation errors for IDE integrators, and includes additional minor improvements. The wrapper URL update is correctly formatted.
However, the Gradle wrapper JAR file (
gradle/wrapper/gradle-wrapper.jar) typically needs to be updated alongside the wrapper properties file. Verify that this file was regenerated or updated to match the 8.9 distribution. You can regenerate it by running:./gradlew wrapper --gradle-version 8.9Additionally, given the significant scope mentioned in the PR summary (60+ files with changes across proto, annotations, and resource management), confirm that:
- All build plugins and tools are compatible with Gradle 8.9
- CI/CD pipelines are configured to support Gradle 8.9
- Gradle 8.9 provisioning uses a different marker file (.ready), so all existing toolchains will be re-provisioned on first use
apitest/src/test/java/bisq/apitest/scenario/TradeTest.java (2)
81-82: LGTM - appropriate deprecation suppression for test instantiation.The variable-level
@SuppressWarnings("deprecation")annotation correctly suppresses warnings for the test object instantiation. Note that this test is currently disabled.
104-105: LGTM - consistent with other test suppressions.The annotation pattern matches the previous test method and correctly suppresses deprecation warnings. This test is also currently disabled.
core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java (1)
73-80: LGTM - consistent deprecation suppression.The annotation follows the same pattern applied across payment payload classes for proto conversion methods.
core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java (1)
80-92: LGTM - deprecation suppression aligned with PR strategy.The annotation is correctly applied to the proto conversion method, consistent with the broader Gradle 8.9 upgrade changes.
core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java (1)
74-81: LGTM - standard proto deprecation suppression.The annotation correctly suppresses deprecation warnings for the proto conversion logic.
core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java (1)
70-77: LGTM - consistent annotation pattern.The deprecation suppression is appropriately applied to the proto conversion method.
core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java (1)
67-74: LGTM - deprecation handling complete.The annotation follows the established pattern for proto conversion methods across all payment payload classes.
p2p/src/test/java/bisq/network/p2p/storage/messages/AddDataMessageTest.java (1)
67-85: LGTM - test-level deprecation suppression with noted TODO.The annotation appropriately suppresses deprecation warnings for this protobuf conversion test. Note the commented TODO on line 79 regarding the use of proto resolvers, which may be related to the deprecation concerns.
core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java (1)
80-92: LGTM - Appropriately scoped deprecation suppression.The
@SuppressWarnings("deprecation")annotation is correctly placed on thefromProtomethod to suppress warnings from the protobuf-generated API. The annotation is narrowly scoped to only this method rather than the entire class.desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep2View.java (1)
242-244: Acceptable, though the scope is broad.The
@SuppressWarnings("deprecation")annotation is placed on theaddContent()method which spans ~250 lines with many payment method form invocations. While this is a large scope for suppression, applying it at method level is still preferable to class level. The deprecation warnings likely stem from the various*Form.addFormForBuyer()calls interacting with deprecated payment payload APIs.core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java (1)
74-84: LGTM - Consistent with the PR pattern.The
@SuppressWarnings("deprecation")annotation is correctly and narrowly scoped to thefromProtomethod, consistent with the same pattern applied across other payment payload classes in this PR.core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java (1)
82-90: LGTM - Consistent with the PR pattern.The
@SuppressWarnings("deprecation")annotation is correctly and narrowly scoped to thefromProtomethod, matching the pattern applied to other payment payload classes in this PR.desktop/src/main/java/bisq/desktop/main/account/content/fiataccounts/FiatAccountsView.java (1)
612-725: Scoped deprecation suppression on payment-method form factory looks goodSuppressing deprecation warnings at this private factory method keeps the scope tight and avoids noise elsewhere, with no behavioral impact.
core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java (1)
81-90: fromProto deprecation suppression is consistent and low-riskAdding
@SuppressWarnings("deprecation")on this staticfromProtoaligns with other payloads, keeps warnings centralized at the deserialization boundary, and doesn’t alter behavior.core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java (1)
74-84: StrikeAccountPayload.fromProto suppression matches the shared patternThe deprecation suppression is correctly placed on the static
fromProtofactory and keeps the method signature and logic intact.desktop/src/main/java/bisq/desktop/components/MenuItem.java (1)
141-149: Unchecked suppression on generic array creation is reasonable hereThe
@SuppressWarnings("unchecked")is scoped to the local array variable produced from a well-typedList<Class<? extends View>>, so it safely silences the unavoidable generic-array warning without widening the suppression scope.core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java (1)
82-94: TransferwiseUsdAccountPayload.fromProto deprecation suppression looks fineThis keeps deprecation warnings localized at the proto deserialization boundary and is consistent with other payload classes in the PR.
desktop/src/main/java/bisq/desktop/common/fxml/FxmlViewLoader.java (1)
60-100: Expanding suppression onloadto include deprecation is acceptableThe
loadmethod is an integration point that necessarily touches deprecated APIs, so adding"deprecation"to the existing"unchecked"suppression is a pragmatic choice and doesn’t affect runtime behavior.core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java (1)
67-74: CelPayAccountPayload.fromProto deprecation suppression aligns with other payloadsThe annotation cleanly suppresses deprecation at the serialization boundary without altering logic or signatures.
core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java (1)
72-79: Deprecation suppression onfromProtolooks correctScoping
@SuppressWarnings("deprecation")to thefromProtofactory is appropriate here; it keeps existing proto mapping intact and avoids leaking suppression more broadly.core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java (1)
75-85: Consistent deprecation suppression onfromProtoApplying
@SuppressWarnings("deprecation")at thefromProtomethod level matches the pattern across payloads and avoids noise from proto deprecations without altering behavior.core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java (1)
77-85: Method-level suppression is appropriateThe
@SuppressWarnings("deprecation")onfromProtocleanly isolates proto-related deprecation warnings while keeping the deserialization logic as-is.core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java (1)
105-115: Minimal-scope suppression on proto builder is sensibleUsing
@SuppressWarnings("deprecation")on the localprotobuf.PaymentAccountPayload.Builderkeeps the deprecation handling tightly scoped to the deprecated constructor call and avoids hiding other potential warnings in this class.core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java (1)
72-79:fromProtodeprecation suppression matches project patternThe added
@SuppressWarnings("deprecation")onfromProtois consistent with other payment payloads and keeps proto deprecation noise localized.core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java (1)
89-106: Deprecation suppression on bank-accountfromProtois fineAdding
@SuppressWarnings("deprecation")tofromProtocleanly handles deprecated proto usage without affecting the bank account field mapping.core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java (1)
74-84: PaytmfromProtosuppression is correctly scopedThe new
@SuppressWarnings("deprecation")onfromProtois consistent with similar payloads and keeps the Gradle/Java toolchain clean without changing the deserialization behavior.core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java (1)
72-80: MonesefromProtodeprecation suppression looks goodThe
@SuppressWarnings("deprecation")annotation onfromProtois in line with the rest of the payload classes and introduces no behavioral change.core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java (1)
89-89: LGTM: Appropriate deprecation warning suppression.The
@SuppressWarnings("deprecation")annotation onfromProtomethods across all payment payload files is appropriate for handling protobuf deprecation warnings. This is a standard pattern when working with deprecated protobuf APIs that cannot be immediately updated.Note: This comment applies to the identical changes in NationalBankAccountPayload.java, ClearXchangeAccountPayload.java, PixAccountPayload.java, InteracETransferAccountPayload.java, USPostalMoneyOrderAccountPayload.java, SbpAccountPayload.java, and WesternUnionAccountPayload.java.
proto/src/main/proto/pb.proto (1)
1923-1923: Verify removal of deprecated field references.Unable to automatically verify removal of
buyer_security_deposit_as_longandbuyerSecurityDepositAsLongreferences from the codebase due to repository access limitations. Please manually confirm:
- No remaining references to this field exist in Java, proto, or other source files
- All dependent code that previously used field 30 has been updated or removed
- Backward compatibility for reading older persisted data is maintained (typically handled automatically by protobuf field reservation)
core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java (1)
101-115: Method-level deprecation suppression here is appropriateAnnotation is narrowly scoped to the factory method that touches deprecated proto APIs and doesn’t affect behavior; matches the pattern used across other payloads.
core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java (1)
67-74: Localized suppression keeps deprecation noise containedThe deprecation warning is suppressed only on the proto factory, which is the right granularity and leaves the rest of the class warning-clean.
core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java (1)
72-79: Consistent deprecation suppression on proto factoryfromProto is the only place using deprecated proto accessors, so method-level suppression here is reasonable and keeps the rest of the class strict.
core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java (1)
71-78: Deprecation suppression is scoped correctlyThe warning is suppressed only where the deprecated proto fields are read, which is a sensible, minimal change.
core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java (1)
74-84: Suppression aligns with the rest of the proto factoriesAdding @SuppressWarnings("deprecation") directly on fromProto cleanly isolates the deprecated proto usage without touching surrounding logic.
core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java (1)
80-80: LGTM - Consistent with PR-wide pattern.The deprecation suppression follows the same approach as other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java (1)
74-74: LGTM - Consistent with PR-wide pattern.The deprecation suppression follows the same approach as other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java (1)
73-73: LGTM - Consistent with PR-wide pattern.The deprecation suppression follows the same approach as other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java (1)
89-89: LGTM - Consistent with PR-wide pattern.The deprecation suppression follows the same approach as other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java (1)
94-94: LGTM - Consistent with PR-wide pattern.The deprecation suppression follows the same approach as other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java (1)
94-94: LGTM - Consistent with PR-wide pattern.The deprecation suppression follows the same approach as other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java (1)
74-74: LGTM - Consistent with PR-wide pattern.The deprecation suppression follows the same approach as other payment payload classes in this PR.
core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java (1)
110-110: Verify whether the suppressed methods are standard protobuf APIs or project-specific generated/custom code.The methods referenced—
getExcludeFromJsonDataMap()andgetMaxTradePeriod()—do not appear in standard Google Protocol Buffers documentation. Before requesting migration to alternative APIs, confirm whether these are generated code from Bisq-specific.protofiles, custom helper methods, or internal wrappers. If they are project-generated, examine the corresponding.protodefinitions and the protoc version used; if they are custom methods, consult the project's internal API documentation for deprecation status and recommended alternatives.core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java (1)
73-80: Deprecation suppression onfromProtolooks appropriateMethod-level
@SuppressWarnings("deprecation")keeps the mapping logic intact and localized; the proto field mapping remains symmetrical withtoProtoMessage(). No issues from my side.core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java (1)
142-165: Consistent deprecation suppression onfromProtoAdding
@SuppressWarnings("deprecation")here is in line with other payloads and keeps the proto‑to‑POJO mapping unchanged. Looks good.core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java (1)
78-89: Method-scoped deprecation suppression is fine here
@SuppressWarnings("deprecation")onfromProtois targeted, aligns with the rest of the PR, and doesn’t alter behavior. No further changes needed.core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java (1)
75-82:fromProtodeprecation warning suppression looks goodThe added
@SuppressWarnings("deprecation")is localized tofromProto, and the field mapping remains identical. LGTM.core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java (1)
97-113: Deprecation suppression on SEPA proto boundary methods is reasonableAdding
@SuppressWarnings("deprecation")to bothtoProtoMessage()andfromProto(...)keeps warnings confined to the proto boundary while preserving existing serialization/deserialization behavior. This matches the pattern elsewhere in the PR.Also applies to: 115-129
gradle/verification-metadata.xml (1)
2-2: Verification metadata updates appear structurally consistent with Gradle 8.9 upgradeThe schema bump to
dependency-verification-1.3.xsdand the added entries for develocity, updated Kotlin/Jacoco/ASM artifacts, etc., match the existing XML structure (components, artifacts, sha256). Assuming these were regenerated via Gradle’s dependency-verification tooling and./gradlewverification passes, I don’t see issues.If you haven’t already, please re-run Gradle’s dependency verification (e.g.,
./gradlew help --verify-dependencies) to confirm all checksums and schema usage are accepted by Gradle 8.9.Also applies to: 972-979, 2972-2979, 3034-3041, 3050-3057, 3066-3070, 3077-3083, 3092-3099, 3172-3176, 3185-3232, 3241-3248, 3257-3264, 3273-3280, 3289-3296, 3305-3312, 3313-3320, 3332-3342, 3351-3358, 3359-3366, 3375-3382, 3383-3390, 3399-3406, 3415-3422, 3431-3438, 3447-3454, 3455-3462, 3471-3478, 3487-3494, 3495-3502, 3511-3518, 3527-3534, 3543-3550, 3559-3566, 3575-3582, 3615-3625, 4132-4136, 4153-4160, 4177-4181, 4198-4205, 4222-4229, 3034-3091
core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java (1)
75-84: Deprecation suppression on FasterPayments proto methods is fineThe new
@SuppressWarnings("deprecation")annotations ontoProtoMessage()andfromProto(...)are consistent with other payment payloads and don’t alter the serialization logic. Looks good.Also applies to: 86-95
core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java (1)
70-77: Method‑scoped deprecation suppression looks appropriateThe
@SuppressWarnings("deprecation")is narrowly scoped tofromProto, which keeps the warning noise down without masking issues elsewhere. No behavioral change introduced here.core/src/main/java/bisq/core/trade/statistics/TradeStatisticsConverter.java (1)
57-112: Targeted deprecation suppressions around legacy trade stats are reasonableAdding
@SuppressWarnings("deprecation")on the constructor and bothconvertToTradeStatistics3overloads keeps the compiler quiet while you still need to interop withTradeStatistics2and related deprecated pieces. Scope is tight and does not affect behavior.Also applies to: 119-157, 159-181
core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java (1)
67-74: Consistent deprecation suppression on proto factoryThe
@SuppressWarnings("deprecation")onfromProtomatches the pattern used in other payment payloads for legacy proto fields. Mapping code remains unchanged.core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java (1)
80-89: Localized deprecation suppression on fromProtoSuppressing deprecation warnings directly on
fromProtokeeps the proto interop path intact while avoiding noise from deprecated fields. No functional impact.core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java (1)
82-91: Deprecation suppression on Amazon gift card fromProto is consistentThe
@SuppressWarnings("deprecation")onfromProtofollows the same pattern as other payment payloads dealing with legacy proto fields. Implementation remains unchanged.core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java (1)
75-83: Swish fromProto deprecation suppression is aligned with the restAdding
@SuppressWarnings("deprecation")directly tofromProtois a minimal, well‑scoped way to keep compatibility with deprecated proto elements while compiling cleanly under Gradle 8.9.core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java (2)
231-282: Scoped deprecation suppression ontoProtoMessageis appropriateLimiting
@SuppressWarnings("deprecation")to the proto serialization method keeps the unavoidable use of deprecated proto fields contained while keeping the rest of the class honest with respect to deprecation warnings. No behavioral change, and consistent with the PR’s warning‑cleanup goal.
284-337:fromProtodeprecation suppression is well‑targetedApplying
@SuppressWarnings("deprecation")only on the staticfromProto(protobuf.OfferPayload)factory cleanly isolates deprecation noise to the deserialization boundary, which is where backward‑compatibility with older proto schemas is expected. The construction logic remains unchanged.core/src/main/java/bisq/core/proto/CoreProtoResolver.java (2)
101-259: Deprecation suppression onPaymentAccountPayloadresolver is justifiedThis resolver must continue to handle older, deprecated payment account variants for existing data, so placing
@SuppressWarnings("deprecation")on the method is a sensible, narrowly scoped way to keep the Gradle 8.9 build clean without changing behavior.
261-285:PersistableNetworkPayloadresolver suppression matches compatibility needsSimilarly, adding
@SuppressWarnings("deprecation")to thefromProto(protobuf.PersistableNetworkPayload)override cleanly acknowledges that it works with deprecated proto payloads while preserving full backward compatibility. No logic or error‑handling changes introduced.
| public void copyInputStreamToFileNew(final InputStream source, | ||
| final File destination, | ||
| int fileSize) throws IOException { | ||
| try (final FileOutputStream output = FileUtils.openOutputStream(destination)) { | ||
| final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; | ||
| long count = 0; | ||
| int n; | ||
| while (EOF != (n = source.read(buffer))) { | ||
| output.write(buffer, 0, n); | ||
| count += n; | ||
| log.trace("Progress: {}/{}", count, fileSize); | ||
| updateProgress(count, fileSize); | ||
| } | ||
| } finally { | ||
| IOUtils.closeQuietly(source); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputStream is never closed, introducing a resource leak
download(...) calls urlConnection.getInputStream() and passes it here, but this method only wraps the FileOutputStream in try‑with‑resources. The source InputStream is never closed on any path, which can leak HTTP/file resources and also silently changes the previous contract where this method closed the stream.
You can restore safe behavior and keep the new style by including the InputStream in the try‑with‑resources:
- public void copyInputStreamToFileNew(final InputStream source,
- final File destination,
- int fileSize) throws IOException {
- try (final FileOutputStream output = FileUtils.openOutputStream(destination)) {
- final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
- long count = 0;
- int n;
- while (EOF != (n = source.read(buffer))) {
- output.write(buffer, 0, n);
- count += n;
- log.trace("Progress: {}/{}", count, fileSize);
- updateProgress(count, fileSize);
- }
- }
- }
+ public void copyInputStreamToFileNew(final InputStream source,
+ final File destination,
+ int fileSize) throws IOException {
+ try (InputStream input = source;
+ FileOutputStream output = FileUtils.openOutputStream(destination)) {
+ final byte[] buffer = new byte[DEFAULT_BUFFER_SIZE];
+ long count = 0;
+ int n;
+ while (EOF != (n = input.read(buffer))) {
+ output.write(buffer, 0, n);
+ count += n;
+ log.trace("Progress: {}/{}", count, fileSize);
+ updateProgress(count, fileSize);
+ }
+ }
+ }This keeps the new resource‑safety for the output file and also ensures the input stream is always closed, matching the prior behavior.
🤖 Prompt for AI Agents
In
desktop/src/main/java/bisq/desktop/main/overlays/windows/downloadupdate/DownloadTask.java
around lines 127 to 140, the method copyInputStreamToFileNew wraps only the
FileOutputStream in try-with-resources so the passed-in InputStream "source" is
never closed; modify the method to include the source InputStream in the
try-with-resources (i.e., try (InputStream src = source; FileOutputStream output
= FileUtils.openOutputStream(destination)) { ... }) so the input stream is
always closed on all paths, preserving the previous behavior and avoiding
resource leaks while keeping the output stream handling unchanged.
Summary by CodeRabbit
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.