Skip to content

Commit d175a76

Browse files
dmulloy2dmulloy2
committed
Refactor package caching, account for some edge cases
1 parent 4afb38a commit d175a76

File tree

3 files changed

+53
-41
lines changed

3 files changed

+53
-41
lines changed

‎modules/API/src/main/java/com/comphenix/protocol/utility/CachedPackage.java‎

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
1-
/*
1+
/**
22
* ProtocolLib - Bukkit server library that allows access to the Minecraft protocol.
33
* Copyright (C) 2012 Kristian S. Stangeland
44
*
5-
* This program is free software; you can redistribute it and/or modify it under the terms of the
6-
* GNU General Public License as published by the Free Software Foundation; either version 2 of
5+
* This program is free software; you can redistribute it and/or modify it under the terms of the
6+
* GNU General Public License as published by the Free Software Foundation; either version 2 of
77
* the License, or (at your option) any later version.
88
*
9-
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
10-
* without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
9+
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
10+
* without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
1111
* See the GNU General Public License for more details.
1212
*
13-
* You should have received a copy of the GNU General Public License along with this program;
14-
* if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
13+
* You should have received a copy of the GNU General Public License along with this program;
14+
* if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
1515
* 02111-1307 USA
1616
*/
17-
1817
package com.comphenix.protocol.utility;
1918

2019
import java.util.Map;
@@ -33,7 +32,7 @@ class CachedPackage {
3332
private final Map<String, Optional<Class<?>>> cache;
3433
private final String packageName;
3534
private final ClassSource source;
36-
35+
3736
/**
3837
* Construct a new cached package.
3938
* @param packageName - the name of the current package.
@@ -44,45 +43,51 @@ public CachedPackage(String packageName, ClassSource source) {
4443
this.cache = Maps.newConcurrentMap();
4544
this.source = source;
4645
}
47-
46+
4847
/**
4948
* Associate a given class with a class name.
5049
* @param className - class name.
5150
* @param clazz - type of class.
5251
*/
5352
public void setPackageClass(String className, Class<?> clazz) {
54-
cache.put(className, Optional.<Class<?>>fromNullable(clazz));
53+
if (clazz != null) {
54+
cache.put(className, Optional.<Class<?>> of(clazz));
55+
} else {
56+
cache.remove(className);
57+
}
5558
}
56-
59+
5760
/**
5861
* Retrieve the class object of a specific class in the current package.
5962
* @param className - the specific class.
6063
* @return Class object.
6164
* @throws RuntimeException If we are unable to find the given class.
6265
*/
6366
public Class<?> getPackageClass(String className) {
64-
try {
65-
Optional<Class<?>> result = cache.get(Preconditions.checkNotNull(className, "className cannot be NULL"));
66-
67-
// Concurrency is not a problem - we don't care if we look up a class twice
68-
if (result == null) {
69-
// Look up the class dynamically
70-
result = Optional.<Class<?>>fromNullable(source.loadClass(combine(packageName, className)));
71-
if (!result.isPresent())
72-
throw new IllegalArgumentException("Source " + source + " returned NULL for " + className);
67+
Preconditions.checkNotNull(className, "className cannot be null!");
7368

74-
cache.put(className, result);
75-
}
76-
77-
// Class has been looked for and hasn't been found in the past
69+
// See if we've already looked it up
70+
if (cache.containsKey(className)) {
71+
Optional<Class<?>> result = cache.get(className);
7872
if (!result.isPresent()) {
79-
throw new ClassNotFoundException();
73+
throw new RuntimeException("Cannot find class " + className);
8074
}
8175

8276
return result.get();
83-
} catch (ClassNotFoundException e) {
84-
setPackageClass(className, null);
85-
throw new RuntimeException("Cannot find class " + combine(packageName, className), e);
77+
}
78+
79+
try {
80+
// Try looking it up
81+
Class<?> clazz = source.loadClass(combine(packageName, className));
82+
if (clazz == null) {
83+
throw new IllegalArgumentException("Source " + source + " returned null for " + className);
84+
}
85+
86+
cache.put(className, Optional.<Class<?>> of(clazz));
87+
return clazz;
88+
} catch (ClassNotFoundException ex) {
89+
cache.put(className, Optional.<Class<?>> absent());
90+
throw new RuntimeException("Cannot find class " + className, ex);
8691
}
8792
}
8893

‎modules/API/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@
6363
import com.comphenix.protocol.reflect.fuzzy.FuzzyFieldContract;
6464
import com.comphenix.protocol.reflect.fuzzy.FuzzyMatchers;
6565
import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract;
66-
import com.comphenix.protocol.utility.RemappedClassSource.RemapperUnavaibleException;
67-
import com.comphenix.protocol.utility.RemappedClassSource.RemapperUnavaibleException.Reason;
66+
import com.comphenix.protocol.utility.RemappedClassSource.RemapperUnavailableException;
67+
import com.comphenix.protocol.utility.RemappedClassSource.RemapperUnavailableException.Reason;
6868
import com.comphenix.protocol.wrappers.nbt.NbtFactory;
6969
import com.comphenix.protocol.wrappers.nbt.NbtType;
7070
import com.google.common.base.Joiner;
@@ -934,6 +934,9 @@ public static Class<?> getMinecraftServerClass() {
934934
return getMinecraftClass("MinecraftServer");
935935
} catch (RuntimeException e) {
936936
useFallbackServer();
937+
938+
// Reset cache and try again
939+
setMinecraftClass("MinecraftServer", null);
937940
return getMinecraftClass("MinecraftServer");
938941
}
939942
}
@@ -982,8 +985,10 @@ public static Class<?> getPlayerListClass() {
982985
try {
983986
return getMinecraftClass("ServerConfigurationManager", "PlayerList");
984987
} catch (RuntimeException e) {
985-
// Try again
986988
useFallbackServer();
989+
990+
// Reset cache and try again
991+
setMinecraftClass("ServerConfigurationManager", null);
987992
return getMinecraftClass("ServerConfigurationManager");
988993
}
989994
}
@@ -1628,8 +1633,10 @@ public static Class<?> getAttributeModifierClass() {
16281633
try {
16291634
return getMinecraftClass("AttributeModifier");
16301635
} catch (RuntimeException e) {
1631-
// Initialize first
16321636
getAttributeSnapshotClass();
1637+
1638+
// Reset cache and try again
1639+
setMinecraftClass("AttributeModifier", null);
16331640
return getMinecraftClass("AttributeModifier");
16341641
}
16351642
}
@@ -2108,7 +2115,7 @@ private static ClassSource getClassSource() {
21082115
// Attempt to use MCPC
21092116
try {
21102117
return classSource = new RemappedClassSource().initialize();
2111-
} catch (RemapperUnavaibleException e) {
2118+
} catch (RemapperUnavailableException e) {
21122119
if (e.getReason() != Reason.MCPC_NOT_PRESENT)
21132120
reporter.reportWarning(MinecraftReflection.class, Report.newBuilder(REPORT_CANNOT_FIND_MCPC_REMAPPER));
21142121
} catch (Exception e) {

‎modules/API/src/main/java/com/comphenix/protocol/utility/RemappedClassSource.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
import com.comphenix.protocol.reflect.FieldUtils;
3030
import com.comphenix.protocol.reflect.MethodUtils;
31-
import com.comphenix.protocol.utility.RemappedClassSource.RemapperUnavaibleException.Reason;
31+
import com.comphenix.protocol.utility.RemappedClassSource.RemapperUnavailableException.Reason;
3232

3333
class RemappedClassSource extends ClassSource {
3434
private Object classRemapper;
@@ -53,7 +53,7 @@ public RemappedClassSource(ClassLoader loader) {
5353
/**
5454
* Attempt to load the MCPC remapper.
5555
* @return TRUE if we succeeded, FALSE otherwise.
56-
* @throws RemapperUnavaibleException If the remapper is not present.
56+
* @throws RemapperUnavailableException If the remapper is not present.
5757
*/
5858
public RemappedClassSource initialize() {
5959
try {
@@ -64,14 +64,14 @@ public RemappedClassSource initialize() {
6464

6565
String version = server.getVersion();
6666
if (!version.contains("MCPC") && !version.contains("Cauldron")) {
67-
throw new RemapperUnavaibleException(Reason.MCPC_NOT_PRESENT);
67+
throw new RemapperUnavailableException(Reason.MCPC_NOT_PRESENT);
6868
}
6969

7070
// Obtain the Class remapper used by MCPC+/Cauldron
7171
this.classRemapper = FieldUtils.readField(getClass().getClassLoader(), "remapper", true);
7272

7373
if (this.classRemapper == null) {
74-
throw new RemapperUnavaibleException(Reason.REMAPPER_DISABLED);
74+
throw new RemapperUnavailableException(Reason.REMAPPER_DISABLED);
7575
}
7676

7777
// Initialize some fields and methods used by the Jar Remapper
@@ -82,7 +82,7 @@ public RemappedClassSource initialize() {
8282

8383
return this;
8484

85-
} catch (RemapperUnavaibleException e) {
85+
} catch (RemapperUnavailableException e) {
8686
throw e;
8787
} catch (Exception e) {
8888
// Damn it
@@ -115,7 +115,7 @@ public String getClassName(String path) {
115115
}
116116
}
117117

118-
public static class RemapperUnavaibleException extends RuntimeException {
118+
public static class RemapperUnavailableException extends RuntimeException {
119119
private static final long serialVersionUID = 1L;
120120

121121
public enum Reason {
@@ -139,7 +139,7 @@ public String getMessage() {
139139

140140
private final Reason reason;
141141

142-
public RemapperUnavaibleException(Reason reason) {
142+
public RemapperUnavailableException(Reason reason) {
143143
super(reason.getMessage());
144144
this.reason = reason;
145145
}

0 commit comments

Comments
 (0)