Skip to content

Commit 401cc68

Browse files
objzobjz
committed
fixed reload re-registering commands
1 parent 0d8b734 commit 401cc68

File tree

6 files changed

+182
-274
lines changed

6 files changed

+182
-274
lines changed

‎backends/src/main/java/dev/objz/commandbridge/backends/net/in/RegistrationHandler.java‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import dev.objz.commandbridge.net.proto.MessageType;
1515
import io.undertow.websockets.core.WebSocketChannel;
1616

17+
import java.util.List;
1718
import java.util.Objects;
1819

1920
public final class RegistrationHandler extends InboundHandler {
@@ -33,17 +34,6 @@ public void accept(WebSocketChannel ch, Envelope env) {
3334
} catch (Exception e) {
3435
Log.error(e, "Failed to handle REGISTER_COMMANDS from {}", env.from());
3536
}
36-
if (rc == null || rc.commands() == null || rc.commands().isEmpty()) {
37-
Log.warn("Received empty REGISTER_COMMANDS");
38-
Feedback f = Feedback.empty();
39-
reply(ch, env, MessageType.REGISTER_COMMANDS_RESULT, f)
40-
.dispatch()
41-
.exceptionally(ex -> {
42-
Log.warn("Failed to send FEEDBACK: {}", ex.toString());
43-
return null;
44-
});
45-
return;
46-
}
4737

4838
try {
4939
registry.unregisterAll();
@@ -52,26 +42,37 @@ public void accept(WebSocketChannel ch, Envelope env) {
5242
Log.warn("Failed to unregister previous commands: {}", e.getMessage());
5343
}
5444

45+
if (rc == null || rc.commands() == null || rc.commands().isEmpty()) {
46+
Log.warn("Received empty or malformed REGISTER_COMMANDS. All commands unregistered");
47+
Feedback f = new Feedback(0, 0, 0,
48+
List.of("Received empty registration request"),
49+
List.of());
50+
reply(ch, env, MessageType.REGISTER_COMMANDS_RESULT, f).dispatch().exceptionally(ex -> {
51+
Log.warn("Failed to send FEEDBACK: {}", ex.toString());
52+
return null;
53+
});
54+
return;
55+
}
56+
5557
FeedbackCollector fc = new FeedbackCollector();
5658
for (CommandStub s : rc.commands()) {
5759
try {
5860
registry.register(s);
5961
fc.success();
6062
} catch (Throwable t) {
6163
Log.error(t, "Registration failed for '{}'", s != null ? s.name() : "<null>");
62-
fc.failure("register '" + (s != null ? s.name() : "<null>") + "': " + t.getMessage());
64+
fc.failure("register '" + (s != null ? s.name() : "<null>") + "': "
65+
+ t.getMessage());
6366
}
6467
}
6568
ws.setServerId(env.from());
6669
Feedback f = fc.build();
6770
Summary.feedbackSummary("Registration", f, env.from());
6871
Summary.feedbackDetails(f, env.from(), true);
6972

70-
reply(ch, env, MessageType.REGISTER_COMMANDS_RESULT, f)
71-
.dispatch()
72-
.exceptionally(ex -> {
73-
Log.warn("Failed to send FEEDBACK: {}", ex.toString());
74-
return null;
75-
});
73+
reply(ch, env, MessageType.REGISTER_COMMANDS_RESULT, f).dispatch().exceptionally(ex -> {
74+
Log.warn("Failed to send FEEDBACK: {}", ex.toString());
75+
return null;
76+
});
7677
}
7778
}

‎backends/src/main/java/dev/objz/commandbridge/backends/platform/cmd/CommandRegistry.java‎

Lines changed: 9 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import dev.objz.commandbridge.net.payloads.cmd.CommandStub;
1212
import dev.objz.commandbridge.net.proto.MessageType;
1313
import dev.objz.commandbridge.scripting.model.records.mapping.ArgMapping;
14-
1514
import java.util.ArrayList;
1615
import java.util.HashSet;
1716
import java.util.List;
@@ -21,7 +20,6 @@
2120
public final class CommandRegistry implements CommandRegistryInterface {
2221

2322
private final Set<String> registeredCommands = ConcurrentHashMap.newKeySet();
24-
private final Set<String> pendingUnregister = ConcurrentHashMap.newKeySet();
2523
private final ArgumentMapperInterface<Argument<?>> argumentMapper;
2624
private final OutNode<Object> outNode;
2725
private final Object registrationLock = new Object();
@@ -30,34 +28,16 @@ public CommandRegistry(ArgumentMapperInterface<Argument<?>> mapper, OutNode<Obje
3028
this.argumentMapper = mapper;
3129
this.outNode = outNode;
3230
}
33-
//TODO: fix: commands wont be re registerd when reloading
31+
3432
@Override
3533
public void register(CommandStub stub) throws Exception {
3634
String cmdName = stub.name();
3735

3836
synchronized (registrationLock) {
39-
// If this command is pending unregistration, force complete it
40-
if (pendingUnregister.contains(cmdName)) {
41-
Log.debug("Waiting for command '{}' to finish unregistering", cmdName);
42-
try {
43-
CommandAPI.unregister(cmdName);
44-
} catch (Exception ignored) {
45-
}
46-
pendingUnregister.remove(cmdName);
47-
}
48-
49-
// Double-check if already registered
5037
if (registeredCommands.contains(cmdName)) {
51-
Log.debug("Command '{}' already registered, unregistering first", cmdName);
52-
try {
53-
CommandAPI.unregister(cmdName);
54-
} catch (Exception e) {
55-
Log.warn("Failed to unregister existing command '{}': {}", cmdName,
56-
e.getMessage());
57-
}
38+
Log.debug("Command '{}' is already registered, it will be replaced.", cmdName);
5839
}
5940

60-
// Now register the command
6141
CommandAPICommand cmd = new CommandAPICommand(cmdName);
6242

6343
if (stub.description() != null && !stub.description().isBlank()) {
@@ -91,28 +71,28 @@ public void register(CommandStub stub) throws Exception {
9171

9272
int argCount = stub.args() != null ? stub.args().size() : 0;
9373
int aliasCount = stub.aliases() != null ? stub.aliases().size() : 0;
94-
Log.debug("Registered command '{}' with {} arg(s) and {} alias(es)", cmdName, argCount,
95-
aliasCount);
74+
Log.debug("Registered command '{}' with {} arg(s) and {} alias(es)", cmdName,
75+
argCount, aliasCount);
9676
}
9777
}
9878

9979
@Override
10080
public void unregisterAll() throws Exception {
10181
synchronized (registrationLock) {
82+
if (registeredCommands.isEmpty()) {
83+
return;
84+
}
10285
Set<String> toUnregister = new HashSet<>(registeredCommands);
103-
pendingUnregister.addAll(toUnregister);
104-
86+
Log.info("Unregistering {} command(s)...", toUnregister.size());
10587
for (String cmdName : toUnregister) {
10688
try {
107-
CommandAPI.unregister(cmdName);
89+
CommandAPI.unregister(cmdName, true);
10890
Log.debug("Unregistered command '{}'", cmdName);
10991
} catch (Exception e) {
11092
Log.warn("Failed to unregister command '{}': {}", cmdName, e.getMessage());
11193
}
11294
}
113-
11495
registeredCommands.clear();
115-
pendingUnregister.clear();
11696
}
11797
}
11898
}

‎velocity/src/main/java/dev/objz/commandbridge/velocity/RegistrationManager.java‎

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,7 @@
11
package dev.objz.commandbridge.velocity;
22

33
import static java.util.stream.Collectors.toUnmodifiableSet;
4-
5-
import java.time.Duration;
6-
import java.util.ArrayList;
7-
import java.util.HashMap;
8-
import java.util.HashSet;
9-
import java.util.List;
10-
import java.util.Map;
11-
import java.util.Objects;
12-
import java.util.Set;
13-
import java.util.concurrent.ConcurrentHashMap;
14-
154
import com.velocitypowered.api.proxy.ProxyServer;
16-
175
import dev.objz.commandbridge.config.model.VelocityConfig;
186
import dev.objz.commandbridge.logging.Log;
197
import dev.objz.commandbridge.net.OutNode;
@@ -29,6 +17,15 @@
2917
import dev.objz.commandbridge.velocity.net.out.ctx.RegistrationRequestContext;
3018
import dev.objz.commandbridge.velocity.net.session.ClientSession;
3119
import dev.objz.commandbridge.velocity.net.session.SessionHub;
20+
import java.time.Duration;
21+
import java.util.ArrayList;
22+
import java.util.HashMap;
23+
import java.util.HashSet;
24+
import java.util.List;
25+
import java.util.Map;
26+
import java.util.Objects;
27+
import java.util.Set;
28+
import java.util.concurrent.ConcurrentHashMap;
3229

3330
public final class RegistrationManager {
3431

@@ -40,20 +37,23 @@ public final class RegistrationManager {
4037
private final Map<String, Set<Script>> backendByClient = new ConcurrentHashMap<>();
4138
private final Set<Script> velocityScripts = ConcurrentHashMap.newKeySet();
4239

43-
public RegistrationManager(ProxyServer proxy,
44-
SessionHub sessions,
45-
VelocityConfig config,
40+
public RegistrationManager(ProxyServer proxy, SessionHub sessions, VelocityConfig config,
4641
OutNode<Object> outNode) {
4742
this.sessions = Objects.requireNonNull(sessions);
4843
this.registry = new CommandRegistry(new ArgumentMapper(proxy));
4944
this.outNode = Objects.requireNonNull(outNode);
50-
this.registerTimeout = Duration.ofSeconds(
51-
Objects.requireNonNull(config).timeouts().registerTimeout());
45+
this.registerTimeout = Duration.ofSeconds(Objects.requireNonNull(config).timeouts().registerTimeout());
5246
}
5347

5448
public void load(List<Script> scripts) {
5549
clearState();
5650

51+
try {
52+
registry.unregisterAll();
53+
} catch (Exception e) {
54+
Log.error(e, "Failed to unregister all Velocity commands before reload");
55+
}
56+
5757
if (scripts == null || scripts.isEmpty()) {
5858
Log.warn("No scripts to register");
5959
return;
@@ -68,11 +68,8 @@ public void load(List<Script> scripts) {
6868
continue;
6969
}
7070

71-
var locations = s.register().stream()
72-
.filter(Objects::nonNull)
73-
.map(IdMapping::location)
74-
.filter(Objects::nonNull)
75-
.collect(toUnmodifiableSet());
71+
var locations = s.register().stream().filter(Objects::nonNull).map(IdMapping::location)
72+
.filter(Objects::nonNull).collect(toUnmodifiableSet());
7673

7774
if (locations.contains(Location.VELOCITY)) {
7875
try {
@@ -87,33 +84,35 @@ public void load(List<Script> scripts) {
8784
}
8885

8986
if (locations.contains(Location.BACKEND)) {
90-
s.register().stream()
91-
.filter(m -> m != null && m.location() == Location.BACKEND)
92-
.map(IdMapping::id)
93-
.filter(Objects::nonNull)
87+
s.register().stream().filter(m -> m != null && m.location() == Location.BACKEND)
88+
.map(IdMapping::id).filter(Objects::nonNull)
9489
.forEach(backendId -> backendStaging
9590
.computeIfAbsent(backendId, k -> new ArrayList<>())
9691
.add(s));
9792
}
9893
}
9994

10095
backendStaging.forEach((backendId, list) -> backendByClient.put(backendId, new HashSet<>(list)));
96+
10197
int ok = velocityCollector.ok;
10298
if (ok > 0) {
103-
Log.success(true, "Registered '{}' " + Log.plural(ok, "Velocity command", "Velocity commands"),
99+
Log.success(true,
100+
"Registered '{}' " + Log.plural(ok, "Velocity command", "Velocity commands"),
104101
ok);
105102
}
106103
if (!velocityCollector.errors.isEmpty()) {
107-
Log.error("Failed to register '{}' " + Log.plural(velocityCollector.errors.size(),
108-
"Velocity command", "Velocity commands"),
104+
Log.error("Failed to register '{}' "
105+
+ Log.plural(velocityCollector.errors.size(), "Velocity command",
106+
"Velocity commands"),
109107
velocityCollector.errors.size());
110108
}
111109

112110
if (!backendByClient.isEmpty()) {
113111
int total = backendByClient.values().stream().mapToInt(Set::size).sum();
114112
int clients = backendByClient.size();
115-
Log.success(true, "Prepared '{}' " + Log.plural(total, "backend command", "backend commands")
116-
+ " for '{}' " + Log.plural(clients, "client", "clients"),
113+
Log.success(true,
114+
"Prepared '{}' " + Log.plural(total, "backend command", "backend commands")
115+
+ " for '{}' " + Log.plural(clients, "client", "clients"),
117116
total, clients);
118117
}
119118
}
@@ -130,8 +129,7 @@ public void onClientAuthenticated(ClientSession session) {
130129
return;
131130
}
132131

133-
outNode.send(
134-
MessageType.REGISTER_COMMANDS,
132+
outNode.send(MessageType.REGISTER_COMMANDS,
135133
new RegistrationRequestContext(session, scripts, registerTimeout));
136134
}
137135

@@ -140,8 +138,7 @@ public void reload() {
140138
if (s.status() == AuthStatus.AUTH_OK && s.ch() != null && s.ch().isOpen()) {
141139
var set = backendByClient.get(s.id());
142140
if (set != null && !set.isEmpty()) {
143-
outNode.send(
144-
MessageType.REGISTER_COMMANDS,
141+
outNode.send(MessageType.REGISTER_COMMANDS,
145142
new RegistrationRequestContext(s, set, registerTimeout));
146143
}
147144
}

0 commit comments

Comments
 (0)