Support fallback constructor/parameter selection #309

oschwald · 2025-10-14T14:08:48Z

Prefer @MaxMindDbConstructor; fallback to record canonical or single
public constructor
Infer parameter names: annotation > record component > Java parameter
when -parameters is set; throw ParameterNotFoundException if names
unavailable
Enable -parameters in maven-compiler-plugin

- Prefer @MaxMindDbConstructor; fallback to record canonical or single public constructor - Infer parameter names: annotation > record component > Java parameter when `-parameters` is set; throw ParameterNotFoundException if names unavailable - Enable -parameters in maven-compiler-plugin

Add coercion for INT32/UINT16/UINT32 to target primitives with range checks.

We need both useDefault and defaultValue as otherwise there is no way to distinguish between the default value being the empty string and no default being set. If defaultValue is not set but useDefault is, we will use the Java default value, e.g., 0 or the empty string.

Copilot

Pull Request Overview

This PR enhances constructor and parameter resolution for MaxMind DB deserialization by adding fallback mechanisms and improving flexibility. The changes enable using records and POJOs without explicit annotations while maintaining backward compatibility.

Adds fallback constructor selection: prefer @MaxMindDbConstructor, fall back to record canonical constructor or single public constructor
Introduces parameter name inference: annotation > record component > Java parameter name (with -parameters flag)
Implements lookup context injection via @MaxMindDbIpAddress and @MaxMindDbNetwork annotations

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pom.xml	Enables `-parameters` compiler flag and updates version to 4.0.0-SNAPSHOT
src/main/java/com/maxmind/db/Reader.java	Adds creators cache and passes lookup context to decoder
src/main/java/com/maxmind/db/Decoder.java	Implements fallback constructor selection, parameter name inference, context injection, and type conversion
src/main/java/com/maxmind/db/DatabaseRecord.java	Removes legacy constructor accepting InetAddress and int
src/main/java/com/maxmind/db/Networks.java	Updates to use Network object and pass lookup context
src/main/java/com/maxmind/db/BufferHolder.java	Refactors stream reading to properly handle large databases with chunking
src/main/java/com/maxmind/db/CachedConstructor.java	Extends to store parameter defaults, injections, and context requirements
src/main/java/com/maxmind/db/CachedCreator.java	New record for caching creator method metadata
src/main/java/com/maxmind/db/ParameterInjection.java	New enum defining injection types for constructor parameters
src/main/java/com/maxmind/db/MaxMindDbParameter.java	Adds useDefault and defaultValue fields
src/main/java/com/maxmind/db/MaxMindDbIpAddress.java	New annotation for injecting lookup IP address
src/main/java/com/maxmind/db/MaxMindDbNetwork.java	New annotation for injecting lookup network
src/main/java/com/maxmind/db/MaxMindDbCreator.java	New annotation for custom type conversion methods
src/test/java/com/maxmind/db/ReaderTest.java	Adds comprehensive tests for new features including records, implicit parameters, context injection, creator methods, and primitive handling
README.md	Documents new constructor selection, parameter inference, defaults, and context injection features
CHANGELOG.md	Documents breaking changes and new features

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-24T16:54:20Z

src/main/java/com/maxmind/db/BufferHolder.java

+    // Reasonable I/O buffer size for reading from InputStream.
+    // This is separate from chunk size which determines MultiBuffer chunk allocation.
+    private static final int IO_BUFFER_SIZE = 16 * 1024; // 16KB


[nitpick] The comment states '16KB' but should specify that this is for stream I/O operations. Consider rephrasing to: 'Reasonable I/O buffer size (16KB) for reading from InputStream. This is separate from chunkSize which determines MultiBuffer chunk allocation.'

Suggested change

// Reasonable I/O buffer size for reading from InputStream.

// This is separate from chunk size which determines MultiBuffer chunk allocation.

private static final int IO_BUFFER_SIZE = 16 * 1024; // 16KB

// Reasonable I/O buffer size (16KB) for reading from InputStream.

// This is separate from chunkSize which determines MultiBuffer chunk allocation.

private static final int IO_BUFFER_SIZE = 16 * 1024;

Copilot · 2025-10-24T16:54:21Z

pom.xml

-            <url>https://oss.sonatype.org/service/local/staging/deploy/maven2/</url>
-        </repository>
-    </distributionManagement>
 </project>


The distributionManagement section was removed. If this repository still needs to publish artifacts to Maven Central or other repositories, this removal could break the release process. Verify that artifact distribution is handled elsewhere or restore this configuration if needed.

This adds support for marking static factory methods with @MaxMindDbCreator to enable custom deserialization logic, similar to Jackson's @JsonCreator. The decoder now automatically invokes creator methods when decoding values to target types, allowing for custom type conversions such as string-to-enum mappings with non-standard representations. This eliminates the need for redundant constructors that only perform type conversions, as the decoder can now apply conversions automatically via annotated static factory methods. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

The previous implementation attempted to allocate a temporary I/O buffer equal to chunkSize (~2GB), causing OutOfMemoryError even with increased heap settings. This was a bug introduced when BufferHolder was refactored to use DEFAULT_CHUNK_SIZE for the I/O buffer. Changes: - Use separate IO_BUFFER_SIZE constant (16KB) for reading from streams - Use ByteArrayOutputStream to accumulate data into chunkSize-sized chunks - Guarantee all non-final chunks are exactly chunkSize bytes - Support databases >2GB by creating multiple chunks - Use chunks.size() to determine SingleBuffer vs MultiBuffer - Consistent with pre-MultiBuffer approach but with >2GB support This fixes macOS CI failures and supports databases of any size with minimal memory overhead. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Replace Stack with ArrayDeque in Networks.java for better performance by removing unnecessary synchronization overhead. Replace Vector with ArrayList in test code for consistency with modern Java practices. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

horgh · 2025-10-24T19:06:30Z

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

+            return value;
+        }
+        if (target.equals(Integer.TYPE) || target.equals(Integer.class)) {
+            if (value < 0 || value > Integer.MAX_VALUE) {


I'm wondering if we'd need a < 0 check for the int function too given we also use it with unsigned. Not sure, just seeing this being different from the first function made me wonder. Or maybe this should be using MIN_VALUE?

coerceFromInt gets used for int32 too. I think I will convert this to use Integer.MIN_VALUE. The current code conflates what we are expecting decodeUint32 to return with what the type can hold.

Make the Integer range check consistent with Short and Byte conversions by using Integer.MIN_VALUE instead of checking for < 0. This ensures all narrowing conversions follow the same pattern and correctly handle the full range of Integer values. Addresses PR feedback in #309. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Implement coerceFromBigInteger() method to enable type coercion for UINT64 and UINT128 values, making them consistent with UINT16, UINT32, and INT32 types. This allows UINT64/UINT128 values to be decoded into smaller types (Long, Integer, Short, Byte) when they fit within the target type's range. Add comprehensive tests covering successful coercions and proper range checking for out-of-bounds values. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

For UINT64/UINT128 values less than 8 bytes (which fit in long's positive range), decode directly to long instead of BigInteger when the target is a typed field. This avoids unnecessary BigInteger object allocation for common cases like UINT64 values under 2^56. Maintains backward compatibility by preserving BigInteger return type for Object.class targets (e.g., Map decoding). Performance impact: - UINT64(500) into Long field: decodes as long (no BigInteger allocation) - UINT64(500) into Object/Map: still returns BigInteger (backward compatible) - UINT64 >= 8 bytes: always uses BigInteger (correctness) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

oschwald force-pushed the greg/eng-3230 branch from 403117d to 1a75d10 Compare October 23, 2025 16:50

oschwald added 6 commits October 23, 2025 11:30

Decode int primitive constructor params

4278ea7

Add coercion for INT32/UINT16/UINT32 to target primitives with range checks.

Add lookup context injection annotations

88c3ecd

Drop DatabaseRecord InetAddress constructor

90d18b4

Handle nested context-only constructors

d746dad

oschwald force-pushed the greg/eng-3230 branch 2 times, most recently from 250601f to 2654c85 Compare October 23, 2025 22:24

oschwald requested a review from Copilot October 24, 2025 16:53

Copilot AI reviewed Oct 24, 2025

View reviewed changes

oschwald and others added 5 commits October 24, 2025 09:59

Publish snapshot release

d6c7e61

Remove outdated distributionManagement section

8944a97

oschwald force-pushed the greg/eng-3230 branch from 9b82f3f to 5f3ac3b Compare October 24, 2025 16:59

horgh requested changes Oct 24, 2025

View reviewed changes

oschwald and others added 3 commits October 24, 2025 15:18

horgh approved these changes Oct 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Support fallback constructor/parameter selection #309

Support fallback constructor/parameter selection #309

Uh oh!

oschwald commented Oct 14, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 24, 2025

Uh oh!

Copilot AI Oct 24, 2025

Uh oh!

horgh Oct 24, 2025

Uh oh!

oschwald Oct 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Support fallback constructor/parameter selection #309

Are you sure you want to change the base?

Support fallback constructor/parameter selection #309

Uh oh!

Conversation

oschwald commented Oct 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

horgh Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

oschwald Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants