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

SO-4992: ETag and Cache-Control header support #1270

Merged
merged 18 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
15b71f2
refactor(core): move rate limit configuration to RateLimitConfig class
cmark Mar 8, 2023
50b77ac
feat(core): support setting Cache-Control and ETag headers
cmark Mar 13, 2023
7aaf55c
feat(api): propagate If-None-Match header to core system
cmark Mar 13, 2023
863f584
feat(api): handle NotModifiedException as HTTP 304 Not Modified response
cmark Mar 13, 2023
8fc528e
test: add basic Cache-Control and ETag header test cases
cmark Mar 14, 2023
b700876
chore(api): resolve compile errors in RateLimitConfig after cherry-pick
cmark Mar 27, 2024
f0a7497
feat(index): move ETag value computation to RevisionBranchRef
cmark Mar 27, 2024
e489145
chore: fix license text in cherry-picked files
cmark Mar 27, 2024
221aaea
fix(api): write custom headers immediately into the HTTP response...
cmark Mar 27, 2024
ffe7bf9
test(api): resolve SnomedApiCacheControlTest issues
cmark Mar 28, 2024
d465e45
test: resolve failure in SnomedPartialLoadingApiTest
cmark Mar 28, 2024
cdcbb9a
test(api): add three more test cases to verify ETag and IfNoneMatch...
cmark Mar 28, 2024
4cc651a
chore(test): use existing test helper to assert Location header presence
cmark Mar 28, 2024
a1186ed
chore(api): use RestController annotation instead of simple Controller
cmark Mar 28, 2024
f67bb59
fix(api): merge headers coming from promise response and from...
cmark Mar 28, 2024
ad781a7
chore(api): change since tag to 9.2
cmark Apr 2, 2024
984789e
chore(deps): upgrade to bucket4j 8.10.1
cmark Apr 2, 2024
d7515ae
feat(api): introduce `api.rate_limit.capacity` configuration...
cmark Apr 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion commons/com.b2international.commons/.classpath
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
</attributes>
</classpathentry>
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry exported="true" kind="lib" path="lib/bucket4j-core-8.7.0.jar"/>
<classpathentry exported="true" kind="lib" path="lib/bucket4j-core-8.10.1.jar"/>
<classpathentry exported="true" kind="lib" path="lib/zjsonpatch-0.4.16.jar"/>
<classpathentry exported="true" kind="lib" path="lib/jgrapht-core-1.5.2.jar"/>
<classpathentry exported="true" kind="lib" path="lib/jheaps-0.14.jar"/>
Expand Down
2 changes: 1 addition & 1 deletion commons/com.b2international.commons/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ Import-Package: jakarta.el;version="[5.0.1,6.0.0)",
jakarta.validation.valueextraction;version="[3.0.2,4.0.0)",
org.slf4j;version="2.0.0"
Bundle-ClassPath: .,
lib/bucket4j-core-8.7.0.jar,
lib/bucket4j-core-8.10.1.jar,
lib/zjsonpatch-0.4.16.jar,
lib/jgrapht-core-1.5.2.jar,
lib/jheaps-0.14.jar,
Expand Down
4 changes: 2 additions & 2 deletions commons/com.b2international.commons/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

<properties>
<!-- Rate limiting -->
<bucket4j.version>8.7.0</bucket4j.version>
<bucket4j.version>8.10.1</bucket4j.version>
<!-- RFC 6902 JSON Patch implementation - zjsonpatch requires Jackson 2.14+ and Commons Collections 4.4 -->
<zjsonpatch.version>0.4.16</zjsonpatch.version>
<!-- Graph algorithms lib -->
Expand Down Expand Up @@ -123,7 +123,7 @@
jboss-logging,
picocli,
jsr305,
javax.annotation-api,
javax.annotation-api
</includeArtifactIds>
<outputDirectory>${basedir}/lib</outputDirectory>
</configuration>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright 2024 B2i Healthcare, https://b2ihealthcare.com
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.b2international.commons.exceptions;

/**
* @since 9.2
*/
public class NotModifiedException extends ApiException {

private static final long serialVersionUID = 1L;

public NotModifiedException() {
super("");
}

@Override
protected Integer getStatus() {
return 304;

Check warning on line 31 in commons/com.b2international.commons/src/com/b2international/commons/exceptions/NotModifiedException.java

View check run for this annotation

Codecov / codecov/patch

commons/com.b2international.commons/src/com/b2international/commons/exceptions/NotModifiedException.java#L31

Added line #L31 was not covered by tests
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@
import com.b2international.index.query.Expression;
import com.b2international.index.query.Expressions;
import com.b2international.index.query.Expressions.ExpressionBuilder;
import com.google.common.base.Charsets;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.hash.Hashing;

/**
* Reference to a set of revision branch segments to query the contents visible from the branch at a given time.
Expand Down Expand Up @@ -218,4 +220,11 @@ public RevisionBranchRef restrictTo(long timestamp) {
.collect(Collectors.toCollection(TreeSet::new)), deletedBranch);
}

/**
* @return an ETag value for this branch reference
*/
public String eTag() {
return Hashing.murmur3_128().hashString(toString(), Charsets.UTF_8).toString();
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 B2i Healthcare, https://b2ihealthcare.com
* Copyright 2023-2024 B2i Healthcare, https://b2ihealthcare.com
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,8 +33,8 @@
import com.b2international.snowowl.core.ApplicationContext;
import com.b2international.snowowl.core.Resources;
import com.b2international.snowowl.core.events.util.Response;
import com.b2international.snowowl.core.rate.ApiConfiguration;
import com.b2international.snowowl.core.rate.ApiPlugin;
import com.b2international.snowowl.core.rate.RateLimitConfig;
import com.b2international.snowowl.core.rate.RateLimitConsumption;
import com.b2international.snowowl.core.rate.RateLimiter;
import com.b2international.snowowl.core.request.ResourceRequests;
Expand All @@ -55,11 +55,11 @@ public class RateLimitTest {
public void setup() throws Exception {
// inject rate limit feature into the system until we run the rate limit tests
Environment env = ApplicationContext.getServiceForClass(Environment.class);
ApiConfiguration apiConfig = new ApiConfiguration();
RateLimitConfig rateLimitConfig = new RateLimitConfig();
// this means that the user is able to perform 2 requests at a time, with one second refill rate (default value, but just in case fix it here as well)
apiConfig.setOverdraft(2L);
apiConfig.setRefillRate(1L);
new ApiPlugin().initRateLimiter(env, apiConfig);
rateLimitConfig.setCapacity(2L);
rateLimitConfig.setRefillRate(1L);
new ApiPlugin().initRateLimiter(env, rateLimitConfig);
this.rateLimiter = env.service(RateLimiter.class);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ public RestApiError handle(final MissingPathVariableException e) {
return RestApiError.of(ApiError.builder("Missing path parameter: '" + e.getVariableName() + "'.").build()).build(HttpStatus.BAD_REQUEST.value());
}

@ExceptionHandler
@ResponseStatus(HttpStatus.NOT_MODIFIED)
public ResponseEntity<Void> handle(final NotModifiedException e) {
return new ResponseEntity<>(HttpStatus.NOT_MODIFIED);
}

@ExceptionHandler
@ResponseStatus(HttpStatus.UNAUTHORIZED)
public ResponseEntity<RestApiError> handle(final UnauthorizedException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,21 @@ public IdentityProvider identityProvider() {
@Scope(scopeName = WebApplicationContext.SCOPE_REQUEST, proxyMode = ScopedProxyMode.INTERFACES)
public Provider<IEventBus> eventBus(@Autowired HttpServletRequest request) {
final String authorization = extractAuthorizationToken(request);
return () -> new AuthorizedEventBus(ApplicationContext.getInstance().getServiceChecked(IEventBus.class), ImmutableMap.of(HttpHeaders.AUTHORIZATION, authorization));
ImmutableMap.Builder<String, String> headers = ImmutableMap.builder();
headers.put(HttpHeaders.AUTHORIZATION, authorization);

if (!Strings.isNullOrEmpty(request.getHeader(ApiConfiguration.IF_NONE_MATCH_HEADER))) {
headers.put(ApiConfiguration.IF_NONE_MATCH_HEADER, request.getHeader(ApiConfiguration.IF_NONE_MATCH_HEADER));
}

return () -> new AuthorizedEventBus(ApplicationContext.getInstance().getServiceChecked(IEventBus.class), headers.build());
}

/*
* Prefer Authorization header content, but allow token query parameter as well.
*/
private String extractAuthorizationToken(HttpServletRequest request) {
String authorizationToken = request.getHeader("Authorization");
String authorizationToken = request.getHeader(HttpHeaders.AUTHORIZATION);
if (Strings.isNullOrEmpty(authorizationToken)) {
authorizationToken = request.getParameter("token");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@
*/
package com.b2international.snowowl.core.rest.admin;

import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.RestController;

import com.b2international.snowowl.core.ServerInfo;
import com.b2international.snowowl.core.events.util.Promise;
Expand All @@ -33,7 +32,7 @@
* @since 5.8
*/
@Tag(description="Administration", name = CoreApiConfig.ADMINISTRATION)
@Controller
@RestController
@RequestMapping(value = "/info")
public class ServerInfoRestService extends AbstractRestService {

Expand All @@ -42,7 +41,7 @@ public class ServerInfoRestService extends AbstractRestService {
description="Retrieves information about the running server, including version, available repositories, etc."
)
@RequestMapping(method = { RequestMethod.GET, RequestMethod.HEAD }, produces = { AbstractRestService.JSON_MEDIA_TYPE })
public @ResponseBody Promise<ServerInfo> info() {
public Promise<ServerInfo> info() {
return RepositoryRequests.prepareGetServerInfo()
.buildAsync()
.execute(getBus());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019-2023 B2i Healthcare, https://b2ihealthcare.com
* Copyright 2019-2024 B2i Healthcare, https://b2ihealthcare.com
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,21 +15,27 @@
*/
package com.b2international.snowowl.core.rest.util;

import java.util.Map;
import java.util.Map.Entry;
import java.util.stream.Stream;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.core.MethodParameter;
import org.springframework.http.HttpHeaders;
import org.springframework.http.ResponseEntity;
import org.springframework.http.ResponseEntity.BodyBuilder;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.async.DeferredResult;
import org.springframework.web.context.request.async.WebAsyncUtils;
import org.springframework.web.method.support.AsyncHandlerMethodReturnValueHandler;
import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
import org.springframework.web.method.support.ModelAndViewContainer;

import com.b2international.snowowl.core.api.SnowowlRuntimeException;
import com.b2international.snowowl.core.events.util.Promise;
import com.b2international.snowowl.core.events.util.Response;
import com.google.common.collect.Iterables;

import jakarta.servlet.http.HttpServletResponse;

/**
* @since 7.2
Expand All @@ -54,7 +60,7 @@
final Promise<?> promise = (Promise<?>) returnValue;
final DeferredResult<ResponseEntity<?>> result = new DeferredResult<>();
promise
.thenRespond(promiseResponse -> setDeferredResult(result, promiseResponse))
.thenRespond(promiseResponse -> setDeferredResult(result, promiseResponse, webRequest))
.fail(err -> {
if (result.isSetOrExpired()) {
LOG.warn("Deferred result is already set or expired, could not deliver Throwable.", err);
Expand All @@ -70,36 +76,51 @@
}
}

private Response<?> setDeferredResult(DeferredResult<ResponseEntity<?>> result, Response<?> promiseResponse) {
private Response<?> setDeferredResult(DeferredResult<ResponseEntity<?>> result, Response<?> promiseResponse, NativeWebRequest webRequest) {
if (result.isSetOrExpired()) {
LOG.warn("Deferred result is already set or expired, could not deliver result {}.", promiseResponse);
} else {
} else {

Check warning on line 82 in core/com.b2international.snowowl.core.rest/src/com/b2international/snowowl/core/rest/util/PromiseMethodReturnValueHandler.java

View check run for this annotation

Codecov / codecov/patch

core/com.b2international.snowowl.core.rest/src/com/b2international/snowowl/core/rest/util/PromiseMethodReturnValueHandler.java#L82

Added line #L82 was not covered by tests
final Object body = promiseResponse.getBody();
final ResponseEntity<?> response;

final Stream<Map.Entry<String, String>> responseHeaders;

if (body instanceof ResponseEntity<?> b) {
// return a custom ResponseEntity, copy it and apply headers returned from the system
HttpHeaders headers = b.getHeaders();
// append headers returned from system
promiseResponse.getHeaders().forEach((headerName, headerValue) -> {
headers.set(headerName, headerValue);
});
response = new ResponseEntity<>(b.getBody(), headers, b.getStatusCode());

// returning a standard object as response, with the given status code and without the headers to prevent header duplication
response = ResponseEntity.status(b.getStatusCode()).body(b.getBody());
responseHeaders = Stream.concat(promiseResponse.getHeaders().entrySet().stream(), b.getHeaders().isEmpty() ? Stream.empty() : flattenHttpHeaders(b));
} else {
// returning a standard object as reponse, use HTTP 200 OK
BodyBuilder responseBuilder = ResponseEntity.ok();
// append headers returned from system
promiseResponse.getHeaders().forEach((headerName, headerValue) -> {
responseBuilder.header(headerName, headerValue);
});
response = responseBuilder
.body(body);
// returning a standard object as response, use HTTP 200 OK
response = ResponseEntity.ok().body(body);
responseHeaders = promiseResponse.getHeaders().entrySet().stream();
}

// append headers returned from system directly into the HTTP Response
// see Spring Security issue not being able to properly prevent duplicate caching headers
// https://github.com/spring-projects/spring-security/issues/12865
responseHeaders.forEach((entry) -> {
// XXX using set header here, for most of our use cases we only need a single response header, so overwrite anything that has been injected by Spring earlier
webRequest.getNativeResponse(HttpServletResponse.class).setHeader(entry.getKey(), entry.getValue());
});

result.setResult(response);
}
return null;
}

private Stream<Entry<String, String>> flattenHttpHeaders(ResponseEntity<?> b) {
return b.getHeaders()
.entrySet()
.stream()
.map(entry -> {
// raise an error if we'd like to set a multi-valued HTTP response header
if (entry.getValue().size() > 1) {
throw new SnowowlRuntimeException("Multi-valued response headers are not supported yet");

Check warning on line 118 in core/com.b2international.snowowl.core.rest/src/com/b2international/snowowl/core/rest/util/PromiseMethodReturnValueHandler.java

View check run for this annotation

Codecov / codecov/patch

core/com.b2international.snowowl.core.rest/src/com/b2international/snowowl/core/rest/util/PromiseMethodReturnValueHandler.java#L118

Added line #L118 was not covered by tests
}
return Map.entry(entry.getKey(), Iterables.getFirst(entry.getValue(), null));
});
}

@Override
public boolean supportsReturnType(MethodParameter returnType) {
Class<?> type = returnType.getParameterType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,5 @@ public String toString() {
.append(", children=").append(children).append("]");
return builder.toString();
}



}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020-2023 B2i Healthcare, https://b2ihealthcare.com
* Copyright 2020-2024 B2i Healthcare, https://b2ihealthcare.com
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,16 +15,24 @@
*/
package com.b2international.snowowl.core.context;

import java.util.Objects;

import com.b2international.commons.exceptions.NotModifiedException;
import com.b2international.index.revision.RevisionSearcher;
import com.b2international.snowowl.core.ResourceURI;
import com.b2international.snowowl.core.TerminologyResource;
import com.b2international.snowowl.core.domain.BranchContext;
import com.b2international.snowowl.core.events.DelegatingRequest;
import com.b2international.snowowl.core.events.Request;
import com.b2international.snowowl.core.events.util.RequestHeaders;
import com.b2international.snowowl.core.events.util.ResponseHeaders;
import com.b2international.snowowl.core.rate.ApiConfiguration;
import com.b2international.snowowl.core.request.BranchRealtimeContentRequest;
import com.b2international.snowowl.core.request.BranchSnapshotContentRequest;
import com.b2international.snowowl.core.request.RepositoryRequest;
import com.b2international.snowowl.core.uri.ResourceURIPathResolver;
import com.b2international.snowowl.core.uri.ResourceURIPathResolver.PathWithVersion;
import com.google.common.base.Strings;

/**
* @since 7.5
Expand Down Expand Up @@ -60,7 +68,32 @@ public R execute(TerminologyResourceContext context) {
}

return new RepositoryRequest<R>(resource.getToolingId(),
snapshot ? new BranchSnapshotContentRequest<>(path, next()) : new BranchRealtimeContentRequest<>(path, next())
snapshot ? new BranchSnapshotContentRequest<>(path, nextWithCaching()) : new BranchRealtimeContentRequest<>(path, next())
).execute(context);
}

private Request<BranchContext, R> nextWithCaching() {
return ctx -> {

final String eTag = ctx.service(RevisionSearcher.class).ref().eTag();

// before executing the request, check whether we have an If-None-Match header set in the incoming request
// if yes, check the current ETag with any of the attached values, if none matches evaluate the query otherwise send back HTTP 304
String ifNoneMatchHeaderValue = ctx.service(RequestHeaders.class).header(ApiConfiguration.IF_NONE_MATCH_HEADER);
if (!Strings.isNullOrEmpty(ifNoneMatchHeaderValue) && Objects.equals(ifNoneMatchHeaderValue.replaceAll("\"", ""), eTag)) {
throw new NotModifiedException();
}

R response = next().execute(ctx);

// once we have the response ready calculate Cache-Control and ETag headers
final ApiConfiguration apiConfiguration = ctx.service(ApiConfiguration.class);
final ResponseHeaders responseHeaders = ctx.service(ResponseHeaders.class);
responseHeaders.set(ApiConfiguration.ETAG_HEADER, eTag);
// configure HTTP Cache-Control headers here using the currently configured global api.cache_control value
responseHeaders.set(ApiConfiguration.CACHE_CONTROL_HEADER, apiConfiguration.getCacheControl());

return response;
};
}
}
Loading
Loading