Skip to content

Conversation

@itssimon
Copy link
Member

@itssimon itssimon commented Jan 22, 2026

Summary by CodeRabbit

  • New Features

    • Added public accessor to retrieve request-logging configuration.
  • Chores

    • Enforced consistent code formatting across the project and CI.
    • Standardized editor configuration for files.
  • Refactor

    • Hardened network retry/error handling behavior.
    • Improved handling of callback initialization to avoid stale state.
  • Tests

    • Updated tests to align with new formatting outcomes.

✏️ Tip: You can customize this high-level summary in your review settings.

@itssimon itssimon self-assigned this Jan 22, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Adds Spotless formatting via Maven and an .editorconfig; replaces pre-commit hooks with a Maven formatting check in CI. Applies widespread code formatting changes across Java sources and tests and introduces behavioral changes in ApitallyClient sendHubRequest and RequestLogger public accessor/JSON handling.

Changes

Cohort / File(s) Summary
Build & CI
/.editorconfig, .github/workflows/tests.yaml, .pre-commit-config.yaml, pom.xml
Add .editorconfig; replace pre-commit hooks with Maven Spotless check in CI; add spotless-maven-plugin (Google Java Format 1.33.0 AOSP) to pom.xml; remove pre-commit hook entries.
Apitally client & logging core
src/main/java/io/apitally/common/ApitallyClient.java, src/main/java/io/apitally/common/ApitallyAppender.java
Formatting and import reordering; in ApitallyClient.sendHubRequest change: failures now result in logged return of RETRYABLE_ERROR via CompletableFuture.supplyAsync with internal retry rather than throwing a RetryableHubRequestException.
Request logging
src/main/java/io/apitally/common/RequestLogger.java, src/main/java/io/apitally/common/RequestLoggingCallbacks.java
Add public RequestLoggingConfig getConfig(); introduce ObjectMapper; refactor pattern compilation, masking/filtering and JSON masking logic; many formatting and safety checks updated. Javadoc formatting tweaks in callbacks.
Counters & registry
src/main/java/io/apitally/common/ConsumerRegistry.java, src/main/java/io/apitally/common/RequestCounter.java, src/main/java/io/apitally/common/ServerErrorCounter.java, src/main/java/io/apitally/common/ValidationErrorCounter.java
Import reorderings and formatting; ValidationErrorCounter and ServerErrorCounter iterators refactored (lambda styles); no behavioral API changes.
Utilities & infra
src/main/java/io/apitally/common/InstanceLock.java, src/main/java/io/apitally/common/TempGzipFile.java, src/main/java/io/apitally/common/RequestLoggingCallbacks.java
Formatting and reflowing of method calls and control expressions; no semantic changes.
DTOs
src/main/java/io/apitally/common/dto/*
Constructor parameter lists and imports reformatted across Request, RequestLogItem, Requests, ServerError, ServerErrors, StartupData, SyncData, ValidationError(s), RequestResponseBase; no signature/type changes.
Spring config & utils
src/main/java/io/apitally/spring/ApitallyAutoConfiguration.java, src/main/java/io/apitally/spring/ApitallyExceptionResolver.java, src/main/java/io/apitally/spring/ApitallyProperties.java, src/main/java/io/apitally/spring/ApitallyFilter.java, src/main/java/io/apitally/spring/ApitallyUtils.java, src/main/java/io/apitally/spring/UseApitally.java
Mostly formatting and import reorganizations; ApitallyProperties.setCallbacksClass now explicitly sets callbacks to null on initialization failure; ApitallyUtils modifier order normalized. ApitallyFilter extensive reflow only.
Tests & examples
src/test/java/... (many files, e.g., ApitallyClientTest.java, RequestLoggerTest.java, ApitallyFilterTest.java, TestController.java, TestItem.java, ...)
Widespread import consolidation and formatting; one observable test expectation formatting change in RequestLoggerTest (response body string compacted: "{"items":[]}"). Most changes are stylistic.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.06% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add Spotless' accurately reflects the primary changes in the pull request, which involve integrating Spotless Maven plugin for code formatting while removing pre-commit hooks.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove-pre-commit

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Jan 22, 2026

Codecov Report

❌ Patch coverage is 80.05952% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.68%. Comparing base (1392b58) to head (ecb4a48).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...c/main/java/io/apitally/common/ApitallyClient.java 60.97% 31 Missing and 1 partial ⚠️
...rc/main/java/io/apitally/spring/ApitallyUtils.java 48.00% 10 Missing and 3 partials ⚠️
...c/main/java/io/apitally/spring/ApitallyFilter.java 87.69% 1 Missing and 7 partials ⚠️
.../io/apitally/spring/ApitallyAutoConfiguration.java 0.00% 7 Missing ⚠️
...rc/main/java/io/apitally/common/RequestLogger.java 96.42% 0 Missing and 3 partials ⚠️
...in/java/io/apitally/common/ServerErrorCounter.java 91.66% 0 Missing and 2 partials ⚠️
...ava/io/apitally/common/ValidationErrorCounter.java 90.47% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main      #53      +/-   ##
============================================
+ Coverage     77.95%   78.68%   +0.72%     
+ Complexity      357      354       -3     
============================================
  Files            34       34              
  Lines          1184     1276      +92     
  Branches        150      162      +12     
============================================
+ Hits            923     1004      +81     
- Misses          178      187       +9     
- Partials         83       85       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@itssimon itssimon changed the title Add Spotless Format code with Spotless Jan 22, 2026
@itssimon itssimon merged commit 28f8a88 into main Jan 22, 2026
17 checks passed
@itssimon itssimon deleted the remove-pre-commit branch January 22, 2026 12:43
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/io/apitally/spring/ApitallyProperties.java (1)

60-77: Good defensive changes, but consider handling non-assignable class case.

The explicit setCallbacks(null) in the catch block (line 72) and else branch (lines 74-76) are good defensive additions. However, there's a silent failure path: if the class loads successfully but does not implement RequestLoggingCallbacks, the code silently does nothing—no warning is logged, and callbacks retains its previous value.

Consider adding a warning and explicit null assignment for this case:

Suggested improvement
 public void setCallbacksClass(String callbacksClass) {
     this.callbacksClass = callbacksClass;
     if (callbacksClass != null) {
         try {
             Class<?> clazz = Class.forName(callbacksClass);
             if (RequestLoggingCallbacks.class.isAssignableFrom(clazz)) {
                 setCallbacks(
                         (RequestLoggingCallbacks)
                                 clazz.getDeclaredConstructor().newInstance());
+            } else {
+                logger.warn(
+                        "Class {} does not implement RequestLoggingCallbacks",
+                        callbacksClass);
+                setCallbacks(null);
             }
         } catch (ReflectiveOperationException e) {
             logger.error("Failed to initialize request logging callbacks", e);
             setCallbacks(null);
         }
     } else {
         setCallbacks(null);
     }
 }
🤖 Fix all issues with AI agents
In `@pom.xml`:
- Around line 115-129: The Spotless config uses google-java-format version
1.33.0 (inside the <googleJavaFormat> block) which requires JDK 21 at runtime
while the plugin (<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId> version 3.1.0) targets Java 17;
fix by either updating your build/runtime JDK to 21 so the formatter can run, or
change the <googleJavaFormat><version> value to a pre-1.29.x release compatible
with Java 17 (e.g., 1.28.x) and keep the rest of the Spotless configuration
intact; update whichever option you choose consistently in the pom where
<googleJavaFormat> version is declared.
🧹 Nitpick comments (2)
src/main/java/io/apitally/spring/ApitallyProperties.java (1)

54-54: Annotation placement may reduce readability.

Having the @Pattern annotation on the same line as the field declaration is unusual. Typically, annotations are placed on their own line for better readability. However, if this is the output from Spotless with the configured settings, it's acceptable to keep for consistency.

src/main/java/io/apitally/common/ApitallyClient.java (1)

225-271: Avoid blocking the common pool in sendHubRequest.
CompletableFuture.supplyAsync without an executor runs on the ForkJoinPool common pool, but the body uses blocking HttpClient.send. Consider a dedicated executor (and shutting it down in shutdown) or switching to sendAsync to prevent pool starvation under load.

♻️ Example adjustment
-        return CompletableFuture.supplyAsync(
-                () -> {
+        return CompletableFuture.supplyAsync(
+                () -> {
                     // ...
-                });
+                }, hubRequestExecutor);

Comment on lines +115 to +129
<plugin>
<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId>
<version>3.1.0</version>
<configuration>
<java>
<googleJavaFormat>
<version>1.33.0</version>
<style>AOSP</style>
</googleJavaFormat>
<removeUnusedImports />
<formatAnnotations />
</java>
</configuration>
</plugin>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

com.diffplug.spotless spotless-maven-plugin latest version 2024 2025

💡 Result:

  • Latest released in 2024: spotless-maven-plugin 2.43.0 (released Jan 23, 2024). [1]

  • Latest released in 2025: spotless-maven-plugin 2.46.0 (released Jul 20, 2025). [2]

Sources: Maven Central listings for com.diffplug.spotless:spotless-maven-plugin. [1] [2]

References:
[1] https://repo.maven.apache.org/maven2/com/diffplug/spotless/spotless-maven-plugin/
[2] https://repo.maven.apache.org/maven2/com/diffplug/spotless/spotless-maven-plugin/2.46.0/


🌐 Web query:

google-java-format 1.33.0 Java 17 compatibility requirements

💡 Result:

Short answer: google-java-format 1.33.0 can format Java 17 source, but the formatter itself requires a newer JDK to run — the project raised the minimum JDK to run google-java-format to JDK 21 (change introduced in v1.29.0). See the README note that the minimum runtime is declared in core/pom.xml and the v1.29.0 release note. [1][2]

Sources:

  • google-java-format README (notes about minimum Java in core/pom.xml / usage). [1]
  • google-java-format releases (v1.29.0: "The minimum support JDK version to run google-java-format is now JDK 21"). [2]

🌐 Web query:

spotless-maven-plugin 3.1.0 release date Java compatibility

💡 Result:

  • Release date: spotless-maven-plugin 3.1.0 was published to Maven Central on 2025-11-18. [1]

  • Java compatibility: Spotless bumped its required Java to Java 17 in the 3.0.0 series, so 3.1.0 requires Java 17 (or newer). [2]

Sources:
[1] Maven Central (com.diffplug.spotless:spotless-maven-plugin — 3.1.0 listing).
[2] diffplug/spotless GitHub releases — note for Maven/3.0.0: "Bump the required Java to 17."


Incompatible JDK requirement: google-java-format 1.33.0 requires JDK 21 to run.

The plugin version 3.1.0 requires Java 17, but google-java-format 1.33.0 (used within it) requires JDK 21 runtime as of v1.29.0. This means your build environment must have JDK 21 available for the formatter to execute, even if compiling with Java 17. Either upgrade to a higher Java version for the build or use an older google-java-format version compatible with Java 17.

🤖 Prompt for AI Agents
In `@pom.xml` around lines 115 - 129, The Spotless config uses google-java-format
version 1.33.0 (inside the <googleJavaFormat> block) which requires JDK 21 at
runtime while the plugin (<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId> version 3.1.0) targets Java 17;
fix by either updating your build/runtime JDK to 21 so the formatter can run, or
change the <googleJavaFormat><version> value to a pre-1.29.x release compatible
with Java 17 (e.g., 1.28.x) and keep the rest of the Spotless configuration
intact; update whichever option you choose consistently in the pom where
<googleJavaFormat> version is declared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants