Start adding some functionality to Mask #158

kashike · 2016-11-07T00:16:19Z

codecov-io · 2016-11-07T03:59:41Z

Codecov Report

Merging #158 into next will increase coverage by 0.86%.
The diff coverage is 86.36%.

@@             Coverage Diff              @@
##               next     #158      +/-   ##
============================================
+ Coverage     32.58%   33.44%   +0.86%     
- Complexity      465      485      +20     
============================================
  Files           192      192              
  Lines          4125     4180      +55     
  Branches        445      449       +4     
============================================
+ Hits           1344     1398      +54     
  Misses         2726     2726              
- Partials         55       56       +1

Impacted Files	Coverage Δ	Complexity Δ
...org/kitteh/irc/client/library/util/StringUtil.java	`88.88% <100%> (+4.51%)`	`20 <5> (+5)`	⬆️
...c/client/library/implementation/ActorProvider.java	`8.55% <100%> (-0.27%)`	`5 <2> (ø)`
.../java/org/kitteh/irc/client/library/util/Mask.java	`82.35% <82.35%> (+82.35%)`	`15 <15> (+15)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2500b91...ca95af5. Read the comment docs.

mbax · 2016-11-07T04:03:17Z

src/main/java/org/kitteh/irc/client/library/util/Mask.java

@@ -31,6 +31,7 @@
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Objects;
+import java.util.Optional;


lol768 · 2016-11-07T19:25:57Z

src/main/java/org/kitteh/irc/client/library/util/Mask.java

+     * @return collection of users that match this mask
+     */
+    @Nonnull
+    public Collection<User> getMatches(@Nonnull Channel channel) {


Are duplicates possible? Why not return a Set?

lol768 · 2016-11-07T19:26:43Z

There's one coverage miss introduced by this... is it feasible to cover it?

lol768

We need to be careful about how we present these methods and make it clear they aren't equivalent to the authoritative answer as given with TESTMASK or similar.

lol768 · 2016-11-07T19:28:46Z

src/main/java/org/kitteh/irc/client/library/util/Mask.java

+    }
+
+    /**
+     * Gets a collection of users that match this mask in the provided channel.


You should note about the shortcomings of this method and how it's not the same as the set of users the IRCd would ban for a +b set with the exact same mask.

Zarthus · 2016-11-07T19:34:22Z

src/test/java/org/kitteh/irc/client/library/util/MaskTest.java

+        final Channel channel = Mockito.mock(Channel.class);
+        final User mbaxter = new TestUser("mbaxter", "~mbax", "kitten.institute", null);
+        final User kashike = new TestUser("kashike", "kashike", "is.a.miserable.ninja", null);
+        final User lol768 = new TestUser("lol768", "lol768", "catastrophe.esper.net", null);


ideally use a host different than *.esper.net, I'm fine with using lynvie.com for mine or just somethinf spoofed.

Zarthus · 2016-11-07T19:35:06Z

src/test/java/org/kitteh/irc/client/library/util/MaskTest.java

+/**
+ * It's a mask test.
+ */
+public class MaskTest {


add a test for spoofed masks? freenode/staff/potato comes to mind and lol (invalid hostname, valid spoof/mask), or catbot is catbot@�4,1nyan�7nyan�8nyan�3nyan�11nyan�10nyan�12nyan�13nyan� * nyan catbot (colors in hostnames, yay quakenet)

also a case insensitivity test (Zarthus@potato == zarThus@potAto)

also wildcards (? and *, I see some for * - none for ?)

and CIDR-style ban support would be nice as well

and CIDR-style ban support would be nice as well

Would need to be restricted to IP based hostmasks.

Zarthus · 2016-11-08T20:12:45Z

src/main/java/org/kitteh/irc/client/library/util/Mask.java

+    // Let's just do it assuming no IRCD can handle following the rules.
+    // New pattern: ([^!@]+)!([^!@]+)@([^!@]+)
+    public static final Pattern NICK_PATTERN = Pattern.compile("([^!@]+)!([^!@]+)@([^!@]+)");
+    public static final char WILDCARD_CHARACTER = '*';


What about ?

Zarthus · 2016-11-08T20:15:48Z

src/main/java/org/kitteh/irc/client/library/util/StringUtil.java

+            switch (character) {
+                case '*':
+                case '?':
+                    builder.append('.').append(character); // * to .* and ? to .?


That's not how a wildcard-? works, a wildcard-? translates to regex ., a wildcard-* translates to regex .*

Regex .? would be "zero or one of any", When in reality wildcard-? matches exactly one of any.

Case in point:

+b ????*!*@*

Bans all four-or-more letter nicks.

With your regex, this would ban all 0 or more letter nicks.

Zarthus · 2016-11-08T20:30:25Z

src/main/java/org/kitteh/irc/client/library/util/StringUtil.java

+        }
+        builder.insert(0, '^');
+        builder.append('$');
+        return Pattern.compile(builder.toString());


Should include the case insensitivity flag Pattern.CASE_INSENSITIVE

mbax · 2016-11-21T01:50:01Z

src/main/java/org/kitteh/irc/client/library/util/Mask.java

-    private Mask(@Nonnull String string) {
-        this.string = string;
+    /**
+     * Creates a Mask from the given user.


Perhaps "given user string"

mbax · 2016-11-21T01:51:38Z

src/main/java/org/kitteh/irc/client/library/util/Mask.java

+     * @return true if user matches this mask
+     */
+    @Override
+    public boolean test(User user) {


@Nonnull User user

Bippity boppity boop

kashike · 2017-10-08T19:50:28Z

Superseded by #207.

kashike force-pushed the feature/mask branch from f07e49a to a8e12d7 Compare November 7, 2016 00:17

mbax added enhancement [MINOR] new API / feature work in progress labels Nov 7, 2016

mbax added this to the 3.0.0 milestone Nov 7, 2016

mbax reviewed Nov 7, 2016

View reviewed changes

lol768 reviewed Nov 7, 2016

View reviewed changes

lol768 suggested changes Nov 7, 2016

View reviewed changes

Zarthus reviewed Nov 7, 2016

View reviewed changes

Zarthus reviewed Nov 8, 2016

View reviewed changes

kashike force-pushed the feature/mask branch from 2f2237f to 63669e6 Compare November 20, 2016 07:53

mbax reviewed Nov 21, 2016

View reviewed changes

mbax force-pushed the next branch from d9b8d70 to d1839e2 Compare December 7, 2016 01:47

mbax modified the milestones: 3.0.0, 3.1.0 Dec 29, 2016

kashike force-pushed the feature/mask branch 3 times, most recently from 353b3f0 to 804ee4f Compare January 25, 2017 10:07

mbax modified the milestones: 3.1.0, Future! Feb 9, 2017

mbax force-pushed the next branch from 06a04ce to 62bf9c6 Compare February 20, 2017 20:58

mbax force-pushed the next branch from 62bf9c6 to bf8aa4a Compare March 1, 2017 01:24

mbax force-pushed the next branch from bf8aa4a to 20fab99 Compare April 16, 2017 23:35

mbax and others added 2 commits June 3, 2017 20:40

Add StringUtil#wildcardToPattern

435d312

Add functionality to Mask

ca95af5

Bippity boppity boop

kashike force-pushed the feature/mask branch from 804ee4f to ca95af5 Compare June 4, 2017 03:41

mbax force-pushed the next branch from 4023212 to 84d14b8 Compare August 9, 2017 03:15

mbax force-pushed the next branch from 84d14b8 to 1075e4b Compare August 21, 2017 15:15

mbax added baxtoberfest and removed baxtoberfest labels Oct 8, 2017

kashike mentioned this pull request Oct 8, 2017

I'm the master of disguise #207

Closed

kashike closed this Oct 8, 2017

mbax removed this from the Future! milestone Jan 1, 2018

mbax removed the work in progress label Jan 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start adding some functionality to Mask #158

Start adding some functionality to Mask #158

kashike commented Nov 7, 2016

codecov-io commented Nov 7, 2016 •

edited

Loading

mbax Nov 7, 2016

lol768 Nov 7, 2016

lol768 commented Nov 7, 2016

lol768 left a comment

lol768 Nov 7, 2016

Zarthus Nov 7, 2016

Zarthus Nov 7, 2016 •

edited

Loading

Zarthus Nov 7, 2016

Zarthus Nov 7, 2016 •

edited

Loading

Zarthus Nov 7, 2016

lol768 Nov 9, 2016

Zarthus Nov 8, 2016

Zarthus Nov 8, 2016 •

edited

Loading

Zarthus Nov 8, 2016

mbax Nov 21, 2016

mbax Nov 21, 2016

kashike commented Oct 8, 2017

Start adding some functionality to Mask #158

Start adding some functionality to Mask #158

Conversation

kashike commented Nov 7, 2016

codecov-io commented Nov 7, 2016 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lol768 commented Nov 7, 2016

lol768 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zarthus Nov 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zarthus Nov 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zarthus Nov 8, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kashike commented Oct 8, 2017

codecov-io commented Nov 7, 2016 •

edited

Loading

Zarthus Nov 7, 2016 •

edited

Loading

Zarthus Nov 7, 2016 •

edited

Loading

Zarthus Nov 8, 2016 •

edited

Loading