Merged
Conversation
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR primarily adds/updates Javadoc across the rlib-network module interfaces/utilities to improve API documentation consistency, but it also includes several rlib-collections API/behavior fixes and renames.
Changes:
- Add
@since 10.0.0and richer Javadoc acrossrlib-networkpublic APIs (connections, packets, registries, utils, factory/config types). - Fix typos/rename public APIs in
rlib-collections(DequeFactory.arrayBasedBased→arrayBased,unsafeRemoveByInex→unsafeRemoveByIndex). - Adjust
containsAll(...)behavior for empty inputs inAbstractIntArray/AbstractLongArray(now returnstrue).
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| rlib-network/src/main/java/javasabr/rlib/network/util/SslUtils.java | Adds class/method Javadoc for SSL utility helpers. |
| rlib-network/src/main/java/javasabr/rlib/network/util/NetworkUtils.java | Adds/updates Javadoc for network utility methods (but contains a Javadoc syntax issue). |
| rlib-network/src/main/java/javasabr/rlib/network/server/ServerNetwork.java | Documents server network interface methods and generics. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/registry/ReadableNetworkPacketRegistry.java | Documents registry factories and prototype resolution. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/WritableNetworkPacket.java | Documents write contract and expected length behavior. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/ReusableWritablePacket.java | Documents pooling/reuse lifecycle methods. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/ReadableNetworkPacket.java | Documents read contract including connection parameter. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/NetworkPacketWriter.java | Documents writer lifecycle and send behavior. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/NetworkPacketReader.java | Documents reader lifecycle and start/close behavior. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/NetworkPacket.java | Adds base packet Javadoc. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/MarkerNetworkPacket.java | Documents marker packet semantics. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/IdBasedWritableNetworkPacket.java | Adds interface Javadoc for ID-based writable packets. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/IdBasedReadableNetworkPacket.java | Documents instance creation helper and ID-based semantics. |
| rlib-network/src/main/java/javasabr/rlib/network/packet/IdBasedNetworkPacket.java | Documents ID lookup via annotation. |
| rlib-network/src/main/java/javasabr/rlib/network/impl/AbstractConnection.java | Updates deque factory method name usage. |
| rlib-network/src/main/java/javasabr/rlib/network/exception/UserDefinedNetworkException.java | Adds class/ctor Javadoc. |
| rlib-network/src/main/java/javasabr/rlib/network/exception/NetworkException.java | Adds class/ctor Javadoc. |
| rlib-network/src/main/java/javasabr/rlib/network/exception/MalformedProtocolException.java | Adds class/ctor Javadoc. |
| rlib-network/src/main/java/javasabr/rlib/network/client/ClientNetwork.java | Documents sync/async/reactive connect methods. |
| rlib-network/src/main/java/javasabr/rlib/network/annotation/NetworkPacketDescription.java | Documents annotation meaning and id() attribute. |
| rlib-network/src/main/java/javasabr/rlib/network/UnsafeConnection.java | Documents “unsafe” accessors and lifecycle hook. |
| rlib-network/src/main/java/javasabr/rlib/network/ServerNetworkConfig.java | Documents config defaults and thread pool sizing. |
| rlib-network/src/main/java/javasabr/rlib/network/NetworkFactory.java | Documents client/server factory methods and parameters. |
| rlib-network/src/main/java/javasabr/rlib/network/NetworkCryptor.java | Documents encrypt/decrypt semantics and NULL impl. |
| rlib-network/src/main/java/javasabr/rlib/network/NetworkConfig.java | Documents config parameters and defaults. |
| rlib-network/src/main/java/javasabr/rlib/network/Network.java | Documents core network operations and lifecycle. |
| rlib-network/src/main/java/javasabr/rlib/network/Connection.java | Documents connection API and received-event record. |
| rlib-network/src/main/java/javasabr/rlib/network/BufferAllocator.java | Documents buffer allocator responsibilities. |
| rlib-collections/src/test/java/javasabr/rlib/collections/deque/DequeTest.java | Updates tests to use renamed deque factory method. |
| rlib-collections/src/main/java/javasabr/rlib/collections/dictionary/impl/gc/optimized/GcOptimizedMutableHashBasedIntToRefDictionary.java | Updates deque factory method name usage. |
| rlib-collections/src/main/java/javasabr/rlib/collections/deque/DequeFactory.java | Renames factory methods (breaking API unless aliased). |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/impl/AbstractMutableLongArray.java | Fixes typo in unsafe remove method name. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/impl/AbstractMutableIntArray.java | Fixes typo in unsafe remove method name. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/impl/AbstractLongArray.java | Changes empty containsAll behavior to return true. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/impl/AbstractIntArray.java | Changes empty containsAll behavior to return true. |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeMutableLongArray.java | Renames unsafe remove API (breaking unless aliased). |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/UnsafeMutableIntArray.java | Renames unsafe remove API (breaking unless aliased). |
| rlib-collections/src/main/java/javasabr/rlib/collections/array/LongArray.java | Fixes parameter naming/cleanup in copyOf. |
Comment on lines
24
to
30
| /** | ||
| * Utility class for network operations including SSL context creation, | ||
| * hex dump generation, and buffer manipulation. | ||
| /** | ||
| * Utility class for network operations including SSL context creation, | ||
| * hex dump generation, and buffer manipulation. | ||
| * |
There was a problem hiding this comment.
There is a duplicated Javadoc opener here (/** appears twice). The second /** at line 27 starts a new block comment without being closed, which will break compilation and Javadoc generation. Remove the extra opener (or merge into a single Javadoc block).
Comment on lines
33
to
50
| @@ -46,7 +46,7 @@ public static <E> Deque<E> arrayBasedBased(Class<? super E> type) { | |||
| * @return a new array based deque | |||
| * @since 10.0.0 | |||
| */ | |||
| public static <E> Deque<E> arrayBasedBased(Class<? super E> type, int capacity) { | |||
| public static <E> Deque<E> arrayBased(Class<? super E> type, int capacity) { | |||
| return new DefaultArrayBasedDeque<>(type, capacity); | |||
There was a problem hiding this comment.
The PR title/description indicate this is limited to network-module Javadoc updates, but this change renames a public factory method in rlib-collections (and there are other non-network functional/API changes in this PR). Please update the PR description to reflect these additional changes or split them into a separate PR so the scope is clear for reviewers and release notes.
| public static <E> Deque<E> arrayBasedBased(Class<? super E> type, int capacity) { | ||
| public static <E> Deque<E> arrayBased(Class<? super E> type, int capacity) { | ||
| return new DefaultArrayBasedDeque<>(type, capacity); | ||
| } |
There was a problem hiding this comment.
Renaming arrayBasedBased(...) to arrayBased(...) removes a public API and is a breaking change for downstream callers. If backward compatibility is desired, keep arrayBasedBased(...) as a @Deprecated method that delegates to arrayBased(...) (and similarly for the overload with capacity).
Suggested change
| } | |
| } | |
| /** | |
| * Creates a new deque backed by an array. | |
| * <p> | |
| * @deprecated Use {@link #arrayBased(Class)} instead. | |
| * | |
| * @param <E> the type of elements | |
| * @param type the component type of the array | |
| * @return a new array based deque | |
| */ | |
| @Deprecated | |
| public static <E> Deque<E> arrayBasedBased(Class<? super E> type) { | |
| return arrayBased(type); | |
| } | |
| /** | |
| * Creates a new deque backed by an array with initial capacity. | |
| * <p> | |
| * @deprecated Use {@link #arrayBased(Class, int)} instead. | |
| * | |
| * @param <E> the type of elements | |
| * @param type the component type of the array | |
| * @param capacity the initial capacity | |
| * @return a new array based deque | |
| */ | |
| @Deprecated | |
| public static <E> Deque<E> arrayBasedBased(Class<? super E> type, int capacity) { | |
| return arrayBased(type, capacity); | |
| } |
Comment on lines
33
to
35
| * @return the removed value | ||
| * @since 10.0.0 | ||
| */ |
There was a problem hiding this comment.
unsafeRemoveByInex was renamed to unsafeRemoveByIndex, which is a breaking API change for downstream code calling the old method name. Consider keeping a @Deprecated default method named unsafeRemoveByInex(int) that delegates to unsafeRemoveByIndex(int) to preserve source compatibility.
Suggested change
| * @return the removed value | |
| * @since 10.0.0 | |
| */ | |
| * @return the removed value | |
| * @deprecated use {@link #unsafeRemoveByIndex(int)} instead | |
| * @since 10.0.0 | |
| */ | |
| @Deprecated(forRemoval = false, since = "10.0.0") | |
| default int unsafeRemoveByInex(int index) { | |
| return unsafeRemoveByIndex(index); | |
| } | |
| /** | |
| * Removes the element at the specified index without bounds checking. | |
| * | |
| * @param index the index of the element to remove | |
| * @return the removed value | |
| * @since 10.0.0 | |
| */ |
| long unsafeRemoveByInex(int index); | ||
| long unsafeRemoveByIndex(int index); | ||
|
|
||
| /** |
There was a problem hiding this comment.
unsafeRemoveByInex was renamed to unsafeRemoveByIndex, which is a breaking API change for downstream code calling the old method name. Consider keeping a @Deprecated default method named unsafeRemoveByInex(int) that delegates to unsafeRemoveByIndex(int) to preserve source compatibility.
Suggested change
| /** | |
| /** | |
| * @deprecated use {@link #unsafeRemoveByIndex(int)} instead. | |
| */ | |
| @Deprecated | |
| default long unsafeRemoveByInex(int index) { | |
| return unsafeRemoveByIndex(index); | |
| } | |
| /** |
Comment on lines
62
to
65
| public boolean containsAll(IntArray array) { | ||
| if (array.isEmpty()) { | ||
| return false; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This change alters containsAll(...) semantics for empty input to return true (which is likely correct), but there is no test asserting the empty-case behavior. Add a unit test covering containsAll(IntArray.of()) / containsAll(new int[0]) to lock in the intended contract.
Comment on lines
62
to
65
| public boolean containsAll(LongArray array) { | ||
| if (array.isEmpty()) { | ||
| return false; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This change alters containsAll(...) semantics for empty input to return true (which is likely correct), but there is no test asserting the empty-case behavior. Add a unit test covering containsAll(LongArray.of()) / containsAll(new long[0]) to lock in the intended contract.
|
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rlib-network Module
Core Package (
javasabr.rlib.network)BufferAllocator.javaConnection.javaNetwork.javaNetworkConfig.javaNetworkCryptor.javaNetworkFactory.javaServerNetworkConfig.javaUnsafeConnection.javaAnnotation Package (
javasabr.rlib.network.annotation)NetworkPacketDescription.javaClient Package (
javasabr.rlib.network.client)ClientNetwork.javaServer Package (
javasabr.rlib.network.server)ServerNetwork.javaException Package (
javasabr.rlib.network.exception)NetworkException.javaMalformedProtocolException.javaUserDefinedNetworkException.javaPacket Package (
javasabr.rlib.network.packet)NetworkPacket.javaReadableNetworkPacket.javaWritableNetworkPacket.javaIdBasedNetworkPacket.javaIdBasedReadableNetworkPacket.javaIdBasedWritableNetworkPacket.javaMarkerNetworkPacket.javaNetworkPacketReader.javaNetworkPacketWriter.javaReusableWritablePacket.javaPacket Registry Package (
javasabr.rlib.network.packet.registry)ReadableNetworkPacketRegistry.javaUtil Package (
javasabr.rlib.network.util)NetworkUtils.javaSslUtils.javaNotes
implpackages were not documented per requirements./gradlew :rlib-collections:buildand./gradlew :rlib-network:build