Skip to content

GH-1052 Fix ignore calling synchronous events in async code #1052

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ public interface IgnoreService {

CompletableFuture<Boolean> isIgnored(UUID requester, UUID target);

CompletableFuture<Boolean> ignore(UUID requester, UUID target);
void ignore(UUID requester, UUID target);

CompletableFuture<Boolean> ignoreAll(UUID requester);
void ignoreAll(UUID requester);

CompletableFuture<Boolean> unIgnore(UUID requester, UUID target);
void unIgnore(UUID requester, UUID target);

CompletableFuture<Boolean> unIgnoreAll(UUID requester);
void unIgnoreAll(UUID requester);

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.eternalcode.core.feature.ignore;

import com.eternalcode.annotations.scan.command.DescriptionDocs;
import com.eternalcode.commons.scheduler.Scheduler;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.notice.NoticeService;
import com.eternalcode.core.user.User;
Expand All @@ -19,11 +20,13 @@ class IgnoreCommand {

private final IgnoreService ignoreService;
private final NoticeService noticeService;
private final Scheduler scheduler;

@Inject
IgnoreCommand(IgnoreService ignoreService, NoticeService noticeService) {
IgnoreCommand(IgnoreService ignoreService, NoticeService noticeService, Scheduler scheduler) {
this.ignoreService = ignoreService;
this.noticeService = noticeService;
this.scheduler = scheduler;
}

@Execute
Expand All @@ -48,10 +51,9 @@ void ignore(@Context User sender, @Arg User target) {
return;
}

this.ignoreService.ignore(senderUuid, targetUuid).thenAccept(cancelled -> {
if (cancelled) {
return;
}

this.scheduler.run(() -> {
this.ignoreService.ignore(senderUuid, targetUuid);

this.noticeService.create()
.player(senderUuid)
Expand All @@ -62,21 +64,19 @@ void ignore(@Context User sender, @Arg User target) {
});
}


@Execute(name = "-all", aliases = "*")
@DescriptionDocs(description = "Ignore all players")
void ignoreAll(@Context User sender) {
UUID senderUuid = sender.getUniqueId();

this.ignoreService.ignoreAll(senderUuid).thenAccept(cancelled -> {
if (cancelled) {
return;
}
this.ignoreService.ignoreAll(senderUuid);

this.noticeService.create()
.player(senderUuid)
.notice(translation -> translation.privateChat().ignoreAll())
.send();
Comment on lines +73 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Inconsistent execution pattern

The ignoreAll method calls the service directly while ignore uses the scheduler. This creates an inconsistent pattern - both should use the same approach for predictability.

-        this.ignoreService.ignoreAll(senderUuid);
-
-        this.noticeService.create()
-            .player(senderUuid)
-            .notice(translation -> translation.privateChat().ignoreAll())
-            .send();
+        this.scheduler.run(() -> {
+            this.ignoreService.ignoreAll(senderUuid);
+
+            this.noticeService.create()
+                .player(senderUuid)
+                .notice(translation -> translation.privateChat().ignoreAll())
+                .send();
+        });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.ignoreService.ignoreAll(senderUuid);
this.noticeService.create()
.player(senderUuid)
.notice(translation -> translation.privateChat().ignoreAll())
.send();
this.scheduler.run(() -> {
this.ignoreService.ignoreAll(senderUuid);
this.noticeService.create()
.player(senderUuid)
.notice(translation -> translation.privateChat().ignoreAll())
.send();
});
🤖 Prompt for AI Agents
In
eternalcore-core/src/main/java/com/eternalcode/core/feature/ignore/IgnoreCommand.java
around lines 73 to 78, the ignoreAll method calls the ignoreService directly
while the ignore method uses a scheduler, causing inconsistency. To fix this,
modify the ignoreAll call to also use the scheduler for execution, matching the
pattern used by ignore, ensuring both methods execute asynchronously and
consistently.


this.noticeService.create()
.player(senderUuid)
.notice(translation -> translation.privateChat().ignoreAll())
.send();
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import com.eternalcode.core.feature.ignore.event.UnIgnoreEvent;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;

import com.eternalcode.core.util.FutureHandler;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

Expand All @@ -29,45 +29,46 @@ public CompletableFuture<Boolean> isIgnored(UUID requester, UUID target) {
}

@Override
public CompletableFuture<Boolean> ignore(UUID requester, UUID target) {
public void ignore(UUID requester, UUID target) {
IgnoreEvent event = this.caller.callEvent(new IgnoreEvent(requester, target));

if (event.isCancelled()) {
return CompletableFuture.completedFuture(false);
return;
}

return this.ignoreRepository.ignore(requester, target).thenApply(unused -> true);
this.ignoreRepository.ignore(requester, target).thenApply(unused -> FutureHandler.whenSuccess(null));
}

@Override
public CompletableFuture<Boolean> ignoreAll(UUID requester) {
public void ignoreAll(UUID requester) {
IgnoreAllEvent event = this.caller.callEvent(new IgnoreAllEvent(requester));

if (event.isCancelled()) {
return CompletableFuture.completedFuture(false);
return;
}

return this.ignoreRepository.ignoreAll(requester).thenApply(unused -> true);
this.ignoreRepository.ignoreAll(requester).thenApply(unused -> FutureHandler.whenSuccess(null));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^

}

@Override
public CompletableFuture<Boolean> unIgnore(UUID requester, UUID target) {
public void unIgnore(UUID requester, UUID target) {
UnIgnoreEvent event = this.caller.callEvent(new UnIgnoreEvent(requester, target));

if (event.isCancelled()) {
return CompletableFuture.completedFuture(false);
return;
}
return this.ignoreRepository.unIgnore(requester, target).thenApply(unused -> true);

this.ignoreRepository.unIgnore(requester, target).thenApply(unused -> FutureHandler.whenSuccess(null));
}

@Override
public CompletableFuture<Boolean> unIgnoreAll(UUID requester) {
public void unIgnoreAll(UUID requester) {
UnIgnoreAllEvent event = this.caller.callEvent(new UnIgnoreAllEvent(requester));

if (event.isCancelled()) {
return CompletableFuture.completedFuture(false);
return;
}
return this.ignoreRepository.unIgnoreAll(requester).thenApply(unused -> true);
}

this.ignoreRepository.unIgnoreAll(requester).thenApply(unused -> FutureHandler.whenSuccess(null));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.eternalcode.core.feature.ignore;

import com.eternalcode.annotations.scan.command.DescriptionDocs;
import com.eternalcode.commons.scheduler.Scheduler;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.notice.NoticeService;
import com.eternalcode.core.user.User;
Expand All @@ -19,11 +20,13 @@ class UnIgnoreCommand {

private final IgnoreService ignoreService;
private final NoticeService noticeService;
private final Scheduler scheduler;

@Inject
public UnIgnoreCommand(IgnoreService ignoreService, NoticeService noticeService) {
public UnIgnoreCommand(IgnoreService ignoreService, NoticeService noticeService, Scheduler scheduler) {
this.ignoreService = ignoreService;
this.noticeService = noticeService;
this.scheduler = scheduler;
}

@Execute
Expand All @@ -48,10 +51,9 @@ void ignore(@Context User sender, @Arg User target) {
return;
}

this.ignoreService.unIgnore(senderUuid, targetUuid).thenAccept(cancelled -> {
if (cancelled) {
return;
}

this.scheduler.run(() -> {
this.ignoreService.unIgnore(senderUuid, targetUuid);

this.noticeService.create()
.player(senderUuid)
Expand All @@ -67,16 +69,12 @@ void ignore(@Context User sender, @Arg User target) {
void unIgnoreAll(@Context User sender) {
UUID senderUuid = sender.getUniqueId();

this.ignoreService.unIgnoreAll(senderUuid).thenAccept(cancelled -> {
if (cancelled) {
return;
}
this.ignoreService.unIgnoreAll(senderUuid);

this.noticeService.create()
.player(senderUuid)
.notice(translation -> translation.privateChat().unIgnoreAll())
.send();
});
}
this.noticeService.create()
.player(senderUuid)
.notice(translation -> translation.privateChat().unIgnoreAll())
.send();

}
}