Skip to content

Server-rest: serialise endpoint route registration in MajordomoRestPlugin #274

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.

Sign up for GitHub

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

Open
t0susnik wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Server-rest: serialise endpoint route registration in MajordomoRestPlugin #274

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 18 additions & 12 deletions server-rest/src/main/java/io/opencmw/server/rest/MajordomoRestPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ public class MajordomoRestPlugin extends BasicMdpWorker {
private static final String TAG_NOTIFY_ID = "notifyID";
private static final AtomicLong REQUEST_COUNTER = new AtomicLong();
private static final AtomicLong NOTIFY_COUNTER = new AtomicLong();
// Javalin's route table is not thread-safe, and RestServer.getInstance() is a single JVM-wide Javalin
// instance shared by every MajordomoRestPlugin instance - static, deliberately, to serialise route
// registration across all of them, not just de-duplicate per key like 'registeredEndpoints' does.
private static final Object ROUTE_REGISTRATION_LOCK = new Object();
private final transient Javalin restInstance = RestServer.getInstance(); // NOSONAR NOPMD -- transient is necessary

static {
Expand Down Expand Up @@ -333,7 +337,7 @@ protected void reconnectToBroker() {
}

protected void registerEndPoint(final String endpoint) {
// needs to be synchronised since Javalin get(..), put(..) seem to be not thread safe (usually initialised during startup)
// 'registeredEndpoints' de-duplicates repeated registration of the same endpoint
registeredEndpoints.computeIfAbsent(endpoint, ep -> {
final var requestMsg = new MdpMessage(null, PROT_CLIENT, GET_REQUEST, INTERNAL_SERVICE_OPENAPI.getBytes(UTF_8), EMPTY_FRAME, URI.create(INTERNAL_SERVICE_OPENAPI), ep.getBytes(UTF_8), "", RBAC);
final CustomFuture<MdpMessage> openApiReply = dispatchRequest(requestMsg);
Expand All @@ -348,19 +352,21 @@ protected void registerEndPoint(final String endpoint) {
var openApi = getOpenApiDocumentation(handlerClassName);

final Set<Role> accessRoles = RestServer.getDefaultRole();
restInstance.routes(() -> {
ApiBuilder.before(ep, restCtx -> {
// for some strange reason this needs to be executed to be able to read 'restCtx.formParamMap()'
if ("POST".equals(restCtx.method())) {
final Map<String, List<String>> map = restCtx.formParamMap();
if (map.size() == 0) {
LOGGER.atDebug().addArgument(restCtx.req.getPathInfo()).log("{} called without form data");
synchronized (ROUTE_REGISTRATION_LOCK) {
restInstance.routes(() -> {
ApiBuilder.before(ep, restCtx -> {
// for some strange reason this needs to be executed to be able to read 'restCtx.formParamMap()'
if ("POST".equals(restCtx.method())) {
final Map<String, List<String>> map = restCtx.formParamMap();
if (map.isEmpty()) {
LOGGER.atDebug().addArgument(restCtx.req.getPathInfo()).log("{} called without form data");
}
}
}
});
post(ep + "*", OpenApiBuilder.documented(openApi, getDefaultServiceRestHandler(ep)), accessRoles);
get(ep + "*", OpenApiBuilder.documented(openApi, getDefaultServiceRestHandler(ep)), accessRoles);
});
post(ep + "*", OpenApiBuilder.documented(openApi, getDefaultServiceRestHandler(ep)), accessRoles);
get(ep + "*", OpenApiBuilder.documented(openApi, getDefaultServiceRestHandler(ep)), accessRoles);
});
}

return openApi;
} catch (final Exception e) { // NOSONAR NOPMD -- erroneous worker replies shall not stop the broker
Expand Down
Loading