killbill / killbill-api

Kill Bill APIs
https://killbill.io
Apache License 2.0
18 stars 34 forks source link

Api 85 address spotbugs errors #106

Closed xsalefter closed 2 years ago

xsalefter commented 2 years ago
  1. Decide to use defensive copy for Throwable.

  2. Use <XXX>.copyOf() for List and Map. This probably cause an issue if other repositories modify those attribute, and need to test. Unfortunately DefaultBlockingState in killbill repository still not up-to-date with the latest API interfaces from killbill-api and have unimplemented interfaces, thus put this as draft.

  3. Add Joda's Period and DateTime in spotbugs-exclude.xml. Most of Joda classeses are immutable.

pierre commented 2 years ago

@xsalefter https://github.com/killbill/killbill/pull/1762 has been merged.

xsalefter commented 2 years ago

tested against killbill. FYI, copy-constructor to Throwable was mistake. Copying from commit desc:

TestPermissionAnnotationMethodInterceptor#verifyAopedTester() in killbill repository show that #cause maybe used to determine actual exception type.

In other words,

private void verifyAopedTester(final IAopTester aopedTester) {
        // Anonymous user
        logout();
        try {
            aopedTester.createRefund();
            Assert.fail();
        } catch (UnauthenticatedException e) {
            // Good!
        } catch (Exception e) {
            Assert.fail(e.getLocalizedMessage());
        }
    // ... Other part of code 
}

Wrapping Throwable make test go to Exception block (instead of UnauthenticatedException)