Skip to content

Commit a456895

Browse files
committed
fix(users): not overriding fields if user is found in cache + comments
1 parent 6ee9c7d commit a456895

File tree

2 files changed

+162
-106
lines changed

2 files changed

+162
-106
lines changed

src/main/java/me/itzg/helpers/users/ManageUsersCommand.java

Lines changed: 103 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -50,26 +50,25 @@ public class ManageUsersCommand implements Callable<Integer> {
5050
private static final ComparableVersion MIN_VERSION_USES_JSON = new ComparableVersion("1.7.3");
5151

5252
@SuppressWarnings("unused")
53-
@Option(names = {"--help", "-h"}, usageHelp = true)
53+
@Option(names = { "--help", "-h" }, usageHelp = true)
5454
boolean help;
5555

56-
@Option(names = {"--offline"}, required = false, description = "Use for offline server, UUIDs are generated")
56+
@Option(names = { "--offline" }, required = false, description = "Use for offline server, UUIDs are generated")
5757
boolean offline;
5858

5959
@Option(names = "--output-directory", defaultValue = ".")
6060
Path outputDirectory;
6161

62-
@Option(names = {"--type", "-t"}, required = true, description = "Allowed: ${COMPLETION-CANDIDATES}")
62+
@Option(names = { "--type", "-t" }, required = true, description = "Allowed: ${COMPLETION-CANDIDATES}")
6363
Type type;
6464

65-
@Option(names = "--existing", defaultValue = "SYNCHRONIZE",
66-
description = "Select the behavior when the resulting file already exists\nAllowed: ${COMPLETION-CANDIDATES}")
65+
@Option(names = "--existing", defaultValue = "SYNCHRONIZE", description = "Select the behavior when the resulting file already exists\nAllowed: ${COMPLETION-CANDIDATES}")
6766
ExistingFileBehavior existingFileBehavior;
6867

6968
@Option(names = "--version", description = "Minecraft game version. If not provided, assumes JSON format")
7069
String version;
7170

72-
@Option(names = {"-f", "--input-is-file"})
71+
@Option(names = { "-f", "--input-is-file" })
7372
boolean inputIsFile;
7473

7574
@Option(names = "--mojang-api-base-url", defaultValue = "${env:MOJANG_API_BASE_URL:-https://api.mojang.com}")
@@ -78,17 +77,15 @@ public class ManageUsersCommand implements Callable<Integer> {
7877
@Option(names = "--playerdb-api-base-url", defaultValue = "${env:PLAYERDB_API_BASE_URL:-https://playerdb.co}")
7978
String playerdbApiBaseUrl;
8079

81-
@Option(names = "--user-api-provider", defaultValue = "${env:USER_API_PROVIDER:-playerdb}",
82-
description = "Allowed: ${COMPLETION-CANDIDATES}"
83-
)
80+
@Option(names = "--user-api-provider", defaultValue = "${env:USER_API_PROVIDER:-playerdb}", description = "Allowed: ${COMPLETION-CANDIDATES}")
8481
UserApiProvider userApiProvider;
8582

8683
@ArgGroup(exclusive = false)
8784
SharedFetchArgs sharedFetchArgs = new SharedFetchArgs();
8885

8986
@Parameters(split = ",", paramLabel = "INPUT", description = "One or more Mojang usernames, UUID, or ID (UUID without dashes);"
90-
+ " however, when offline, only UUID/IDs can be provided."
91-
+ "%nWhen input is a file, only one local file path or URL can be provided")
87+
+ " however, when offline, only UUID/IDs can be provided."
88+
+ "%nWhen input is a file, only one local file path or URL can be provided")
9289
List<String> inputs = Collections.emptyList();
9390

9491
final ObjectMapper objectMapper = ObjectMappers.defaultMapper();
@@ -106,8 +103,7 @@ public Integer call() throws Exception {
106103
}
107104

108105
processInputAsFile(sharedFetch, inputs.get(0));
109-
}
110-
else {
106+
} else {
111107
processJavaUserIdList(sharedFetch, inputs);
112108
}
113109

@@ -122,8 +118,7 @@ private void processJavaUserIdList(SharedFetch sharedFetch, List<String> inputs)
122118
verifyNotUuids(userDefs);
123119

124120
final Path resultFile = outputDirectory.resolve(
125-
type == Type.JAVA_OPS ? "ops.txt" : "white-list.txt"
126-
);
121+
type == Type.JAVA_OPS ? "ops.txt" : "white-list.txt");
127122

128123
if (handleSkipExistingFile(resultFile)) {
129124
return;
@@ -136,40 +131,36 @@ private void processJavaUserIdList(SharedFetch sharedFetch, List<String> inputs)
136131

137132
log.debug("Writing users list to {}: {}", resultFile, users);
138133
Files.write(resultFile, users);
139-
}
140-
else {
134+
} else {
141135
final Path resultFile = outputDirectory.resolve(
142-
type == Type.JAVA_OPS ? "ops.json" : "whitelist.json"
143-
);
136+
type == Type.JAVA_OPS ? "ops.json" : "whitelist.json");
144137

145138
if (handleSkipExistingFile(resultFile)) {
146139
return;
147140
}
148141

149142
objectMapper.writeValue(resultFile.toFile(),
150-
reconcile(sharedFetch, userDefs,
151-
loadExistingJavaJson(resultFile)
152-
)
153-
);
143+
reconcile(sharedFetch, userDefs,
144+
loadExistingJavaJson(resultFile)));
154145
}
155146
}
156147

157148
private boolean handleSkipExistingFile(Path resultFile) {
158149
if (existingFileBehavior == ExistingFileBehavior.SKIP
159-
&& Files.exists(resultFile)) {
150+
&& Files.exists(resultFile)) {
160151
log.info("The file {} already exists, so no changes will be made", resultFile);
161152
return true;
162153
}
163154
return false;
164155
}
165156

166-
private List<? extends JavaUser> reconcile(SharedFetch sharedFetch, List<UserDef> userDefs, List<? extends JavaUser> existing) {
157+
private List<? extends JavaUser> reconcile(SharedFetch sharedFetch, List<UserDef> userDefs,
158+
List<? extends JavaUser> existing) {
167159

168160
final List<JavaUser> reconciled;
169161
if (existingFileBehavior == ExistingFileBehavior.MERGE) {
170162
reconciled = new ArrayList<>(existing);
171-
}
172-
else {
163+
} else {
173164
reconciled = new ArrayList<>(inputs.size());
174165
}
175166

@@ -181,17 +172,16 @@ private List<? extends JavaUser> reconcile(SharedFetch sharedFetch, List<UserDef
181172
if (type == Type.JAVA_OPS) {
182173
final JavaOp resolvedOp = resolvedUser instanceof JavaOp ? ((JavaOp) resolvedUser) : null;
183174
reconciled.add(JavaOp.builder()
184-
.name(resolvedUser.getName())
185-
.uuid(resolvedUser.getUuid())
186-
.level(resolvedOp != null ? resolvedOp.getLevel() : 4)
187-
.bypassesPlayerLimit(resolvedOp != null && resolvedOp.isBypassesPlayerLimit())
188-
.build());
189-
}
190-
else {
175+
.name(resolvedUser.getName())
176+
.uuid(resolvedUser.getUuid())
177+
.level(resolvedOp != null ? resolvedOp.getLevel() : 4)
178+
.bypassesPlayerLimit(resolvedOp != null && resolvedOp.isBypassesPlayerLimit())
179+
.build());
180+
} else {
191181
reconciled.add(JavaUser.builder()
192-
.name(resolvedUser.getName())
193-
.uuid(resolvedUser.getUuid())
194-
.build());
182+
.name(resolvedUser.getName())
183+
.uuid(resolvedUser.getUuid())
184+
.build());
195185
}
196186
}
197187
}
@@ -211,74 +201,86 @@ private boolean containsUserByUuid(List<JavaUser> users, String uuid) {
211201
private JavaUser resolveJavaUserId(SharedFetch sharedFetch, List<? extends JavaUser> existing, UserDef user) {
212202

213203
return UuidQuirks.ifIdOrUuid(user.getName())
214-
.map(uuid -> {
215-
for (final JavaUser existingUser : existing) {
216-
if (existingUser.getUuid().equalsIgnoreCase(uuid)) {
217-
log.debug("Resolved '{}' from existing user entry by UUID: {}", user.getName(), existingUser);
218-
return existingUser;
204+
.map(uuid -> {
205+
for (final JavaUser existingUser : existing) {
206+
if (existingUser.getUuid().equalsIgnoreCase(uuid)) {
207+
log.debug("Resolved '{}' from existing user entry by UUID: {}", user.getName(),
208+
existingUser);
209+
return existingUser;
210+
}
219211
}
220-
}
221212

222-
log.debug("Resolved '{}' into new user entry", user.getName());
223-
return JavaUser.builder()
224-
.uuid(uuid)
225-
// username needs to be present, but content doesn't matter
226-
.name("")
227-
.build();
228-
229-
})
230-
.orElseGet(() -> {
231-
JavaUser finalUser = null;
232-
// ...or username
233-
for (final JavaUser existingUser : existing) {
234-
if (existingUser.getName().equalsIgnoreCase(user.getName())) {
235-
log.debug("Resolved '{}' from existing user entry by name: {}", user.getName(), existingUser);
236-
finalUser = existingUser;
213+
log.debug("Resolved '{}' into new user entry", user.getName());
214+
return JavaUser.builder()
215+
.uuid(uuid)
216+
// username needs to be present, but content doesn't matter
217+
.name("")
218+
.build();
219+
220+
})
221+
.orElseGet(() -> {
222+
// User to be used
223+
JavaUser finalUser = null;
224+
225+
// Try to find user in existing users list
226+
for (final JavaUser existingUser : existing) {
227+
if (existingUser.getName().equalsIgnoreCase(user.getName())) {
228+
log.debug("Resolved '{}' from existing user entry by name: {}", user.getName(),
229+
existingUser);
230+
finalUser = existingUser;
231+
}
237232
}
238-
}
239233

240-
if (offline && user.getFlags().contains("offline")) {
241-
log.debug("Resolved '{}' as offline user", user.getName());
234+
// If existing user is not found, build a new one
242235
if (finalUser == null) {
243236
finalUser = JavaUser.builder().name(user.getName()).build();
244237
}
245-
return finalUser.setUuid(getOfflineUUID(user.getName()));
246-
}
247238

248-
final Path userCacheFile = outputDirectory.resolve("usercache.json");
249-
if (Files.exists(userCacheFile)) {
250-
try {
251-
final List<JavaUser> userCache = objectMapper.readValue(userCacheFile.toFile(), LIST_OF_JAVA_USER);
252-
for (final JavaUser existingUser : userCache) {
253-
if (existingUser.getName().equalsIgnoreCase(user.getName())) {
254-
log.debug("Resolved '{}' from user cache by name: {}", user.getName(), existingUser);
255-
return existingUser;
239+
// User is not online, generating offline UUID
240+
if (offline && user.getFlags().contains("offline")) {
241+
log.debug("Resolved '{}' as offline user", user.getName());
242+
// update UUID keeping the other fields in case of existing user
243+
return finalUser.setUuid(getOfflineUUID(user.getName()));
244+
}
245+
246+
final Path userCacheFile = outputDirectory.resolve("usercache.json");
247+
if (Files.exists(userCacheFile)) {
248+
try {
249+
final List<JavaUser> userCache = objectMapper.readValue(userCacheFile.toFile(),
250+
LIST_OF_JAVA_USER);
251+
for (final JavaUser existingUser : userCache) {
252+
if (existingUser.getName().equalsIgnoreCase(user.getName())) {
253+
log.debug("Resolved '{}' from user cache by name: {}", user.getName(),
254+
existingUser);
255+
// UUID from usercache.jsona are safe to use regardless of the user type
256+
// if a UUID is present here, user joined successfully with that UUID
257+
return finalUser.setUuid(existingUser.getUuid());
258+
}
256259
}
260+
} catch (IOException e) {
261+
log.error("Failed to parse usercache.json", e);
257262
}
258-
} catch (IOException e) {
259-
log.error("Failed to parse usercache.json", e);
260263
}
261-
}
262264

263-
final UserApi userApi;
264-
switch (userApiProvider) {
265-
case mojang:
266-
userApi = new MojangUserApi(sharedFetch, mojangApiBaseUrl);
267-
break;
268-
case playerdb:
269-
userApi = new PlayerdbUserApi(sharedFetch, playerdbApiBaseUrl);
270-
break;
271-
default:
272-
throw new GenericException("User API provider was not specified");
273-
}
274-
JavaUser apiUser = userApi.resolveUser(user.getName());
265+
final UserApi userApi;
266+
switch (userApiProvider) {
267+
case mojang:
268+
userApi = new MojangUserApi(sharedFetch, mojangApiBaseUrl);
269+
break;
270+
case playerdb:
271+
userApi = new PlayerdbUserApi(sharedFetch, playerdbApiBaseUrl);
272+
break;
273+
default:
274+
throw new GenericException("User API provider was not specified");
275+
}
276+
JavaUser apiUser = userApi.resolveUser(user.getName());
275277

276-
if (finalUser != null) {
277-
return finalUser.setUuid(apiUser.getUuid());
278-
}else{
279-
return apiUser;
280-
}
281-
});
278+
if (finalUser != null) {
279+
return finalUser.setUuid(apiUser.getUuid());
280+
} else {
281+
return apiUser;
282+
}
283+
});
282284

283285
}
284286

@@ -289,8 +291,7 @@ private List<? extends JavaUser> loadExistingJavaJson(Path userFile) throws IOEx
289291

290292
if (type == Type.JAVA_OPS) {
291293
return objectMapper.readValue(userFile.toFile(), LIST_OF_JAVA_OP);
292-
}
293-
else {
294+
} else {
294295
return objectMapper.readValue(userFile.toFile(), LIST_OF_JAVA_USER);
295296
}
296297
}
@@ -317,10 +318,8 @@ private void verifyNotUuids(List<UserDef> userDefs) {
317318

318319
private void processInputAsFile(SharedFetch sharedFetch, String filePathUrl) throws IOException {
319320
final Path outputFile = outputDirectory.resolve(
320-
usesTextUserList() ?
321-
(type == Type.JAVA_OPS ? "ops.txt" : "whitelist.txt")
322-
: type == Type.JAVA_OPS ? "ops.json" : "whitelist.json"
323-
);
321+
usesTextUserList() ? (type == Type.JAVA_OPS ? "ops.txt" : "whitelist.txt")
322+
: type == Type.JAVA_OPS ? "ops.json" : "whitelist.json");
324323

325324
if (handleSkipExistingFile(outputFile)) {
326325
return;
@@ -331,22 +330,20 @@ private void processInputAsFile(SharedFetch sharedFetch, String filePathUrl) thr
331330

332331
try {
333332
sharedFetch.fetch(new URI(filePathUrl))
334-
.toFile(outputFile)
335-
.execute();
333+
.toFile(outputFile)
334+
.execute();
336335
} catch (URISyntaxException e) {
337336
throw new InvalidParameterException("Given input is not a fully valid URL", e);
338337
}
339-
}
340-
else {
338+
} else {
341339
final Path inputFile = Paths.get(filePathUrl);
342340

343341
if (!Files.exists(outputFile) || !Files.isSameFile(inputFile, outputFile)) {
344342
log.debug("Copying from {} to {}", filePathUrl, outputFile);
345343
Files.copy(inputFile, outputFile, StandardCopyOption.REPLACE_EXISTING);
346-
}
347-
else {
344+
} else {
348345
log.warn("Attempting to synchronize input {} file onto itself: {}",
349-
type, outputFile);
346+
type, outputFile);
350347
}
351348
}
352349
}
@@ -356,7 +353,7 @@ private boolean usesTextUserList() {
356353
}
357354

358355
private static String getOfflineUUID(String username) {
359-
byte[] bytes = DigestUtils.md5("OfflinePlayer:"+username);
356+
byte[] bytes = DigestUtils.md5("OfflinePlayer:" + username);
360357

361358
// Force version = 3 (bits 12-15 of time_hi_and_version)
362359
bytes[6] &= 0x0F;

0 commit comments

Comments
 (0)