Skip to content

Upgrade Guava library from 13.0.1 to 32.0.0-jre#66

Open
AbhishekKumar9984 wants to merge 2 commits into
cdapio:cs_guava_upgradefrom
cloudsufi:twill2-guava-upgrade
Open

Upgrade Guava library from 13.0.1 to 32.0.0-jre#66
AbhishekKumar9984 wants to merge 2 commits into
cdapio:cs_guava_upgradefrom
cloudsufi:twill2-guava-upgrade

Conversation

@AbhishekKumar9984
Copy link
Copy Markdown

Migrated all removed and deprecated Guava APIs across 74 files:

  • Objects.toStringHelper() -> MoreObjects.toStringHelper()
  • Objects.hashCode/equal -> java.util.Objects.hash/equals
  • new Stopwatch()/elapsedTime() -> Stopwatch.createStarted()/elapsed()
  • Charsets.UTF_8 -> StandardCharsets.UTF_8
  • InputSupplier/OutputSupplier -> Callable<Reader/Writer>
  • Guava Optional -> java.util.Optional
  • Futures.addCallback/transform 2-arg -> 3-arg with Executor
  • Futures.immediateCheckedFuture -> Futures.immediateFuture
  • Service.start/stop/startAndWait/stopAndWait -> startAsync/stopAsync/awaitRunning/awaitTerminated
  • ServiceListenerAdapter: implements -> extends Service.Listener
  • Ranges/DiscreteDomains -> Range/DiscreteDomain/ContiguousSet
  • Files.createTempDir() -> Files.createTempDirectory()
  • hasher.putString/hashString -> added Charset parameter
  • Files.newOutputStreamSupplier/newReaderSupplier -> Java NIO equivalents
  • getServiceName() -> serviceName()

Migrated all removed and deprecated Guava APIs across 74 files:
- Objects.toStringHelper() -> MoreObjects.toStringHelper()
- Objects.hashCode/equal -> java.util.Objects.hash/equals
- new Stopwatch()/elapsedTime() -> Stopwatch.createStarted()/elapsed()
- Charsets.UTF_8 -> StandardCharsets.UTF_8
- InputSupplier/OutputSupplier -> Callable<Reader/Writer>
- Guava Optional -> java.util.Optional
- Futures.addCallback/transform 2-arg -> 3-arg with Executor
- Futures.immediateCheckedFuture -> Futures.immediateFuture
- Service.start/stop/startAndWait/stopAndWait -> startAsync/stopAsync/awaitRunning/awaitTerminated
- ServiceListenerAdapter: implements -> extends Service.Listener
- Ranges/DiscreteDomains -> Range/DiscreteDomain/ContiguousSet
- Files.createTempDir() -> Files.createTempDirectory()
- hasher.putString/hashString -> added Charset parameter
- Files.newOutputStreamSupplier/newReaderSupplier -> Java NIO equivalents
- getServiceName() -> serviceName()
@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 11, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request upgrades the Guava dependency to version 32.0.0-jre and performs a comprehensive migration to newer Guava and Java standard library APIs. Key changes include transitioning from deprecated Guava service lifecycle methods to the asynchronous startAsync/stopAsync model, replacing Guava's Charsets, Optional, and Objects with standard Java equivalents, and updating Stopwatch and Range usage. Additionally, explicit executors are now provided for Futures transformations and callbacks to comply with newer Guava requirements. Feedback was provided regarding exception handling in ArgumentsCodec, suggesting that RuntimeException should be allowed to propagate rather than being wrapped in an IOException.

Comment on lines +53 to 57
} catch (IOException e) {
throw e;
} catch (Exception e) {
throw new IOException(e);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation catches all Exception types and wraps them in an IOException. This includes RuntimeException, which should generally be allowed to propagate without being wrapped in a checked exception. It is better to catch IOException and RuntimeException explicitly to rethrow them, and only wrap other checked exceptions.

Suggested change
} catch (IOException e) {
throw e;
} catch (Exception e) {
throw new IOException(e);
}
} catch (IOException | RuntimeException e) {
throw e;
} catch (Exception e) {
throw new IOException(e);
}

Comment on lines +64 to 68
} catch (IOException e) {
throw e;
} catch (Exception e) {
throw new IOException(e);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the encode method, catching all Exception types here wraps RuntimeException in an IOException. It's preferable to let RuntimeException propagate unwrapped.

Suggested change
} catch (IOException e) {
throw e;
} catch (Exception e) {
throw new IOException(e);
}
} catch (IOException | RuntimeException e) {
throw e;
} catch (Exception e) {
throw new IOException(e);
}

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