Jaco was adding some caching to a Java application. Quite wisely, Jaco wrote plenty of tests around his change, ran the test suite, and confirmed everything was green. It ran fine in testing, but when it went to production, everything failed.
Well, as it turned out, the configuration for the production environment loaded slightly different Java classes. One of those "only-loaded-in-production" modules did this:
private static void setRawInstance(final CacheManager cacheManager) {
lock.lock();
try {
final Field field = CacheManager.class.getDeclaredField("singleton");
field.setAccessible(true);
field.set(CacheManager.class, cacheManager);
} catch (Exception e) {
logger.error(e.getMessage(), e);
} finally {
lock.unlock();
}
}
A safe rule of thumb when it comes to using Java's "reflection" features is that you definitely don't use them unless you absolutely need them, and if you think you need them, you're probably wrong. Or, to put it another way: YAGNIEIYTYD. You ain't gonna need it, especially if you think you do.
This code uses reflection to access a private, static property declared on the CacheManager
class. It removes the private
check, using setAccessible
, and then sets the static property equal to the instance cacheManager
passed in.
In other words, this respects your class's privacy about the same amount as the AdTech industry respects yours.
Why was this in production? Why hadn't it broken anybody else's code? How long had this time bomb been sitting out there? That's a mystery.
Jaco sums up this code: "Essentially, the programmer was overwriting the private field of a 3rd party to do his dirty work. Arrogant, dangerous and completely unnecessary."