Skip to content

Modernization changes for new major version #620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Modernization changes for new major version #620

wants to merge 17 commits into from

Conversation

@oschwald
Copy link
Member

  • Move model and record classes to records
  • Deprecate get methods in non-record classes*

@oschwald oschwald force-pushed the greg/eng-3230 branch 2 times, most recently from 09437e4 to 98265c6 Compare October 14, 2025 15:40
oschwald and others added 9 commits October 23, 2025 12:04
@oschwald
Move model and record classes to records
491b003 
This will eventually reduce boilerplate and provide a more consistent
experience for users. I am leaving the old getter methods as deprecated
for one major version to give users the ability to upgrade before having
to migrate to the updated accessors. There are other breaking changes,
but these will likely not affect most users.
@oschwald @claude
Deprecate get* methods in non-record classes
4ebf177 
Public getter methods in non-record classes (DatabaseReader, exception
classes) have been renamed to follow the same naming convention as
records (e.g., metadata() instead of getMetadata()). The old getter
methods are still available but have been deprecated and will be removed
in version 6.0.0.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Update to maxmind-db 4.0.0 and use non-deprecated accessors
2192c59 
- Bump maxmind-db dependency from 3.2.0 to 4.0.0-SNAPSHOT
- Update method calls to use record-style accessors instead of deprecated
  JavaBean-style getters in DatabaseReader.java and test files
- Changes include: getDatabaseType() → databaseType(), getData() → data(),
  getNetwork() → network(), getNetworkAddress() → networkAddress(),
  getPrefixLength() → prefixLength()

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Remove most uses of @MaxMindDbConstructor
fe27a08 
With maxmind-db 4.0.0, records automatically use their canonical
constructor for deserialization when no constructor is explicitly
annotated with @MaxMindDbConstructor. This change removes the
annotation from most records in the codebase.

Changes:
- Removed empty canonical constructors from records with no logic
- Removed @MaxMindDbConstructor from canonical constructors with
  immutability/validation logic, relying on automatic detection
- Added useDefault=true to boolean parameters to leverage automatic
  null-to-false conversion instead of manual Boolean→boolean handling
- Kept @MaxMindDbConstructor only for records requiring custom type
  conversions (String→LocalDate in AnonymousPlusResponse, String→enum
  in ConnectionTypeResponse and Traits)
- Removed unused MaxMindDbConstructor imports

All tests pass.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Remove redundant constructors now that IP/network are auto-injected
89dad0f 
- Remove copy constructors from model classes that only populated IP address and network
  (AnonymousIpResponse, AnonymousPlusResponse, ConnectionTypeResponse, AsnResponse,
  DomainResponse, IspResponse, IpRiskResponse)
- Update DatabaseReader getter methods to return responses directly
- Update annotations to use @MaxMindDbIpAddress and @MaxMindDbNetwork for context injection
- Remove @MaxMindDbConstructor from Traits that was only needed for ConnectionType conversion
- Add @MaxMindDbCreator to ConnectionType.fromString() for enum deserialization

These constructors are no longer needed since IP address and network are now automatically
injected via @MaxMindDbIpAddress and @MaxMindDbNetwork annotations, and enum conversion
is handled by @MaxMindDbCreator.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Use generics to eliminate redundant private getX methods
99b2ec2 
All the private getX methods in DatabaseReader followed similar patterns,
so we've consolidated them into a single generic getResponse() method.
For responses that need locale transformations (City, Country, Enterprise),
we use Optional.map() to apply the transformation.

This also changes the get() method to accept an explicit caller parameter
instead of introspecting the stack trace, making error messages clearer
and more reliable.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Change ipAddress from String to InetAddress in all models
9429631 
Changed the ipAddress field type from String to InetAddress across all
model and record classes. This provides better type safety and makes it
clear that these fields contain IP addresses.

Key changes:
- Updated ipAddress field type to InetAddress in all response/record classes
- Deprecated getIpAddress() methods now call ipAddress().getHostAddress()
  to maintain backward compatibility (still return String)
- Created InetAddressSerializer to serialize InetAddress to String in JSON
- Created InetAddressDeserializer to deserialize JSON strings to InetAddress
- Created InetAddressModule and registered it globally in JsonSerializable
- Updated all tests to call .getHostAddress() when comparing IP addresses

The new ipAddress() accessor returns InetAddress, while the deprecated
getIpAddress() continues to return String for backward compatibility.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald
Remove unused imports
f44dfd8
@oschwald
Remove outdated distributionManagement section
f5299b9
oschwald and others added 2 commits October 23, 2025 12:08
@oschwald
Prepare for a snapshot release
cb99a66
@oschwald @claude
Fix documentation typos in record getters
9309fdc 
Corrected typos and style inconsistencies in deprecated getter javadocs
to match the param documentation:

- MaxMind: "queried" → "queries"
- Subdivision.getConfidence(): "This is a value" → "A value"
- Subdivision.getIsoCode(): "contain" → "containing", "3166-2code" → "3166-2 code"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Add repository configuration for consuming snapshot releases
9d5369d 
This enables Maven to resolve snapshot dependencies from the Central Portal.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald oschwald force-pushed the greg/eng-3230 branch 2 times, most recently from 04cea6f to 9d5369d Compare October 23, 2025 22:30
oschwald and others added 5 commits October 24, 2025 06:56
@oschwald @claude
Replace explicit types with var for local variables
33c09f2 
This modernizes the codebase by using Java's var keyword for local
variables where the type is obvious from the initializer. This includes:
- Local variable declarations with initializers
- Enhanced for-loop variables
- Try-with-resources variables

Some try-block variables were refactored into helper methods to enable
var usage (parsePrefixLength in NetworkDeserializer and readBody in
WebServiceClient).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Convert switch statement to switch expression
0eb3d9a 
Modernize the error code handling in handleErrorWithJsonBody() to use
a switch expression instead of the traditional switch statement. This
makes the code more concise and aligns with the style already used in
ConnectionTypeResponse.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Replace .equals("") with .isEmpty()
f3af96e 
Use the more idiomatic String.isEmpty() method instead of comparing
against an empty string literal. This expresses the intent more
clearly and is the standard Java convention for empty string checks.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Use Optional.orElseThrow() pattern
79e3d9c 
Replace the manual isEmpty() check and get() pattern with the more
functional and concise orElseThrow() method. This eliminates 30 lines
of boilerplate code across 10 lookup methods while maintaining the
exact same behavior.

Before:
  var r = tryCountry(ipAddress);
  if (r.isEmpty()) {
    throw new AddressNotFoundException(...);
  }
  return r.get();

After:
  return tryCountry(ipAddress).orElseThrow(() ->
    new AddressNotFoundException(...));

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald @claude
Seal GeoIp2Exception hierarchy
5218189 
Make GeoIp2Exception a sealed class with an explicit permits clause
listing all allowed subclasses. This provides:

- Explicit API stability guarantees - new exception types cannot be
  added externally
- Compiler-enforced exhaustiveness checking for pattern matching in
  future Java versions
- Clear documentation of the complete exception hierarchy

All subclasses (AddressNotFoundException, AuthenticationException,
InvalidRequestException, OutOfQueriesException, and
PermissionRequiredException) were already marked as final, making them
compatible with the sealed parent.

Note: HttpException extends IOException, not GeoIp2Exception, so it
remains outside this sealed hierarchy.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@oschwald oschwald requested a review from Copilot October 24, 2025 16:53
Copilot AI reviewed Oct 24, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modernizes the codebase for a new major version (5.0.0) by converting model and record classes to Java records and deprecating getter methods in favor of record-style accessor methods.

Key Changes:

  • Converted all model and record classes from traditional classes to Java records
  • Deprecated get* methods throughout the codebase in favor of record accessor methods (e.g., city() instead of getCity())
  • Replaced abstract base classes with interfaces (JsonSerializable, NamedRecord)

Reviewed Changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/main/java/com/maxmind/geoip2/record/*.java Converted record classes (City, Country, Continent, etc.) from classes to records
src/main/java/com/maxmind/geoip2/model/*.java Converted model classes (CityResponse, CountryResponse, etc.) from classes to records
src/main/java/com/maxmind/geoip2/exception/*.java Added new accessor methods and deprecated getters
src/main/java/com/maxmind/geoip2/*.java Added new interfaces and modules for JSON serialization
src/test/java/**/*.java Updated tests to use new accessor methods
pom.xml Updated version to 5.0.0-SNAPSHOT and dependencies

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

src/main/java/com/maxmind/geoip2/record/Traits.java
traits.getStaticIpScore()
);
public Traits() {
this(null, null, (ConnectionType) null, null,
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicit (ConnectionType) null cast is unnecessary. The compiler can infer the type from the parameter. Consider removing the cast for cleaner code.

Suggested change
this(null, null, (ConnectionType) null, null,
this(null, null, null, null,

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@horgh horgh horgh approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@oschwald @horgh