Upgrade Guava library from 13.0.1 to 32.0.0-jre#66
Conversation
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()
|
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. |
There was a problem hiding this comment.
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.
| } catch (IOException e) { | ||
| throw e; | ||
| } catch (Exception e) { | ||
| throw new IOException(e); | ||
| } |
There was a problem hiding this comment.
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.
| } 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); | |
| } |
| } catch (IOException e) { | ||
| throw e; | ||
| } catch (Exception e) { | ||
| throw new IOException(e); | ||
| } |
There was a problem hiding this comment.
Similar to the encode method, catching all Exception types here wraps RuntimeException in an IOException. It's preferable to let RuntimeException propagate unwrapped.
| } 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); | |
| } |
Migrated all removed and deprecated Guava APIs across 74 files: