Skip to content

Comments

Remove JsonInjector class#608

Merged
oschwald merged 1 commit intomainfrom
horgh/rm-jsoninjector
Sep 26, 2025
Merged

Remove JsonInjector class#608
oschwald merged 1 commit intomainfrom
horgh/rm-jsoninjector

Conversation

@horgh
Copy link
Contributor

@horgh horgh commented Sep 26, 2025

This was not doing much given we always passed IP and network as null. In the past, when we used Jackson for MMDB deserialization, this was more extensively used.

@horgh horgh requested a review from Copilot September 26, 2025 19:54
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 removes the JsonInjector class and simplifies Jackson deserialization by removing @JacksonInject annotations that were always passing null values. The change eliminates unnecessary dependency injection complexity since IP address and network parameters were consistently null.

  • Replaces JsonInjector with standard Jackson InjectableValues.Std for locales injection
  • Removes @JacksonInject annotations from constructor parameters across all response models
  • Deletes the JsonInjector class entirely

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

File Description
src/main/java/com/maxmind/geoip2/JsonInjector.java Completely removes the JsonInjector class
src/main/java/com/maxmind/geoip2/WebServiceClient.java Replaces JsonInjector with InjectableValues.Std for locales
src/main/java/com/maxmind/geoip2/record/Traits.java Removes @JacksonInject from ip_address and network parameters
src/main/java/com/maxmind/geoip2/model/*.java Removes @JacksonInject annotations from constructor parameters in all response models

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@horgh
Copy link
Contributor Author

horgh commented Sep 26, 2025

This test suggests behaviour should be as expected still:

public void test200WithDefaultValues() throws Exception {
WebServiceClient client = createSuccessClient("insights", "1.2.3.13",
"{\"traits\":{\"ip_address\":\"1.2.3.13\",\"network\":\"1.2.3.0/24\"}}");
InsightsResponse insights = client.insights(InetAddress
.getByName("1.2.3.13"));
assertThat(insights.toString(),
CoreMatchers.startsWith("com.maxmind.geoip2.model.InsightsResponse [ {"));
City city = insights.getCity();
assertNotNull(city);
assertNull(city.getConfidence());
Continent continent = insights.getContinent();
assertNotNull(continent);
assertNull(continent.getCode());
Country country = insights.getCountry();
assertNotNull(country);
Location location = insights.getLocation();
assertNotNull(location);
assertNull(location.getAccuracyRadius());
assertNull(location.getLatitude());
assertNull(location.getLongitude());
assertNull(location.getTimeZone());
assertThat(location.toString(),
CoreMatchers.equalTo("com.maxmind.geoip2.record.Location [ {} ]"));
MaxMind maxmind = insights.getMaxMind();
assertNotNull(maxmind);
assertNull(maxmind.getQueriesRemaining());
assertNotNull(insights.getPostal());
Country registeredCountry = insights.getRegisteredCountry();
assertNotNull(registeredCountry);
RepresentedCountry representedCountry = insights
.getRepresentedCountry();
assertNotNull(representedCountry);
assertNull(representedCountry.getType());
List<Subdivision> subdivisions = insights.getSubdivisions();
assertNotNull(subdivisions);
assertTrue(subdivisions.isEmpty());
Subdivision subdiv = insights.getMostSpecificSubdivision();
assertNotNull(subdiv);
assertNull(subdiv.getIsoCode());
assertNull(subdiv.getConfidence());
Subdivision leastSpecificSubdiv = insights.getLeastSpecificSubdivision();
assertNotNull(leastSpecificSubdiv);
assertNull(leastSpecificSubdiv.getIsoCode());
assertNull(leastSpecificSubdiv.getConfidence());
Traits traits = insights.getTraits();
assertNotNull(traits);
assertNull(traits.getAutonomousSystemNumber());
assertNull(traits.getAutonomousSystemOrganization());
assertNull(traits.getConnectionType());
assertNull(traits.getDomain());
assertEquals("1.2.3.13", traits.getIpAddress());
assertEquals("1.2.3.0/24", traits.getNetwork().toString());

This was not doing much given we always passed IP and network as null.
In the past, when we used Jackson for MMDB deserialization, this was
more extensively used.
@horgh horgh force-pushed the horgh/rm-jsoninjector branch from dc2a2a4 to 8490468 Compare September 26, 2025 21:13
@oschwald oschwald merged commit a57188e into main Sep 26, 2025
23 checks passed
@oschwald oschwald deleted the horgh/rm-jsoninjector branch September 26, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants