Skip to content

Commit

Permalink
chore: add caching to the service endpoints query (#2475)
Browse files Browse the repository at this point in the history
* chore: add caching to the clients endpoints query

Refs: XRDDEV-2764

* docs: add system parameter

Refs: XRDDEV-2764
  • Loading branch information
enelir authored Dec 13, 2024
1 parent 0871b2d commit 92c14a4
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 11 deletions.
4 changes: 3 additions & 1 deletion doc/Manuals/ug-syspar_x-road_v6_system_parameters.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# X-Road: System Parameters User Guide

Version: 2.91
Version: 2.92
Doc. ID: UG-SYSPAR


Expand Down Expand Up @@ -102,6 +102,7 @@ Doc. ID: UG-SYSPAR
| 20.09.2024 | 2.89 | Acme automatic certificate renewal job related parameters | Mikk-Erik Bachmann |
| 08.11.2024 | 2.90 | Added new parameters *key-named-curve*, *soft-token-pin-keystore-algorithm*, *authentication-key-algorithm* and *signing-key-algorithm* to add ECDSA support for Authentication/Signing certificates | Ovidijus Narkevicius |
| 09.12.2024 | 2.91 | Rename parameters *global_conf_tls_cert_verification* -> *global-conf-tls-cert-verification*, *global_conf_hostname_verification* -> *global-conf-hostname-verification* | Eneli Reimets |
| 12.12.2024 | 2.92 | Added new parameter *server-conf-service-endpoints-cache-size* | Eneli Reimets |


## Table of Contents
Expand Down Expand Up @@ -297,6 +298,7 @@ This chapter describes the system parameters used by the components of the X-Roa
| server-conf-client-cache-size | 100 | | | Maximum number of local clients to keep cached |
| server-conf-service-cache-size | 1000 | | | Maximum number of services to keep cached |
| server-conf-acl-cache-size | 100000 | | | Maximum number of access rights to keep cached in memory. |
| server-conf-service-endpoints-cache-size | 100000 | | | Maximum number of service endpoints to keep cached in memory. |
| enforce-client-is-cert-validity-period-check | false | | | Whether to reject a request when client information system certificate is expired or not yet valid. |
| backup-encryption-enabled | false | | | Whether to encrypt security server backup files using server's OpenPGP key. |
| backup-encryption-keyids | | | | Comma-separated list of additional recipient OpenPGP key identifiers. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ private SystemProperties() {

public static final String SERVER_CONF_ACL_CACHE_SIZE = PROXY_PREFIX + "server-conf-acl-cache-size";

public static final String SERVER_CONF_SERVICE_ENDPOINTS_CACHE_SIZE = PROXY_PREFIX + "server-conf-service-endpoints-cache-size";

/** Property name of the idle time that connections to the ServerProxy Connector are allowed, in milliseconds */
private static final String SERVERPROXY_CONNECTOR_MAX_IDLE_TIME =
Expand Down Expand Up @@ -1878,6 +1879,11 @@ public static long getServerConfAclCacheSize() {
return Long.getLong(SERVER_CONF_ACL_CACHE_SIZE, 100_000);
}

@SuppressWarnings("checkstyle:MagicNumber")
public static long getServerConfServiceEndpointsCacheSize() {
return Long.getLong(SERVER_CONF_SERVICE_ENDPOINTS_CACHE_SIZE, 100_000);
}

private static void checkVersionValidity(int min, int current, String defaultVersion) {
if (min > current || min < 1) {
throw new IllegalArgumentException("Illegal minimum global configuration version in system parameters");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import ee.ria.xroad.common.identifier.ClientId;
import ee.ria.xroad.common.identifier.SecurityServerId;
import ee.ria.xroad.common.identifier.ServiceId;
import ee.ria.xroad.common.metadata.Endpoint;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
Expand All @@ -60,11 +61,11 @@ public class CachingServerConfImpl extends ServerConfImpl {

public static final String TSP_URL = "tsp_url";

private final int expireSeconds;
private volatile SecurityServerId.Conf serverId;
private final Cache<Object, List<String>> tspCache;
private final Cache<ServiceId, Optional<ServiceType>> serviceCache;
private final Cache<AclCacheKey, List<EndpointType>> aclCache;
private final Cache<ServiceId, List<Endpoint>> serviceEndpointsCache;
private final Cache<ClientId, Optional<ClientType>> clientCache;
private final Cache<String, InternalSSLKey> internalKeyCache;

Expand All @@ -75,7 +76,7 @@ public class CachingServerConfImpl extends ServerConfImpl {
@SuppressWarnings("checkstyle:MagicNumber")
public CachingServerConfImpl(GlobalConfProvider globalConfProvider) {
super(globalConfProvider);
expireSeconds = SystemProperties.getServerConfCachePeriod();
int expireSeconds = SystemProperties.getServerConfCachePeriod();

internalKeyCache = CacheBuilder.newBuilder()
.maximumSize(1)
Expand Down Expand Up @@ -106,6 +107,12 @@ public CachingServerConfImpl(GlobalConfProvider globalConfProvider) {
.recordStats()
.build();

serviceEndpointsCache = CacheBuilder.newBuilder()
.weigher((ServiceId k, List<Endpoint> v) -> v.size() + 1)
.maximumWeight(SystemProperties.getServerConfServiceEndpointsCacheSize())
.expireAfterWrite(expireSeconds, TimeUnit.SECONDS)
.recordStats()
.build();
}

@Override
Expand Down Expand Up @@ -163,6 +170,19 @@ public String getMemberStatus(ClientId clientId) {
return getClient(clientId).map(ClientType::getClientStatus).orElse(null);
}

@Override
public List<Endpoint> getServiceEndpoints(ServiceId serviceId) {
try {
return serviceEndpointsCache.get(serviceId, () -> super.getServiceEndpoints(serviceId));
} catch (ExecutionException e) {
if (e.getCause() instanceof CodedException codedException) {
throw codedException;
}
log.debug("Failed to get list of service endpoints", e);
return List.of();
}
}

@Override
public IsAuthentication getIsAuthentication(ClientId clientId) {
return getClient(clientId).map(c -> c.getIsAuthentication() == null ? IsAuthentication.NOSSL
Expand Down Expand Up @@ -192,7 +212,7 @@ public DescriptionType getDescriptionType(ServiceId service) {
@Override
public boolean isSslAuthentication(ServiceId service) {
Optional<ServiceType> serviceTypeOptional = getService(service);
if (!serviceTypeOptional.isPresent()) {
if (serviceTypeOptional.isEmpty()) {
throw new CodedException(X_UNKNOWN_SERVICE, "Service '%s' not found", service);
}
ServiceType serviceType = serviceTypeOptional.get();
Expand Down Expand Up @@ -265,6 +285,8 @@ public void logStatistics() {
serviceCache.stats());
log.trace("ServerConf.aclCache : entries: {}, stats: {}", aclCache.size(),
aclCache.stats());
log.trace("ServerConf.serviceEndpointsCache: entries: {}, stats: {}", serviceEndpointsCache.size(),
serviceEndpointsCache.stats());
}
}

Expand All @@ -274,7 +296,6 @@ public void clearCache() {
internalKeyCache.invalidateAll();
}


private record AclCacheKey(ClientId client, ServiceId serviceId) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -363,12 +363,12 @@ public String getServiceDescriptionURL(ServiceId service) {
}

@Override
public List<Endpoint> getServiceEndpoints(ServiceId service) {
return tx(session -> getClient(session, service.getClientId()).getEndpoint().stream()
.filter(e -> e.getServiceCode().equals(service.getServiceCode()))
public List<Endpoint> getServiceEndpoints(ServiceId serviceId) {
return tx(session -> getClient(session, serviceId.getClientId()).getEndpoint().stream()
.filter(e -> e.getServiceCode().equals(serviceId.getServiceCode()))
.filter(e -> !e.isBaseEndpoint())
.map(e -> createEndpoint(e.getMethod(), e.getPath()))
.collect(Collectors.toList()));
.toList());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,10 @@ List<ServiceId.Conf> getAllowedServicesByDescriptionType(ClientId serviceProvide
String getServiceDescriptionURL(ServiceId service);

/**
* @param service the service identifier
* @param serviceId the service identifier
* @return list of endpoints
*/
List<Endpoint> getServiceEndpoints(ServiceId service);
List<Endpoint> getServiceEndpoints(ServiceId serviceId);

/**
* Log serverconf statistics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import ee.ria.xroad.common.identifier.ClientId;
import ee.ria.xroad.common.identifier.SecurityServerId;
import ee.ria.xroad.common.identifier.ServiceId;
import ee.ria.xroad.common.metadata.Endpoint;
import ee.ria.xroad.common.util.CryptoUtils;

import org.junit.After;
Expand Down Expand Up @@ -323,6 +324,14 @@ public void getTsps() throws Exception {
assertEquals(NUM_TSPS, tspUrls.size());
}

@Test
public void getServiceEndpoints() {
ClientId client1 = createTestClientId(client(1));
ServiceId serviceRest = createTestServiceId(client1.getMemberCode(), "rest", null);
List<Endpoint> endpoints = serverConfProvider.getServiceEndpoints(serviceRest);
assertEquals(2, endpoints.size());
}

/**
* Tests getting services.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,12 @@ static ServerConfType createTestData(Session session) {

EndpointType restEndpoint = new EndpointType(service.getServiceCode(), "GET", "/api/**", false);
session.persist(restEndpoint);
client.getEndpoint().add(restEndpoint);
client.getAcl().add(createAccessRight(restEndpoint, client.getIdentifier()));

EndpointType restEndpoint2 = new EndpointType(service.getServiceCode(), "POST", "/api/test/*", false);
session.persist(restEndpoint2);
client.getEndpoint().add(restEndpoint2);
client.getAcl().add(createAccessRight(restEndpoint2, client.getIdentifier()));

LocalGroupType localGroup = new LocalGroupType();
Expand Down

0 comments on commit 92c14a4

Please sign in to comment.