Skip to content

Commit

Permalink
* Improved Permissions check and translation (#394)
Browse files Browse the repository at this point in the history
* Fix sum of big numbers
* Fix validation command with long expressions
  • Loading branch information
twonirwana authored Dec 21, 2023
1 parent 111d812 commit c7ff877
Show file tree
Hide file tree
Showing 27 changed files with 141 additions and 58 deletions.
2 changes: 1 addition & 1 deletion bot/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies {
implementation(libs.guava)
implementation(libs.micrometer.core)
implementation(libs.commons.lang3)
implementation("org.apache.commons:commons-text:1.11.0")
implementation(libs.commons.text)
implementation("io.micrometer:micrometer-registry-prometheus:1.12.1")
implementation("com.h2database:h2:2.2.224")
implementation("org.apache.derby:derby:10.17.1.0")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public Mono<Void> handleComponentInteractEvent(@NonNull ButtonEventAdaptor event
state = configAndState.getState();
}
final Long answerTargetChannelId = config.getAnswerTargetChannelId();
Optional<String> checkPermissions = event.checkPermissions(answerTargetChannelId);
Optional<String> checkPermissions = event.checkPermissions(answerTargetChannelId, event.getRequester().getUserLocal());
if (checkPermissions.isPresent()) {
return event.editMessage(checkPermissions.get(), null);
}
Expand Down Expand Up @@ -360,7 +360,7 @@ protected Mono<Void> deleteOldAndConcurrentMessageAndData(

@Override
public @NonNull Mono<Void> handleSlashCommandEvent(@NonNull SlashEventAdaptor event, @NonNull Supplier<UUID> uuidSupplier, @NonNull Locale userLocale) {
Optional<String> checkPermissions = event.checkPermissions();
Optional<String> checkPermissions = event.checkPermissions(userLocale);
if (checkPermissions.isPresent()) {
return event.reply(checkPermissions.get(), false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,7 @@ public String toShortString() {
.toList().toString())
.orElse(null);
return String.format("%s=%s", expression,
Joiner.on(",").skipNulls().join(result, rollDetails, errorMessage, warning, fieldStringList))
.replace("▢", "0")
.replace("+", "+")
.replace("−", "-") //minus is not hyphen-minus
.replace("*", "");
Joiner.on(",").skipNulls().join(result, rollDetails, errorMessage, warning, fieldStringList));
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ DirectRollConfig deserializeConfig(ChannelConfigDTO channelConfigDTO) {
public @NonNull Mono<Void> handleSlashCommandEvent(@NonNull SlashEventAdaptor event, @NonNull Supplier<UUID> uuidSupplier, @NonNull Locale userLocale) {
Stopwatch stopwatch = Stopwatch.createStarted();

Optional<String> checkPermissions = event.checkPermissions();
Optional<String> checkPermissions = event.checkPermissions(userLocale);
if (checkPermissions.isPresent()) {
return event.reply(checkPermissions.get(), false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,14 @@ public ValidationCommand(PersistenceManager persistenceManager, CachingDiceEvalu
BotMetrics.incrementValidationCounter(validation.isEmpty());
return validation
.map(s -> List.of(new AutoCompleteAnswer(s, option.getFocusedOptionValue())))
//todo sometimes to long
.orElse(List.of(new AutoCompleteAnswer(option.getFocusedOptionValue(), option.getFocusedOptionValue())));
.orElse(List.of(getValidAutoCompleteMessage(option.getFocusedOptionValue(), userLocale)));
}

private AutoCompleteAnswer getValidAutoCompleteMessage(@NonNull String typedExpression, @NonNull Locale userLocale){
if(typedExpression.length() <= 100){
return new AutoCompleteAnswer(typedExpression, typedExpression);
}
return new AutoCompleteAnswer(I18n.getMessage("validation.autoComplete.tooLong", userLocale), I18n.getMessage("validation.autoComplete.tooLong", userLocale));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ private boolean matchRpgPreset(String typed, RpgSystemCommandPreset.PresetId pre
public @NonNull Mono<Void> handleSlashCommandEvent(@NonNull SlashEventAdaptor event, @NonNull Supplier<UUID> uuidSupplier, @NonNull Locale userLocal) {
Stopwatch stopwatch = Stopwatch.createStarted();

Optional<String> checkPermissions = event.checkPermissions();
Optional<String> checkPermissions = event.checkPermissions(userLocal);
if (checkPermissions.isPresent()) {
return event.reply(checkPermissions.get(), false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.jetbrains.annotations.Nullable;

import java.io.InputStream;
import java.math.BigDecimal;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
Expand Down Expand Up @@ -53,14 +54,14 @@ private static Optional<String> getLabelFromExpressionWithOptionalLabel(String e
}

private static String getResult(Roll roll, boolean sumUp) {
if (sumUp && allElementsAreIntegers(roll) && allElementsHaveNoColor(roll)) {
return String.valueOf(roll.getElements().stream().flatMap(r -> r.asInteger().stream()).mapToInt(i -> i).sum());
if (sumUp && allElementsAreDecimal(roll) && allElementsHaveNoColor(roll)) {
return String.valueOf(roll.getElements().stream().flatMap(r -> r.asDecimal().stream()).reduce(BigDecimal::add).orElseThrow());
}
return roll.getResultString();
}

private static boolean allElementsAreIntegers(Roll roll) {
return roll.getElements().stream().allMatch(r -> r.asInteger().isPresent());
private static boolean allElementsAreDecimal(Roll roll) {
return roll.getElements().stream().allMatch(r -> r.asDecimal().isPresent());
}

private static boolean allElementsHaveNoColor(Roll roll) {
Expand Down
1 change: 1 addition & 0 deletions bot/src/main/resources/botMessages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ quickstart.option.description=Start typing to filter and see more options
#max 100 characters
base.option.dice_image_style.autoComplete.missingStyle.name=Select the dice image style first
validation.autoComplete.example=2d6=
validation.autoComplete.tooLong=Expression is valid but to long for auto complete
base.option.dice.dice_image_style.autoComplete.missing.style.value=none
base.option.dice_color.none.none=none
base.option.dice_color.polyhedral_3d.red_and_white=red_and_white
Expand Down
1 change: 1 addition & 0 deletions bot/src/main/resources/botMessages_de.properties
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ quickstart.option.description=Tippe um zu filtern und mehr Optionen zu sehen
#max 100 characters
base.option.dice_image_style.autoComplete.missingStyle.name=Wähle zuerst einen Würfelbilderstil
validation.autoComplete.example=2d6=
validation.autoComplete.tooLong=Würfelausdruck ist korrekt aber zu lang für AutoComplete
base.option.dice.dice_image_style.autoComplete.missing.style.value=kein
base.option.dice_color.none.none=kein
base.option.dice_color.polyhedral_3d.red_and_white=rot_und_weiß
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public Requester getRequester() {
}

@Override
public Optional<String> checkPermissions(Long answerTargetChannelId) {
public Optional<String> checkPermissions(Long answerTargetChannelId, @NonNull Locale userLocale) {
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public Long getGuildId() {
}

@Override
public Optional<String> checkPermissions() {
public Optional<String> checkPermissions(@NonNull Locale userLocal) {
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ void handleSlashCommandEvent() {
StepVerifier.create(res).verifyComplete();


verify(event).checkPermissions();
verify(event).checkPermissions(Locale.ENGLISH);
verify(event).getCommandString();
verify(event).getOption(any());
verify(event).reply(any(), anyBoolean());
Expand Down Expand Up @@ -306,7 +306,7 @@ void handleSlashCommandEvent_help() {
assertThat(res).isNotNull();


verify(event).checkPermissions();
verify(event).checkPermissions(Locale.ENGLISH);
verify(event).getCommandString();
verify(event, times(2)).getOption(any());
verify(event).replyWithEmbedOrMessageDefinition(EmbedOrMessageDefinition.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void handleComponentInteractEvent() {
StepVerifier.create(res)
.verifyComplete();

verify(slashEventAdaptor).checkPermissions();
verify(slashEventAdaptor).checkPermissions(Locale.ENGLISH);
verify(slashEventAdaptor, never()).reply(any(), anyBoolean());
verify(slashEventAdaptor).acknowledgeAndRemoveSlash();
verify(slashEventAdaptor).getOption("expression");
Expand Down Expand Up @@ -92,7 +92,7 @@ void handleComponentInteractEvent_validationFailed() {

assertThat(res).isNotNull();

verify(slashEventAdaptor).checkPermissions();
verify(slashEventAdaptor).checkPermissions(Locale.ENGLISH);
verify(slashEventAdaptor).getOption("expression");
verify(slashEventAdaptor, times(1)).getCommandString();
verify(slashEventAdaptor, never()).createMessageWithoutReference(any());
Expand Down Expand Up @@ -126,7 +126,7 @@ void handleComponentInteractEvent_help() {

assertThat(res).isNotNull();

verify(slashEventAdaptor).checkPermissions();
verify(slashEventAdaptor).checkPermissions(Locale.ENGLISH);
verify(slashEventAdaptor).getOption("expression");
verify(slashEventAdaptor, times(1)).getCommandString();
verify(slashEventAdaptor, never()).createMessageWithoutReference(any());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void roll_default() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -81,7 +81,7 @@ void roll_NoWarn() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -145,7 +145,7 @@ void roll_default_withLabel() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -194,7 +194,7 @@ void roll_config_full_imageNone() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -244,7 +244,7 @@ void roll_config_withoutExpression() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -292,7 +292,7 @@ void roll_config_withoutExpression_withLabel() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -341,7 +341,7 @@ void roll_config_compact() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -390,7 +390,7 @@ void roll_config_minimal() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -431,7 +431,7 @@ void channelAlias() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down Expand Up @@ -472,7 +472,7 @@ void userChannelAlias() {

List<EmbedOrMessageDefinition> replyMessages = hiddenRollCommandEvent.getAllReplays();
assertThat(replyMessages).hasSize(1);
EmbedOrMessageDefinition replayMessage = replyMessages.get(0);
EmbedOrMessageDefinition replayMessage = replyMessages.getFirst();

ButtonEventAdaptorMock buttonEvent = new ButtonEventAdaptorMock("h", "reveal", replayMessage);
underTest.handleComponentInteractEvent(buttonEvent).block();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void handleComponentInteractEvent() {
StepVerifier.create(res)
.verifyComplete();

verify(slashEventAdaptor).checkPermissions();
verify(slashEventAdaptor).checkPermissions(Locale.ENGLISH);
verify(slashEventAdaptor).reply("/validation expression:1d6", true);
verify(slashEventAdaptor, never()).acknowledgeAndRemoveSlash();
verify(slashEventAdaptor).getOption("expression");
Expand Down Expand Up @@ -112,7 +112,7 @@ void handleComponentInteractEvent_validationFailed() {

assertThat(res).isNotNull();

verify(slashEventAdaptor).checkPermissions();
verify(slashEventAdaptor).checkPermissions(Locale.ENGLISH);
verify(slashEventAdaptor).getOption("expression");
verify(slashEventAdaptor, times(1)).getCommandString();
verify(slashEventAdaptor, never()).createMessageWithoutReference(any());
Expand Down Expand Up @@ -146,7 +146,7 @@ void handleComponentInteractEvent_help() {

assertThat(res).isNotNull();

verify(slashEventAdaptor).checkPermissions();
verify(slashEventAdaptor).checkPermissions(Locale.ENGLISH);
verify(slashEventAdaptor).getOption("expression");
verify(slashEventAdaptor, times(1)).getCommandString();
verify(slashEventAdaptor, never()).createMessageWithoutReference(any());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.time.OffsetDateTime;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.Optional;

public interface ButtonEventAdaptor extends DiscordAdapter {
Expand All @@ -30,7 +31,7 @@ public interface ButtonEventAdaptor extends DiscordAdapter {

Requester getRequester();

Optional<String> checkPermissions(Long answerTargetChannelId);
Optional<String> checkPermissions(Long answerTargetChannelId, @NonNull Locale userLocale);

Mono<Void> createResultMessageWithReference(EmbedOrMessageDefinition answer, Long targetChannelId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
import reactor.core.publisher.Mono;

import java.util.List;
import java.util.Locale;
import java.util.Optional;

public interface SlashEventAdaptor extends DiscordAdapter {
Optional<String> checkPermissions();
Optional<String> checkPermissions(@NonNull Locale userLocale);

Optional<CommandInteractionOption> getOption(@NonNull String optionName);

Expand Down
1 change: 1 addition & 0 deletions discord-connector/jda/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ dependencies {
implementation(libs.guava)
implementation(libs.commons.lang3)
implementation(libs.micrometer.core)
implementation(libs.commons.text)

compileOnly(libs.lombok)
annotationProcessor(libs.lombok)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@

import java.io.InputStream;
import java.time.OffsetDateTime;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.concurrent.ExecutionException;
import java.util.function.Supplier;

Expand Down Expand Up @@ -140,17 +137,17 @@ public Mono<Void> createResultMessageWithReference(EmbedOrMessageDefinition answ
}

@Override
public Optional<String> checkPermissions(Long answerTargetChannelId) {
Optional<String> primaryChannelPermissionCheck = checkPermission(event.getMessageChannel(), event.getGuild(), true);
public Optional<String> checkPermissions(Long answerTargetChannelId, @NonNull Locale userLocale) {
Optional<String> primaryChannelPermissionCheck = checkPermission(event.getMessageChannel(), event.getGuild(), true, userLocale);
if (primaryChannelPermissionCheck.isPresent()) {
return primaryChannelPermissionCheck;
}
if (answerTargetChannelId != null) {
Optional<MessageChannel> answerChannel = Optional.ofNullable(event.getGuild()).map(g -> g.getChannelById(MessageChannel.class, answerTargetChannelId));
if (answerChannel.isEmpty()) {
return Optional.of("Configured answer target channel is not a valid message channel");
return Optional.of(I18n.getMessage("permission.check.target.invalid", userLocale));
}
return checkPermission(answerChannel.get(), event.getGuild(), true);
return checkPermission(answerChannel.get(), event.getGuild(), true, userLocale);
}
return Optional.empty();
}
Expand Down Expand Up @@ -246,7 +243,7 @@ public EmbedOrMessageDefinition getMessageDefinitionOfEventMessageWithoutButtons
title = null;
} else {
type = EmbedOrMessageDefinition.Type.EMBED;
MessageEmbed embed = message.getEmbeds().get(0);
MessageEmbed embed = message.getEmbeds().getFirst();
descriptionOrContent = embed.getDescription();
title = embed.getTitle();
if (embed.getImage() == null) {
Expand Down
Loading

0 comments on commit c7ff877

Please sign in to comment.