Add JavaDoc for IpFilter - #136
Conversation
📝 WalkthroughWalkthroughThe PR enhances the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/juv25d/filter/IpFilter.java`:
- Around line 37-45: The Javadoc comments for the IpFilter constructors and
isAllowed method are detached by stray blank lines; remove the empty line
immediately following each closing */ so the Javadoc block directly precedes the
corresponding declaration (apply to the parameterized constructor
IpFilter(Set<String> whitelist, Set<String> blacklist, boolean allowByDefault),
the no-arg constructor IpFilter(), and the isAllowed(...) method) to ensure
javadoc associates the comments with those members.
- Around line 19-25: Update the JavaDoc in class IpFilter to reflect actual
precedence used in the isAllowed method: list the "present in both lists" rule
before the individual whitelist/blacklist rules so readers see that the
both-case is evaluated first and falls back to allowByDefault; reference the
isAllowed method and the allowByDefault flag when rewording the bullets to
ensure the Javadoc matches code behavior.
| * <p>Decision rules:</p> | ||
| * <ul> | ||
| * <li>If an IP is present in the whitelist, it is allowed.</li> | ||
| * <li>If an IP is present in the blacklist, it is denied.</li> | ||
| * <li>If an IP is present in both lists, {@code allowByDefault} is used.</li> | ||
| * <li>If an IP is present in neither list, {@code allowByDefault} is used.</li> | ||
| * </ul> |
There was a problem hiding this comment.
JavaDoc rule ordering does not match actual precedence in isAllowed.
The list implies whitelist unconditionally takes priority (rule 1), but isAllowed checks "in both" before the pure-whitelist check (line 87 vs. line 89). A developer reading only the Javadoc would expect that a whitelisted IP is always allowed, which is incorrect when the IP also appears in the blacklist. The "both lists" case should be listed first to reflect actual code precedence.
📝 Proposed fix
* <p>Decision rules:</p>
* <ul>
- * <li>If an IP is present in the whitelist, it is allowed.</li>
- * <li>If an IP is present in the blacklist, it is denied.</li>
* <li>If an IP is present in both lists, {`@code` allowByDefault} is used.</li>
+ * <li>If an IP is present in the whitelist only, it is allowed.</li>
+ * <li>If an IP is present in the blacklist only, it is denied.</li>
* <li>If an IP is present in neither list, {`@code` allowByDefault} is used.</li>
* </ul>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * <p>Decision rules:</p> | |
| * <ul> | |
| * <li>If an IP is present in the whitelist, it is allowed.</li> | |
| * <li>If an IP is present in the blacklist, it is denied.</li> | |
| * <li>If an IP is present in both lists, {@code allowByDefault} is used.</li> | |
| * <li>If an IP is present in neither list, {@code allowByDefault} is used.</li> | |
| * </ul> | |
| * <p>Decision rules:</p> | |
| * <ul> | |
| * <li>If an IP is present in both lists, {`@code` allowByDefault} is used.</li> | |
| * <li>If an IP is present in the whitelist only, it is allowed.</li> | |
| * <li>If an IP is present in the blacklist only, it is denied.</li> | |
| * <li>If an IP is present in neither list, {`@code` allowByDefault} is used.</li> | |
| * </ul> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/juv25d/filter/IpFilter.java` around lines 19 - 25, Update
the JavaDoc in class IpFilter to reflect actual precedence used in the isAllowed
method: list the "present in both lists" rule before the individual
whitelist/blacklist rules so readers see that the both-case is evaluated first
and falls back to allowByDefault; reference the isAllowed method and the
allowByDefault flag when rewording the bullets to ensure the Javadoc matches
code behavior.
| /** | ||
| * Creates an {@code IpFilter} with explicit configuration. | ||
| * | ||
| * @param whitelist IP addresses that should be allowed (may be {@code null}) | ||
| * @param blacklist IP addresses that should be denied (may be {@code null}) | ||
| * @param allowByDefault fallback decision when an IP is not listed, or listed in both sets | ||
| */ | ||
|
|
||
| public IpFilter(Set<String> whitelist, Set<String> blacklist, boolean allowByDefault) { |
There was a problem hiding this comment.
Blank lines between Javadoc blocks and their declarations detach the docs from generated output.
Lines 44, 58, and 83 each introduce a blank line between the closing */ and the corresponding constructor/method declaration. The standard javadoc tool requires the comment to immediately precede the element it documents; a blank line causes the association to be silently dropped, so the generated HTML will show these members as undocumented.
📝 Proposed fix (shown for parameterized constructor; apply the same to the no-arg constructor and `isAllowed`)
* `@param` allowByDefault fallback decision when an IP is not listed, or listed in both sets
*/
-
public IpFilter(Set<String> whitelist, Set<String> blacklist, boolean allowByDefault) {Also applies to: 55-59, 77-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/juv25d/filter/IpFilter.java` around lines 37 - 45, The
Javadoc comments for the IpFilter constructors and isAllowed method are detached
by stray blank lines; remove the empty line immediately following each closing
*/ so the Javadoc block directly precedes the corresponding declaration (apply
to the parameterized constructor IpFilter(Set<String> whitelist, Set<String>
blacklist, boolean allowByDefault), the no-arg constructor IpFilter(), and the
isAllowed(...) method) to ensure javadoc associates the comments with those
members.
|
Ahaa good point i'll close this one 👍 |
2 participants
Adds JavaDoc describing the IP filter decision rules and configuration source.
No functional changes.
Summary by CodeRabbit