-
Notifications
You must be signed in to change notification settings - Fork 518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP - Lowercase cleanup #1706
base: master
Are you sure you want to change the base?
WIP - Lowercase cleanup #1706
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ public void increaseCount(String address, String name) { | |
if (isEnabled) { | ||
TimedCounter<String> countsByName = ipLoginFailureCounts.computeIfAbsent( | ||
address, k -> new TimedCounter<>(resetThreshold, TimeUnit.MINUTES)); | ||
countsByName.increment(name); | ||
countsByName.increment(name.toLowerCase()); | ||
} | ||
} | ||
|
||
|
@@ -66,7 +66,7 @@ public void resetCount(String address, String name) { | |
if (isEnabled) { | ||
TimedCounter<String> counter = ipLoginFailureCounts.get(address); | ||
if (counter != null) { | ||
counter.remove(name); | ||
counter.remove(name.toLowerCase()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great catch! |
||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ public class PlayerCache { | |
* @param auth the player auth object to save | ||
*/ | ||
public void updatePlayer(PlayerAuth auth) { | ||
cache.put(auth.getNickname().toLowerCase(), auth); | ||
cache.put(auth.getNickname(), auth); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure about this one? Aha, nickname is lowercase by definition. OK. |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,8 +83,7 @@ public boolean isAuthAvailable(String user) { | |
|
||
@Override | ||
public HashedPassword getPassword(String user) { | ||
user = user.toLowerCase(); | ||
Optional<PlayerAuth> pAuthOpt = cachedAuths.getIfPresent(user); | ||
Optional<PlayerAuth> pAuthOpt = cachedAuths.getIfPresent(user.toLowerCase()); | ||
if (pAuthOpt != null && pAuthOpt.isPresent()) { | ||
return pAuthOpt.get().getPassword(); | ||
} | ||
|
@@ -93,8 +92,7 @@ public HashedPassword getPassword(String user) { | |
|
||
@Override | ||
public PlayerAuth getAuth(String user) { | ||
user = user.toLowerCase(); | ||
return cachedAuths.getUnchecked(user).orElse(null); | ||
return cachedAuths.getUnchecked(user.toLowerCase()).orElse(null); | ||
} | ||
|
||
@Override | ||
|
@@ -117,10 +115,9 @@ public boolean updatePassword(PlayerAuth auth) { | |
|
||
@Override | ||
public boolean updatePassword(String user, HashedPassword password) { | ||
user = user.toLowerCase(); | ||
boolean result = source.updatePassword(user, password); | ||
if (result) { | ||
cachedAuths.refresh(user); | ||
cachedAuths.refresh(user.toLowerCase()); | ||
} | ||
return result; | ||
} | ||
|
@@ -150,10 +147,9 @@ public Set<String> getRecordsToPurge(long until) { | |
|
||
@Override | ||
public boolean removeAuth(String name) { | ||
name = name.toLowerCase(); | ||
boolean result = source.removeAuth(name); | ||
if (result) { | ||
cachedAuths.invalidate(name); | ||
cachedAuths.invalidate(name.toLowerCase()); | ||
} | ||
return result; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent! |
||
} | ||
|
@@ -207,12 +203,12 @@ public boolean isLogged(String user) { | |
|
||
@Override | ||
public void setLogged(final String user) { | ||
source.setLogged(user.toLowerCase()); | ||
source.setLogged(user); | ||
} | ||
|
||
@Override | ||
public void setUnlogged(final String user) { | ||
source.setUnlogged(user.toLowerCase()); | ||
source.setUnlogged(user); | ||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has bit of a funny feel now. It's absolutely 100% correct. I'm just wondering if it'd make sense to have a single
name(String)
function, since obviously from the realname we immediately know the all-lowercase name? This is maybe something we can look at after this pull request (for the sake of merging this soon). I expect that in some cases we do only want to set thename
only (without a realname), but in most cases it would be nice if we just had to pass the name once and could tell the PlayerAuth builder "hey you, do what you need to do" 😄Edit: So to be clear, my suggestion is to do nothing about it for now but we should keep it in the back of our heads.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, we can enhance it in a future cleanup