Skip to content

Commit 82ddc5b

Browse files
horghhorgh
authored
Merge pull request #329 from maxmind/greg/eng-3610
Fix enum decoding issue and improve error message
2 parents 01cd2d0 + 0dbb03c commit 82ddc5b

4 files changed

Lines changed: 226 additions & 12 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
CHANGELOG
22
=========
33

4+
4.0.2
5+
------------------
6+
7+
* Fixed a bug where enums with `@MaxMindDbCreator` would throw
8+
`ConstructorNotFoundException` when the data was stored via a pointer
9+
in the database. This commonly occurred with deduplicated data in larger
10+
databases. Reported by Fabrice Bacchella. GitHub #644 in GeoIP2-java.
11+
* Improved error messages when constructor invocation fails. The error now
12+
correctly identifies null values being passed to primitive parameters
13+
instead of reporting misleading boxed/primitive type mismatches.
14+
415
4.0.1 (2025-12-02)
516
------------------
617

‎src/main/java/com/maxmind/db/Decoder.java‎

Lines changed: 70 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ class Decoder {
3030

3131
private static final int[] POINTER_VALUE_OFFSETS = {0, 0, 1 << 11, (1 << 19) + (1 << 11), 0};
3232

33+
// Sentinel to cache "no creator method exists" to avoid repeated method scanning
34+
private static final CachedCreator NO_CREATOR = new CachedCreator(null, null);
35+
3336
private final NodeCache cache;
3437

3538
private final long pointerBase;
@@ -184,10 +187,17 @@ private boolean requiresLookupContext(Class<?> cls) {
184187
|| cls.equals(Object.class)
185188
|| Map.class.isAssignableFrom(cls)
186189
|| List.class.isAssignableFrom(cls)
190+
|| cls.isEnum()
187191
|| isSimpleType(cls)) {
188192
return false;
189193
}
190194

195+
// Non-enum classes with @MaxMindDbCreator don't require lookup context
196+
// since they just convert simple values (strings, booleans, etc.)
197+
if (getCachedCreator(cls) != null) {
198+
return false;
199+
}
200+
191201
var cached = getCachedConstructor(cls);
192202
if (cached == null) {
193203
cached = loadConstructorMetadata(cls);
@@ -696,16 +706,66 @@ private <T> Object decodeMapIntoObject(int size, Class<T> cls)
696706
var sbErrors = new StringBuilder();
697707
for (var key : parameterIndexes.keySet()) {
698708
var index = parameterIndexes.get(key);
699-
if (parameters[index] != null
700-
&& !parameters[index].getClass().isAssignableFrom(parameterTypes[index])) {
701-
sbErrors.append(" argument type mismatch in " + key + " MMDB Type: "
702-
+ parameters[index].getClass().getCanonicalName()
703-
+ " Java Type: " + parameterTypes[index].getCanonicalName());
709+
if (parameters[index] == null && parameterTypes[index].isPrimitive()) {
710+
sbErrors.append(" null value for primitive ")
711+
.append(parameterTypes[index].getName())
712+
.append(" parameter '").append(key).append("'.");
713+
} else if (parameters[index] != null
714+
&& !isAssignableType(parameters[index].getClass(), parameterTypes[index])) {
715+
sbErrors.append(" argument type mismatch in ").append(key)
716+
.append(" MMDB Type: ")
717+
.append(parameters[index].getClass().getCanonicalName())
718+
.append(" Java Type: ")
719+
.append(parameterTypes[index].getCanonicalName())
720+
.append(".");
704721
}
705722
}
706723
throw new DeserializationException(
707-
"Error creating object of type: " + cls.getSimpleName() + " - " + sbErrors, e);
724+
"Error creating object of type: " + cls.getSimpleName() + " -" + sbErrors, e);
725+
}
726+
}
727+
728+
/**
729+
* Checks if an actual type can be assigned to an expected type,
730+
* accounting for primitive/boxed equivalents (e.g., Boolean to boolean).
731+
*/
732+
private static boolean isAssignableType(Class<?> actual, Class<?> expected) {
733+
if (expected.isAssignableFrom(actual)) {
734+
return true;
735+
}
736+
// Check primitive/boxed equivalents for auto-unboxing
737+
if (expected.isPrimitive()) {
738+
return actual.equals(boxedType(expected));
708739
}
740+
return false;
741+
}
742+
743+
private static Class<?> boxedType(Class<?> primitive) {
744+
if (primitive == boolean.class) {
745+
return Boolean.class;
746+
}
747+
if (primitive == byte.class) {
748+
return Byte.class;
749+
}
750+
if (primitive == short.class) {
751+
return Short.class;
752+
}
753+
if (primitive == int.class) {
754+
return Integer.class;
755+
}
756+
if (primitive == long.class) {
757+
return Long.class;
758+
}
759+
if (primitive == float.class) {
760+
return Float.class;
761+
}
762+
if (primitive == double.class) {
763+
return Double.class;
764+
}
765+
if (primitive == char.class) {
766+
return Character.class;
767+
}
768+
return primitive;
709769
}
710770

711771
private boolean shouldInstantiateFromContext(Class<?> parameterType) {
@@ -960,14 +1020,15 @@ private Object convertValue(Object value, Class<?> targetType) {
9601020

9611021
private CachedCreator getCachedCreator(Class<?> cls) {
9621022
CachedCreator cached = this.creators.get(cls);
1023+
if (cached == NO_CREATOR) {
1024+
return null; // Known to have no creator
1025+
}
9631026
if (cached != null) {
9641027
return cached;
9651028
}
9661029

9671030
CachedCreator creator = findCreatorMethod(cls);
968-
if (creator != null) {
969-
this.creators.putIfAbsent(cls, creator);
970-
}
1031+
this.creators.putIfAbsent(cls, creator != null ? creator : NO_CREATOR);
9711032
return creator;
9721033
}
9731034

‎src/test/java/com/maxmind/db/ReaderTest.java‎

Lines changed: 144 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -493,8 +493,8 @@ public void testNoIpV4SearchTreeStream(int chunkSizes) throws IOException {
493493

494494
private void testNoIpV4SearchTree(Reader reader) throws IOException {
495495

496-
assertEquals("::0/64", reader.get(InetAddress.getByName("1.1.1.1"), String.class));
497-
assertEquals("::0/64", reader.get(InetAddress.getByName("192.1.1.1"), String.class));
496+
assertEquals("::/64", reader.get(InetAddress.getByName("1.1.1.1"), String.class));
497+
assertEquals("::/64", reader.get(InetAddress.getByName("192.1.1.1"), String.class));
498498
}
499499

500500
@ParameterizedTest
@@ -2042,6 +2042,148 @@ private void testIpV6(Reader reader, File file) throws IOException {
20422042
}
20432043
}
20442044

2045+
// =========================================================================
2046+
// Tests for enum with @MaxMindDbCreator when data is stored via pointer
2047+
// See: https://github.com/maxmind/GeoIP2-java/issues/644
2048+
// =========================================================================
2049+
2050+
/**
2051+
* Enum with @MaxMindDbCreator for converting string values.
2052+
* This simulates how ConnectionType enum works in geoip2.
2053+
*/
2054+
enum ConnectionTypeEnum {
2055+
DIALUP("Dialup"),
2056+
CABLE_DSL("Cable/DSL"),
2057+
CORPORATE("Corporate"),
2058+
CELLULAR("Cellular"),
2059+
SATELLITE("Satellite"),
2060+
UNKNOWN("Unknown");
2061+
2062+
private final String name;
2063+
2064+
ConnectionTypeEnum(String name) {
2065+
this.name = name;
2066+
}
2067+
2068+
@MaxMindDbCreator
2069+
public static ConnectionTypeEnum fromString(String s) {
2070+
if (s == null) {
2071+
return UNKNOWN;
2072+
}
2073+
return switch (s) {
2074+
case "Dialup" -> DIALUP;
2075+
case "Cable/DSL" -> CABLE_DSL;
2076+
case "Corporate" -> CORPORATE;
2077+
case "Cellular" -> CELLULAR;
2078+
case "Satellite" -> SATELLITE;
2079+
default -> UNKNOWN;
2080+
};
2081+
}
2082+
}
2083+
2084+
/**
2085+
* Model class that uses the ConnectionTypeEnum for the connection_type field.
2086+
*/
2087+
static class TraitsModel {
2088+
ConnectionTypeEnum connectionType;
2089+
2090+
@MaxMindDbConstructor
2091+
public TraitsModel(
2092+
@MaxMindDbParameter(name = "connection_type")
2093+
ConnectionTypeEnum connectionType
2094+
) {
2095+
this.connectionType = connectionType;
2096+
}
2097+
}
2098+
2099+
/**
2100+
* Top-level model for Enterprise database records.
2101+
*/
2102+
static class EnterpriseModel {
2103+
TraitsModel traits;
2104+
2105+
@MaxMindDbConstructor
2106+
public EnterpriseModel(
2107+
@MaxMindDbParameter(name = "traits")
2108+
TraitsModel traits
2109+
) {
2110+
this.traits = traits;
2111+
}
2112+
}
2113+
2114+
/**
2115+
* This test passes because IP 74.209.24.0 has connection_type stored inline.
2116+
*/
2117+
@ParameterizedTest
2118+
@MethodSource("chunkSizes")
2119+
public void testEnumCreatorWithInlineData(int chunkSize) throws IOException {
2120+
try (var reader = new Reader(getFile("GeoIP2-Enterprise-Test.mmdb"), chunkSize)) {
2121+
var ip = InetAddress.getByName("74.209.24.0");
2122+
var result = reader.get(ip, EnterpriseModel.class);
2123+
assertNotNull(result);
2124+
assertNotNull(result.traits);
2125+
assertEquals(ConnectionTypeEnum.CABLE_DSL, result.traits.connectionType);
2126+
}
2127+
}
2128+
2129+
/**
2130+
* This test verifies that enums with @MaxMindDbCreator work correctly when
2131+
* the data is stored via a pointer (common for deduplication in databases).
2132+
*
2133+
* <p>Previously, this would throw ConstructorNotFoundException because
2134+
* requiresLookupContext() called loadConstructorMetadata() before checking
2135+
* for creator methods.
2136+
*/
2137+
@ParameterizedTest
2138+
@MethodSource("chunkSizes")
2139+
public void testEnumCreatorWithPointerData(int chunkSize) throws IOException {
2140+
try (var reader = new Reader(getFile("GeoIP2-Enterprise-Test.mmdb"), chunkSize)) {
2141+
var ip = InetAddress.getByName("89.160.20.112");
2142+
var result = reader.get(ip, EnterpriseModel.class);
2143+
assertNotNull(result);
2144+
assertNotNull(result.traits);
2145+
assertEquals(ConnectionTypeEnum.CORPORATE, result.traits.connectionType);
2146+
}
2147+
}
2148+
2149+
// Model class with primitive double field for testing null-to-primitive error messages
2150+
static class IpRiskModelWithPrimitive {
2151+
public final double ipRisk;
2152+
2153+
@MaxMindDbConstructor
2154+
public IpRiskModelWithPrimitive(
2155+
@MaxMindDbParameter(name = "ip_risk") double ipRisk
2156+
) {
2157+
this.ipRisk = ipRisk;
2158+
}
2159+
}
2160+
2161+
/**
2162+
* Tests that error messages correctly report null-to-primitive issues instead of
2163+
* misleading boxed/primitive type mismatch messages.
2164+
*
2165+
* <p>IP 11.1.2.3 in the IP Risk test database doesn't have an ip_risk field.
2166+
* When deserializing to a model with a primitive double, the error message should
2167+
* correctly identify "null value for primitive double" rather than reporting
2168+
* misleading Boolean/boolean mismatches.
2169+
*/
2170+
@ParameterizedTest
2171+
@MethodSource("chunkSizes")
2172+
public void testNullToPrimitiveErrorMessage(int chunkSize) throws IOException {
2173+
try (var reader = new Reader(getFile("GeoIP2-IP-Risk-Test.mmdb"), chunkSize)) {
2174+
var ip = InetAddress.getByName("11.1.2.3");
2175+
2176+
var exception = assertThrows(DeserializationException.class,
2177+
() -> reader.get(ip, IpRiskModelWithPrimitive.class));
2178+
2179+
// Error message should mention null value for primitive, not type mismatch
2180+
assertTrue(exception.getMessage().contains("null value for primitive double"),
2181+
"Error message should identify null-to-primitive issue: " + exception.getMessage());
2182+
assertTrue(exception.getMessage().contains("ip_risk"),
2183+
"Error message should name the problematic parameter: " + exception.getMessage());
2184+
}
2185+
}
2186+
20452187
static File getFile(String name) {
20462188
return new File(ReaderTest.class.getResource("/maxmind-db/test-data/" + name).getFile());
20472189
}

‎src/test/resources/maxmind-db‎

Submodule maxmind-db updated 83 files

0 commit comments

Comments
 (0)