Skip to content
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

Jump to bottom

feat(core): add option to hide existence of not-permitted commands #692

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

feat(core): add option to hide existence of not-permitted commands #692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fca5f7e
Return syntax instead of no permission in some cases
jpenilla Feb 21, 2024
42827d4
rename option
jpenilla Feb 25, 2024
f1b833b
add test
jpenilla Feb 25, 2024
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
96 changes: 71 additions & 25 deletions cloud-core/src/main/java/org/incendo/cloud/CommandTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
final PermissionResult permissionResult = this.determinePermissionResult(commandContext.sender(), root);
if (permissionResult.denied()) {
return CompletableFutures.failedFuture(
new NoPermissionException(
permissionResult,
commandContext.sender(),
this.getComponentChain(root)
)
this.noPermissionOrSyntax(permissionResult, commandContext.sender(), root)
);
}

Expand Down Expand Up @@ -329,11 +325,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
);
if (check.denied()) {
return CompletableFutures.failedFuture(
new NoPermissionException(
check,
commandContext.sender(),
this.getComponentChain(root)
)
this.noPermissionOrSyntax(check, commandContext.sender(), root)
);
}
return CompletableFuture.completedFuture(root.command());
Expand All @@ -350,6 +342,72 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
});
}

@SuppressWarnings({"unchecked", "rawtypes"})
private Exception noPermissionOrSyntax(
final PermissionResult permissionResult,
final C sender,
final CommandNode<C> root
) {
final boolean convert = this.commandManager.settings().get(ManagerSetting.HIDE_COMMAND_EXISTENCE);
if (!convert) {
return new NoPermissionException(
permissionResult,
sender,
this.getComponentChain(root)
);
}

if (this.childPermitted(root, sender)) {
return new InvalidSyntaxException(
this.commandManager.commandSyntaxFormatter().apply(sender, (List) this.getComponentChain(root), root),
sender, this.getComponentChain(root)
);
}

final @Nullable List<CommandNode<C>> parentChain = this.permittedParentChain(root, sender);
if (parentChain != null) {
return new InvalidSyntaxException(
this.commandManager.commandSyntaxFormatter().apply(
sender,
parentChain.stream().map(CommandNode::component)
.filter(Objects::nonNull).collect(Collectors.toList()),
root
),
sender, this.getComponentChain(root)
);
}

return new NoPermissionException(
permissionResult,
sender,
this.getComponentChain(root)
);
}

private boolean childPermitted(final CommandNode<C> node, final C sender) {
if (this.determinePermissionResult(sender, node).allowed()) {
return true;
}
for (final CommandNode<C> child : node.children()) {
if (this.childPermitted(child, sender)) {
return true;
}
}
return false;
}

private @Nullable List<CommandNode<C>> permittedParentChain(final CommandNode<C> node, final C sender) {
final @Nullable CommandNode<C> parent = node.parent();
if (parent != null) {
if (this.determinePermissionResult(sender, parent).allowed()) {
return this.getChain(parent);
} else {
return this.permittedParentChain(parent, sender);
}
}
return null;
}

private @Nullable CompletableFuture<@Nullable Command<C>> attemptParseUnambiguousChild(
final @NonNull List<@NonNull CommandComponent<C>> parsedArguments,
final @NonNull CommandContext<C> commandContext,
Expand Down Expand Up @@ -381,11 +439,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
final PermissionResult childCheck = this.determinePermissionResult(sender, child);
if (!commandInput.isEmpty() && childCheck.denied()) {
return CompletableFutures.failedFuture(
new NoPermissionException(
childCheck,
sender,
this.getComponentChain(child)
)
this.noPermissionOrSyntax(childCheck, sender, child)
);
}

Expand Down Expand Up @@ -448,11 +502,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
return CompletableFuture.completedFuture(command);
}
return CompletableFutures.failedFuture(
new NoPermissionException(
check,
sender,
this.getComponentChain(root)
)
this.noPermissionOrSyntax(check, sender, root)
);
} else {
// The child is not a leaf, but may have an intermediary executor, attempt to use it
Expand All @@ -477,11 +527,7 @@ private CommandTree(final @NonNull CommandManager<C> commandManager) {
}

return CompletableFutures.failedFuture(
new NoPermissionException(
check,
sender,
this.getComponentChain(root)
)
this.noPermissionOrSyntax(check, sender, root)
);
}
}
Expand Down
10 changes: 9 additions & 1 deletion cloud-core/src/main/java/org/incendo/cloud/setting/ManagerSetting.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,13 @@ public enum ManagerSetting implements Setting {
* and code inspecting the command tree may need to be adjusted.
*/
@API(status = API.Status.EXPERIMENTAL)
LIBERAL_FLAG_PARSING
LIBERAL_FLAG_PARSING,

/**
* When the sender does not have permission for the parsed command, but does have permission for a preceding or following
* node, the command tree will return a {@link org.incendo.cloud.exception.InvalidSyntaxException} instead of a
* {@link org.incendo.cloud.exception.NoPermissionException}.
*/
@API(status = API.Status.EXPERIMENTAL)
HIDE_COMMAND_EXISTENCE
}
30 changes: 26 additions & 4 deletions cloud-core/src/test/java/org/incendo/cloud/PermissionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,29 @@
package org.incendo.cloud;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.incendo.cloud.exception.InvalidSyntaxException;
import org.incendo.cloud.exception.NoPermissionException;
import org.incendo.cloud.execution.CommandResult;
import org.incendo.cloud.execution.ExecutionCoordinator;
import org.incendo.cloud.key.CloudKey;
import org.incendo.cloud.permission.Permission;
import org.incendo.cloud.permission.PermissionResult;
import org.incendo.cloud.permission.PredicatePermission;
import org.incendo.cloud.setting.ManagerSetting;
import org.incendo.cloud.suggestion.Suggestion;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

Expand All @@ -54,7 +60,8 @@
@ExtendWith(MockitoExtension.class)
class PermissionTest {

@Mock(strictness = Mock.Strictness.LENIENT) private Function<String, Boolean> permissionFunction;
@Mock(strictness = Mock.Strictness.LENIENT)
private Function<String, Boolean> permissionFunction;

private CommandManager<TestCommandSender> manager;

Expand Down Expand Up @@ -117,9 +124,16 @@ void testSubCommandPermission() {
assertThat(exception).hasCauseThat().isInstanceOf(NoPermissionException.class);
}

@Test
void testPermittedNodeFollowingNotPermittedNode() {
@ParameterizedTest
@MethodSource
void testPermittedNodeFollowingNotPermittedNode(
final Class<? extends Exception> expectedException,
final List<ManagerSetting> settings
) {
// Arrange
for (final ManagerSetting setting : settings) {
this.manager.settings().set(setting, true);
}
this.manager.command(this.manager.commandBuilder("root")
.literal("no")
.permission("0"));
Expand All @@ -136,10 +150,18 @@ void testPermittedNodeFollowingNotPermittedNode() {
.executeCommand(sender, "root no yes");

// Assert
assertThat(noPermission).hasFailureThat().isInstanceOf(NoPermissionException.class);
assertThat(noPermission).hasFailureThat().isInstanceOf(expectedException);
assertThat(permitted).hasResult();
}

@SuppressWarnings("unused")
private static List<Arguments> testPermittedNodeFollowingNotPermittedNode() {
return Arrays.asList(
Arguments.of(NoPermissionException.class, Collections.emptyList()),
Arguments.of(InvalidSyntaxException.class, Arrays.asList(ManagerSetting.HIDE_COMMAND_EXISTENCE))
);
}

@Test
void testAndPermissionsMissingOne() {
// Arrange
Expand Down
Loading