Skip to content
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.

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

Backstopper 2.0 - Refactor code from javax ecosystem to jakarta, drop old modules, and pin to java 17 #72

Merged
merged 42 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
3b24eca
Migrate backstopper-core from javax to jakarta
nicmunroe Sep 9, 2024
ad6842d
Migrate backstopper-custom-validators from javax to jakarta
nicmunroe Sep 9, 2024
7141e0b
Migrate backstopper-reusable-tests-junit5 from javax to jakarta
nicmunroe Sep 9, 2024
0535b53
Migrate backstopper-jackson from javax to jakarta
nicmunroe Sep 9, 2024
569ad30
Migrate backstopper-servlet-api from javax to jakarta
nicmunroe Sep 9, 2024
7ae8f41
Make changes to support running builds on java 17
nicmunroe Sep 9, 2024
c696a5d
Switch to java 17
nicmunroe Sep 9, 2024
5871421
Migrate backstopper-spring-web from javax to jakarta
nicmunroe Sep 10, 2024
12552db
Remove the junit4 based backstopper-reusable-tests library
nicmunroe Sep 10, 2024
d8af9d3
Migrate backstopper-spring-web-mvc from javax to jakarta
nicmunroe Sep 10, 2024
45f600f
Delete jaxrs and jersey support
nicmunroe Sep 10, 2024
3a2bd73
Fix for spring-web
nicmunroe Sep 10, 2024
f0390db
Add back spring webmvc sample app
nicmunroe Sep 10, 2024
7621ee6
Migrate backstopper-spring-web-flux from javax to jakarta and move co…
nicmunroe Sep 11, 2024
c8de8cf
Fix logic for missing or bad type conversion on headers or query para…
nicmunroe Sep 11, 2024
05f0ea5
Convert springboot 2 webflux sample app for springboot 3
nicmunroe Sep 11, 2024
3ab4372
Add new classname to spring-web for spring's NoResourceFoundException…
nicmunroe Sep 11, 2024
544a6ef
Migrate backstopper-spring-boot3-webmvc from springboot2 and from jav…
nicmunroe Sep 11, 2024
3547e4e
Adjust backstopper-spring-web-mvc SpringContainerErrorController to d…
nicmunroe Sep 11, 2024
7d3f736
Convert springboot 2 webmvc sample app for springboot 3
nicmunroe Sep 11, 2024
104c931
Delete springboot 1 stuff
nicmunroe Sep 11, 2024
c99d6c6
Remove spring 4 and springboot 1 testonly modules
nicmunroe Sep 11, 2024
991314a
Migrate testonly-spring-reusable-test-support from javax to jakarta, …
nicmunroe Sep 11, 2024
0c99819
Make a split between spring 6.0 and 6.1 and add a testonly module for…
nicmunroe Sep 11, 2024
6120b16
Make spring-reusable-test-support mvc explicit and add testonly-sprin…
nicmunroe Sep 12, 2024
304b1bd
Create spring-webflux-reusable-test-support module for webflux-based …
nicmunroe Sep 12, 2024
e89e211
Minor dependency and docs cleanup in samples
nicmunroe Sep 12, 2024
a324abb
Add testonly modules for springboot 3.0.x for webmvc and webflux
nicmunroe Sep 12, 2024
266a84c
Add testonly modules for springboot 3.1.x for webmvc and webflux
nicmunroe Sep 12, 2024
7d18bd9
Add testonly modules for springboot 3.2.x for webmvc and webflux
nicmunroe Sep 12, 2024
d7ac08d
Clean up springboot 1 and 2 references, and bring disconnected-client…
nicmunroe Sep 12, 2024
d7417d5
Reformat readme and user guide to get rid of long lines
nicmunroe Sep 12, 2024
43307fc
Update readme and user guide after the javax-to-jakarta refactoring
nicmunroe Sep 12, 2024
6d966da
Optimize imports
nicmunroe Sep 12, 2024
84cebd3
IDE automated cleanup
nicmunroe Sep 12, 2024
c440f74
Clean up some warnings, mostly backstopper-core
nicmunroe Sep 13, 2024
c5c8555
More warning cleanups
nicmunroe Sep 13, 2024
e42cd2c
Clean up more warnings
nicmunroe Sep 13, 2024
8786e32
Fix gradle warnings for gradle 9
nicmunroe Sep 13, 2024
b50f901
Update dependencies
nicmunroe Sep 13, 2024
35b07da
Update GHA workflow
nicmunroe Sep 13, 2024
9f385ee
Relax project code coverage requirements a little
nicmunroe Sep 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion .codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ coverage:
project:
default:
enabled: yes
target: 90%
target: 85%
patch:
default:
enabled: yes
Expand Down
15 changes: 8 additions & 7 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Set up JDK 11
uses: actions/setup-java@v2
- uses: actions/checkout@v4
- name: Set up JDK 17
uses: actions/setup-java@v4
with:
java-version: 11
distribution: 'temurin'
- name: Validate Gradle wrapper
uses: gradle/wrapper-validation-action@e6e38bacfdf1a337459f332974bb2327a31aaf4b
java-version: '17'
cache: 'gradle'
- name: Validate Gradle Wrapper Checksums
uses: gradle/actions/wrapper-validation@v3
- name: Build with Gradle
run: ./gradlew clean build
- name: Upload coverage report to CodeCov
Expand All @@ -29,7 +30,7 @@ jobs:
verbose: true
token: ${{ secrets.CODECOV_TOKEN }}
- name: Upload reports and test results to GitHub
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
name: reports-and-test-results
path: |
Expand Down
290 changes: 200 additions & 90 deletions README.md

Large diffs are not rendered by default.

476 changes: 373 additions & 103 deletions USER_GUIDE.md

Large diffs are not rendered by default.

27 changes: 19 additions & 8 deletions backstopper-core/README.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
# Backstopper - core

Backstopper is a framework-agnostic API error handling and (optional) model validation solution for Java 7 and greater.
Backstopper is a framework-agnostic API error handling and (optional) model validation solution for Java 17 and greater.

This `backstopper-core` library contains the key core components necessary for a Backstopper system to work. The [base project README.md](../README.md) and [User Guide](../USER_GUIDE.md) contain the main bulk of information regarding Backstopper, but in particular:

* [Backstopper key components](../USER_GUIDE.md#key_components) - This describes the main classes contained in this core library and what they are for. See the source code and javadocs on classes for further information.
* [Framework-specific modules](../README.md#framework_modules) - The list of specific framework plugin libraries Backstopper currently has support for.
* [Sample applications](../README.md#samples) - The list of sample applications demonstrating how to integrate and use Backstopper in the various supported frameworks.
* [Creating new framework integrations](../USER_GUIDE.md#new_framework_integrations) - Information on how to create new framework integrations.
(NOTE: The [Backstopper 1.x branch](https://github.com/Nike-Inc/backstopper/tree/v1.x) contains a version of
Backstopper for Java 7+, and for the `javax` ecosystem. The current Backstopper supports Java 17+ and the `jakarta`
ecosystem.)

This `backstopper-core` library contains the key core components necessary for a Backstopper system to work.
The [base project README.md](../README.md) and [User Guide](../USER_GUIDE.md) contain the main bulk of information
regarding Backstopper, but in particular:

* [Backstopper key components](../USER_GUIDE.md#key_components) - This describes the main classes contained in this core
library and what they are for. See the source code and javadocs on classes for further information.
* [Framework-specific modules](../README.md#framework_modules) - The list of specific framework plugin libraries
Backstopper currently has support for.
* [Sample applications](../README.md#samples) - The list of sample applications demonstrating how to integrate and use
Backstopper in the various supported frameworks.
* [Creating new framework integrations](../USER_GUIDE.md#new_framework_integrations) - Information on how to create new
framework integrations.

## More Info

See the [base project README.md](../README.md), [User Guide](../USER_GUIDE.md), and Backstopper repository source code and javadocs for all further information.
See the [base project README.md](../README.md), [User Guide](../USER_GUIDE.md), and Backstopper repository source code
and javadocs for all further information.

## License

Expand Down
10 changes: 2 additions & 8 deletions backstopper-core/build.gradle
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
evaluationDependsOn(':')

compileTestJava {
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
}

dependencies {
api(
project(":nike-internal-util"),
"org.slf4j:slf4j-api:$slf4jVersion",
"javax.inject:javax.inject:$javaxInjectVersion",
"javax.validation:validation-api:$javaxValidationVersion"
"jakarta.inject:jakarta.inject-api:$jakartaInjectVersion",
"jakarta.validation:jakarta.validation-api:$jakartaValidationVersion"
)
compileOnly(
"org.jetbrains:annotations:$jetbrainsAnnotationsVersion"
Expand All @@ -24,6 +19,5 @@ dependencies {
"org.assertj:assertj-core:$assertJVersion",
"com.tngtech.java:junit-dataprovider:$junitDataproviderVersion",
"org.hamcrest:hamcrest-all:$hamcrestVersion",
"org.hibernate:hibernate-validator:$hibernateValidatorVersion"
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.nike.backstopper.util.ApiErrorUtil;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

/**
Expand Down Expand Up @@ -41,8 +40,8 @@ public ApiErrorBase(String name, String errorCode, String message, int httpStatu
}

this.metadata = (metadata.isEmpty())
? Collections.<String, Object>emptyMap()
: Collections.unmodifiableMap(new HashMap<>(metadata));
? Collections.emptyMap()
: Map.copyOf(metadata);
}

public ApiErrorBase(String name, int errorCode, String message, int httpStatusCode, Map<String, Object> metadata) {
Expand Down Expand Up @@ -106,6 +105,7 @@ public Map<String, Object> getMetadata() {
}

@Override
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
public boolean equals(Object o) {
return ApiErrorUtil.isApiErrorEqual(this, o);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public int getHttpStatusCode() {
}

@Override
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
public boolean equals(Object o) {
return ApiErrorUtil.isApiErrorEqual(this, o);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ public List<Integer> getStatusCodePriorityOrder() {
* the possibility of null being returned and have a strategy for picking a winner.
*/
public Integer determineHighestPriorityHttpStatusCode(Collection<ApiError> apiErrors) {
if (apiErrors == null || apiErrors.size() == 0) {
if (apiErrors == null || apiErrors.isEmpty()) {
return null;
}

Expand Down Expand Up @@ -368,7 +368,7 @@ public Integer determineHighestPriorityHttpStatusCode(Collection<ApiError> apiEr
logger.error(
"None of the HTTP status codes in the ApiErrors passed to determineHighestPriorityHttpStatusCode() were"
+ " found in the getStatusCodePriorityOrder() list. Offending set of http status codes (these should be "
+ "added to the getStatusCodePriorityOrder() list for this project): " + validStatusCodePossibilities
+ "added to the getStatusCodePriorityOrder() list for this project): {}", validStatusCodePossibilities
);

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

/**
* A sample/example of some core errors that many APIs are likely to need. Any given {@link ProjectApiErrors} could
* return these as its {@link ProjectApiErrors#getCoreApiErrors()} if the error codes and messages associated with these
* return these as its {@code ProjectApiErrors#getCoreApiErrors()} if the error codes and messages associated with these
* are fine for your project (see {@link SampleProjectApiErrorsBase} for a base implementation that does exactly that).
*
* <p>In practice most organizations should copy/paste this class and customize the error codes and messages for their
Expand Down Expand Up @@ -64,7 +64,7 @@ public enum SampleCoreApiError implements ApiError {

SampleCoreApiError(int errorCode, String message, int httpStatusCode) {
this(new ApiErrorBase(
"delegated-to-enum-wrapper-" + UUID.randomUUID().toString(), errorCode, message, httpStatusCode
"delegated-to-enum-wrapper-" + UUID.randomUUID(), errorCode, message, httpStatusCode
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
public abstract class SampleProjectApiErrorsBase extends ProjectApiErrors {

private static final List<ApiError> SAMPLE_CORE_API_ERRORS_AS_LIST =
Arrays.<ApiError>asList(SampleCoreApiError.values());
Arrays.asList(SampleCoreApiError.values());

@Override
protected List<ApiError> getCoreApiErrors() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ protected static String extractMessage(ApiError error) {
}

/**
* Extracts and joins all messages from the input List<{@link ApiError}> if the desired message is null.
*
* Extracts and joins all messages from the input List&lt;{@link ApiError}> if the desired message is null.
* <p>
* Will return null if the input error List is null
*/
protected static String extractMessage(List<ApiError> apiErrors, String desiredMessage) {
Expand Down Expand Up @@ -253,9 +253,9 @@ public StackTraceLoggingBehavior getStackTraceLoggingBehavior() {
*/
@SuppressWarnings("WeakerAccess")
public static class Builder {
private List<ApiError> apiErrors = new ArrayList<>();
private List<Pair<String, String>> extraDetailsForLogging = new ArrayList<>();
private List<Pair<String, List<String>>> extraResponseHeaders = new ArrayList<>();
private final List<ApiError> apiErrors = new ArrayList<>();
private final List<Pair<String, String>> extraDetailsForLogging = new ArrayList<>();
private final List<Pair<String, List<String>>> extraResponseHeaders = new ArrayList<>();
private String message;
private Throwable cause;
private StackTraceLoggingBehavior stackTraceLoggingBehavior;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import java.util.List;

import javax.validation.ConstraintViolation;
import jakarta.validation.ConstraintViolation;

/**
* A runtime exception representing a <b>CLIENT DATA</b> JSR 303 validation failure (i.e. a validation error with data
Expand Down Expand Up @@ -51,8 +51,8 @@ public List<ConstraintViolation<Object>> getViolations() {

/**
* @return The validation groups that were used to do the validation, if any. This may be null or empty; if this is
* null/empty it means the {@link javax.validation.groups.Default} validation group was used as per
* {@link javax.validation.Validator#validate(Object, Class[])} (i.e. no groups were passed in to that
* null/empty it means the {@link jakarta.validation.groups.Default} validation group was used as per
* {@link jakarta.validation.Validator#validate(Object, Class[])} (i.e. no groups were passed in to that
* method).
*/
public Class<?>[] getValidationGroups() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import java.util.Set;

import javax.validation.ConstraintViolation;
import jakarta.validation.ConstraintViolation;

/**
* A runtime exception representing a <b>SERVERSIDE</b> JSR 303 validation failure (i.e. a validation error with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,7 @@ protected ErrorResponseInfo<T> doHandleApiException(
// Add connection type to our extra logging data if appropriate. This particular log message is here so it can
// be done in one spot rather than trying to track down all the different places we're handling
// NetworkExceptionBase subclasses (and possibly missing some by accident).
if (coreException instanceof NetworkExceptionBase) {
NetworkExceptionBase neb = ((NetworkExceptionBase)coreException);
if (coreException instanceof NetworkExceptionBase neb) {
extraDetailsForLogging.add(Pair.of("connection_type", neb.getConnectionType()));
}

Expand All @@ -294,20 +293,20 @@ protected ErrorResponseInfo<T> doHandleApiException(

// Bulletproof against somehow getting a completely empty collection of client errors. This should never happen
// but if it does we want a reasonable response.
if (filteredClientErrors == null || filteredClientErrors.size() == 0) {
if (filteredClientErrors == null || filteredClientErrors.isEmpty()) {
ApiError genericServiceError = projectApiErrors.getGenericServiceError();
UUID trackingUuid = UUID.randomUUID();
String trackingLogKey = "bad_handler_logic_tracking_uuid";
extraDetailsForLogging.add(Pair.of(trackingLogKey, trackingUuid.toString()));
logger.error(String.format(
logger.error(
"Found a situation where we ended up with 0 ApiErrors to return to the client. This should not happen "
+ "and likely indicates a logic error in ApiExceptionHandlerBase, or a ProjectApiErrors that isn't "
+ "setup properly. Defaulting to " + genericServiceError.getName() + " for now, but this should be "
+ "investigated and fixed. Search for %s=%s in the logs to find the log message that contains the "
+ "setup properly. Defaulting to {} for now, but this should be "
+ "investigated and fixed. Search for {}={} in the logs to find the log message that contains the "
+ "details of the request along with the full stack trace of the original exception. "
+ "unfiltered_api_errors=%s",
trackingLogKey, trackingUuid.toString(), utils.concatenateErrorCollection(clientErrors)
));
+ "unfiltered_api_errors={}",
genericServiceError.getName(), trackingLogKey, trackingUuid, utils.concatenateErrorCollection(clientErrors)
);
filteredClientErrors = Collections.singletonList(genericServiceError);
highestPriorityStatusCode = genericServiceError.getHttpStatusCode();
}
Expand Down Expand Up @@ -376,8 +375,11 @@ protected ErrorResponseInfo<T> doHandleApiException(
*/
@SuppressWarnings("UnusedParameters")
protected boolean shouldLogStackTrace(
int statusCode, Collection<ApiError> filteredClientErrors, Throwable originalException,
Throwable coreException, RequestInfoForLogging request
int statusCode,
@SuppressWarnings("unused") Collection<ApiError> filteredClientErrors,
@SuppressWarnings("unused") Throwable originalException,
Throwable coreException,
@SuppressWarnings("unused") RequestInfoForLogging request
) {
if (coreException instanceof ApiException) {
// See if this ApiException is explicitly requesting stack trace logging to be forced on or off.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
import java.util.Set;
import java.util.UUID;

import javax.inject.Named;
import javax.inject.Singleton;
import jakarta.inject.Named;
import jakarta.inject.Singleton;

/**
* Set of reusable utility methods used by the API exception handling chain
Expand Down Expand Up @@ -211,21 +211,21 @@ public String buildErrorMessageForLogs(StringBuilder sb, RequestInfoForLogging r
}

protected @Nullable Object extractOrigErrorRequestUriAttr(@NotNull RequestInfoForLogging request) {
// Corresponds to javax.servlet.RequestDispatcher.ERROR_REQUEST_URI.
return request.getAttribute("javax.servlet.error.request_uri");
// Corresponds to jakarta.servlet.RequestDispatcher.ERROR_REQUEST_URI.
return request.getAttribute("jakarta.servlet.error.request_uri");
}

protected @Nullable Object extractOrigForwardedRequestUriAttr(@NotNull RequestInfoForLogging request) {
// Corresponds to javax.servlet.RequestDispatcher.FORWARD_REQUEST_URI.
Object forwardedRequestUriAttr = request.getAttribute("javax.servlet.forward.request_uri");
// Corresponds to jakarta.servlet.RequestDispatcher.FORWARD_REQUEST_URI.
Object forwardedRequestUriAttr = request.getAttribute("jakarta.servlet.forward.request_uri");

if (forwardedRequestUriAttr != null) {
return forwardedRequestUriAttr;
}

// The forwarded request URI attr was null. Try the path info attr as a last resort.
// Corresponds to javax.servlet.RequestDispatcher.FORWARD_PATH_INFO.
return request.getAttribute("javax.servlet.forward.path_info");
// Corresponds to jakarta.servlet.RequestDispatcher.FORWARD_PATH_INFO.
return request.getAttribute("jakarta.servlet.forward.path_info");
}

/**
Expand Down Expand Up @@ -299,8 +299,8 @@ public String parseSpecificHeaderToString(RequestInfoForLogging request, String


/**
* @param sensitiveHeaders
* @param headerName
* @param sensitiveHeaders The set of sensitive headers names.
* @param headerName The header name in question.
* @return Returns true if the header name is one of the sensitive headers in lower, upper or camel case.
*/
private static boolean containsCaseInSensitive(Set<String> sensitiveHeaders, String headerName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@
try {
body = request.getBody();
} catch (RequestInfoForLogging.GetBodyException e) {
logger.warn("Failed to retrieve request_body while handling exception ex=" + ex, e);
logger.warn("Failed to retrieve request_body while handling exception ex={}", ex, e);

Check warning on line 173 in backstopper-core/src/main/java/com/nike/backstopper/handler/UnhandledExceptionHandlerBase.java

View check run for this annotation

Codecov / codecov/patch

backstopper-core/src/main/java/com/nike/backstopper/handler/UnhandledExceptionHandlerBase.java#L173

Added line #L173 was not covered by tests
body = "[ERROR_EXTRACING_BODY]";
}

Expand Down Expand Up @@ -229,7 +229,7 @@
* Note that in many frameworks if the body has already been read once then it cannot be read again,
* so you may have more work to do than just setting this method to return true.
*/
@SuppressWarnings("UnusedParameters")
@SuppressWarnings("unused")
protected boolean logRequestBodyOnUnhandledExceptions(Throwable ex, RequestInfoForLogging request) {
return false;
}
Expand Down
Loading