Skip to content

Commit

Permalink
Merge pull request kitodo#5681 from matthias-ronge/issue-5580a
Browse files Browse the repository at this point in the history
Remove ID of the command interface
  • Loading branch information
solth authored Nov 30, 2023
2 parents fddfa9b + 17bb214 commit 457cc74
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ public interface CommandInterface {
/**
* Runs a given command.
*
* @param id
* The id, to identify the command and it's results.
* @param command
* The command as a String.
* @return A commandResult, which contains id and result messages.
*/
CommandResult runCommand(Integer id, String command);
CommandResult runCommand(String command);
}
22 changes: 3 additions & 19 deletions Kitodo-API/src/main/java/org/kitodo/api/command/CommandResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@

public class CommandResult {

/** The id of the command. */
private Integer id;

/** The command as a String. */
private String command;

Expand All @@ -31,31 +28,19 @@ public class CommandResult {
/**
* Constructor.
*
* @param id
* The id.
* @param command
* The command.
* @param successful
* If command was successful.
* @param messages
* The resultMessages
*/
public CommandResult(Integer id, String command, boolean successful, List<String> messages) {
this.id = id;
public CommandResult(String command, boolean successful, List<String> messages) {
this.command = command;
this.successful = successful;
this.messages = messages;
}

/**
* Gets the id.
*
* @return The id.
*/
public Integer getId() {
return id;
}

/**
* Gets the command.
*
Expand Down Expand Up @@ -99,8 +84,7 @@ public boolean equals(Object object) {
if (object instanceof CommandResult) {
CommandResult that = (CommandResult) object;

return this.id.equals(that.id)
&& this.successful == that.successful
return this.successful == that.successful
&& this.command.equals(that.command)
&& this.messages.equals(that.messages);
}
Expand All @@ -109,6 +93,6 @@ public boolean equals(Object object) {

@Override
public int hashCode() {
return Objects.hash(id, successful, command, messages);
return Objects.hash(successful, command, messages);
}
}
16 changes: 7 additions & 9 deletions Kitodo-Command/src/main/java/org/kitodo/command/Command.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,12 @@ public class Command implements CommandInterface {
/**
* Method executes a script.
*
* @param id
* The id, to identify the command and it's results.
* @param command
* The command as a String.
* @return The command result.
*/
@Override
public CommandResult runCommand(Integer id, String command) {
public CommandResult runCommand(String command) {
CommandResult commandResult;
Process process;
String[] callSequence = command.split("[\\r\\n\\s]+");
Expand All @@ -53,26 +51,26 @@ public CommandResult runCommand(Integer id, String command) {

outputMessage.addAll(errorMessage);

commandResult = new CommandResult(id, command, errCode == 0, outputMessage);
commandResult = new CommandResult(command, errCode == 0, outputMessage);
if (commandResult.isSuccessful()) {
logger.info("Execution of Command {} {} was successful!: {}", commandResult.getId(),
logger.info("Execution of Command {} was successful!: {}",
commandResult.getCommand(), commandResult.getMessages());
} else {
logger.error("Execution of Command {} {} failed!: {}", commandResult.getId(),
logger.error("Execution of Command {} failed!: {}",
commandResult.getCommand(), commandResult.getMessages());
}
}
} catch (InterruptedException e) {
commandResult = new CommandResult(id, command, false, Collections.singletonList(e.getMessage()));
commandResult = new CommandResult(command, false, Collections.singletonList(e.getMessage()));
logger.error("Execution of Command Thread was interrupted!");
Thread.currentThread().interrupt();
return commandResult;
} catch (IOException e) {
List<String> errorMessages = new ArrayList<>();
errorMessages.add(e.getCause().toString());
errorMessages.add(e.getMessage());
commandResult = new CommandResult(id, command, false, errorMessages);
logger.error("Execution of Command {} {} failed!: {}", commandResult.getId(), commandResult.getCommand(),
commandResult = new CommandResult(command, false, errorMessages);
logger.error("Execution of Command {} failed!: {}", commandResult.getCommand(),
commandResult.getMessages());
return commandResult;
}
Expand Down
17 changes: 8 additions & 9 deletions Kitodo-Command/src/test/java/org/kitodo/command/CommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@

public class CommandTest {
private static String scriptExtension;
private int processId = 3;
private static boolean windows = false;
private static File workingScript = new File(
System.getProperty("user.dir") + "/src/test/resources/working_script.sh");
Expand Down Expand Up @@ -70,7 +69,7 @@ public void shouldRunCommand() {
Command command = new Command();

String commandString = "src/test/resources/working_script" + scriptExtension;
CommandResult commandResult = command.runCommand(processId, commandString);
CommandResult commandResult = command.runCommand(commandString);

List<String> expectedMessages = new ArrayList<>();
if (windows) {
Expand All @@ -80,7 +79,7 @@ public void shouldRunCommand() {

expectedMessages.add("Hello World");

CommandResult expectedCommandResult = new CommandResult(processId, commandString, true, expectedMessages);
CommandResult expectedCommandResult = new CommandResult(commandString, true, expectedMessages);

assertEquals("successful booleans of CommandResults are not identical", expectedCommandResult.isSuccessful(),
commandResult.isSuccessful());
Expand All @@ -95,9 +94,9 @@ public void shouldNotRunNotExistingCommand() {
Command command = new Command();

String commandString = "src/test/resources/notExistingScript" + scriptExtension;
CommandResult commandResult = command.runCommand(processId, commandString);
CommandResult commandResult = command.runCommand(commandString);

CommandResult expectedCommandResult = new CommandResult(processId, commandString, false, null);
CommandResult expectedCommandResult = new CommandResult(commandString, false, null);

assertEquals("Should not run not existing Command", expectedCommandResult.isSuccessful(),
commandResult.isSuccessful());
Expand All @@ -108,9 +107,9 @@ public void shouldNotRunCommandWithFalseSyntax() {
Command command = new Command();

String commandString = "src/test/resources/not_working_script" + scriptExtension;
CommandResult commandResult = command.runCommand(processId, commandString);
CommandResult commandResult = command.runCommand(commandString);

CommandResult expectedCommandResult = new CommandResult(processId, commandString, false, null);
CommandResult expectedCommandResult = new CommandResult(commandString, false, null);

assertEquals("Should not run command with false syntax", expectedCommandResult.isSuccessful(),
commandResult.isSuccessful());
Expand All @@ -121,7 +120,7 @@ public void shouldRunCommandWithParameter() {
Command command = new Command();

String commandString = "src/test/resources/working_script_with_parameters" + scriptExtension + " testParameter";
CommandResult commandResult = command.runCommand(processId, commandString);
CommandResult commandResult = command.runCommand(commandString);

ArrayList<String> expectedMessages = new ArrayList<>();

Expand All @@ -132,7 +131,7 @@ public void shouldRunCommandWithParameter() {

expectedMessages.add("testParameter");

CommandResult expectedCommandResult = new CommandResult(processId, commandString, true, expectedMessages);
CommandResult expectedCommandResult = new CommandResult(commandString, true, expectedMessages);

assertEquals("successful booleans of CommandResults are not identical", expectedCommandResult.isSuccessful(),
commandResult.isSuccessful());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,13 @@
import java.io.File;
import java.io.IOException;
import java.util.List;
import java.util.Random;

import org.kitodo.api.command.CommandInterface;
import org.kitodo.api.command.CommandResult;
import org.kitodo.serviceloader.KitodoServiceLoader;

class CommandService {

private Random random = new Random(1000000);

/**
* Method executes a script string.
*
Expand All @@ -39,7 +36,7 @@ CommandResult runCommand(String script) throws IOException {
KitodoServiceLoader<CommandInterface> serviceLoader = new KitodoServiceLoader<>(CommandInterface.class);
CommandInterface command = serviceLoader.loadModule();

CommandResult commandResult = command.runCommand(random.nextInt(), script);
CommandResult commandResult = command.runCommand(script);
List<String> commandResultMessages = commandResult.getMessages();
if (commandResultMessages.size() > 0 && commandResultMessages.get(0).contains("IOException")) {
throw new IOException(commandResultMessages.get(1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ public URI createProcessLocation(String processId) throws IOException {
File processRootDirectory = new File(KitodoConfig.getKitodoDataDirectory() + File.separator + processId);
String scriptCreateDirMeta = KitodoConfig.getParameter("script_createDirMeta");
String command = scriptCreateDirMeta + ' ' + processRootDirectory.getPath();
if (!processRootDirectory.exists() && !commandService.runCommand(command.hashCode(), command).isSuccessful()) {
if (!processRootDirectory.exists() && !commandService.runCommand(command).isSuccessful()) {
throw new IOException("Could not create processRoot directory.");
}
return fileMapper.unmapUriFromKitodoDataDirectoryUri(Paths.get(processRootDirectory.getPath()).toUri());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,14 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Random;

import org.kitodo.api.command.CommandInterface;
import org.kitodo.api.command.CommandResult;
import org.kitodo.production.services.ServiceManager;
import org.kitodo.serviceloader.KitodoServiceLoader;

public class CommandService {
private final CommandInterface commandModule;
private final ArrayList<CommandResult> finishedCommandResults = new ArrayList<>();
private final Random random = new Random(1000000);

/**
* Initialize Command Service.
Expand All @@ -54,7 +51,7 @@ public CommandResult runCommand(String script) throws IOException {
if (Objects.isNull(script)) {
return null;
}
CommandResult commandResult = commandModule.runCommand(random.nextInt(), script);
CommandResult commandResult = commandModule.runCommand(script);
List<String> commandResultMessages = commandResult.getMessages();
if (!commandResultMessages.isEmpty() && commandResultMessages.get(0).contains("IOException")) {
throw new IOException(commandResultMessages.get(1));
Expand Down Expand Up @@ -110,7 +107,7 @@ public CommandResult runCommand(File scriptFile) throws IOException {
public void runCommandAsync(String script) {
if (Objects.nonNull(script)) {
Flowable<CommandResult> source = Flowable.fromCallable(() ->
commandModule.runCommand(random.nextInt(), script)
commandModule.runCommand(script)
);

Flowable<CommandResult> commandBackgroundWorker = source.subscribeOn(Schedulers.io());
Expand Down

0 comments on commit 457cc74

Please sign in to comment.