Diskussion Unit-Tests für Parametervalidierung - Codereview

Mat · 28.07.2020

Und auch für diese Woche habe ich wieder alten Code herausgekramt.

Die Aufgabe bestand darin, eine Client-Server-Verbindung zu implementieren, die Befehle per TCP entgegen nimmt und verarbeitet.

Interessanterweise hatte ich da aber wohl mal Tests für implementiert und wollte ein bisschen Feedback dazu holen.

Habe so viel vom Originalcode entfernt wie möglich, sodass nur noch funktionierende UnitTests bleiben und man ungefähr sehen kann, wie das Programm aufgebaut ist. Es ist lauffähig aber Client und ein bisschen Drumherum fehlen. Trotzdem noch recht groß, deswegen auch noch zusätzlich als zip dabei.

Tests

Main

MainTest:

package dev;

import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

class MainTest {

  @Test
  void argsAccept0() {
    assertTrue(Main.argsValidator.test(0));
  }

  @Test
  void argsDenyTooMany() {
    assertFalse(Main.argsValidator.test(Main.REQUIRED_ARGS + 1));
  }

  @Test
  void argsDenyTooFew() {
    assertFalse(Main.argsValidator.test(Main.REQUIRED_ARGS - 1));
  }

  @Test
  void argsAcceptValidNumber() {
    assertTrue(Main.argsValidator.test(Main.REQUIRED_ARGS));
  }
}

Main:

package dev;

import dev.util.ServerValidator;

import java.util.function.Predicate;

class Main {
  private static final boolean NO_ARGS_OK = true;
  static final int REQUIRED_ARGS = 2;

  static final Predicate<Integer> argsValidator = numOfArgs -> numOfArgs == REQUIRED_ARGS || (NO_ARGS_OK && numOfArgs == 0);

  public static void main(String[] args) {
    if (!argsValidator.test(args.length)) {
      System.out.println("\nUngültige Anzahl an Parametern übergeben");
      errorMessage();
      return;
    }

    Server server;
    if (args.length == REQUIRED_ARGS) {
      if (!ServerValidator.isPortValid(args[0]) || !ServerValidator.isPwValid(args[1])) {
        System.out.println("\nUngültige Parameter");
        errorMessage();
        return;
      }
      int port = Integer.parseInt(args[0]);
      String pw = args[1];
      server = new Server(port, pw);
    } else {
      server = new Server();
    }
    server.run();
  }

  private static void errorMessage() {
    System.out.println("Verwendung:");
    System.out.println("xyz.jar [{PORT} {PASSWORT}]");
    System.out.println("------------------------------------------------------------------");
  }
}

Session

SessionTest:

package dev;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.net.Socket;
import java.util.HashMap;
import java.util.Map;

import static dev.util.ServerExceptions.SyntaxError;
import static dev.util.ServerExceptions.UnknownCommand;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

class SessionTest {
  private Session session;
  private Map<Command, String> command;

  @BeforeEach
  void setUp() {
    this.session = new Session(new Socket());
    this.command = new HashMap<>();
  }

  @Test
  void denyUnknownCommands() {
    assertThrows(UnknownCommand.class, () -> session.parseCmdFromString("enum existiert nicht"));
  }

  @Test
  void denyWrongCase() {
    assertThrows(UnknownCommand.class, () -> session.parseCmdFromString("bye"));
  }

  @Test
  void byeAccept() throws Exception {
    command.put(Command.BYE, null);
    assertEquals(command, session.parseCmdFromString("BYE"));
  }

  @Test
  void byeDenyParams() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("BYE" + Command.DELIMITER + "text"));
  }

  @Test
  void shutdownAccept() throws Exception {
    command.put(Command.SHUTDOWN, "passwort");
    assertEquals(command, session.parseCmdFromString("SHUTDOWN" + Command.DELIMITER + "passwort"));
  }

  @Test
  void shutdownAcceptDelimiter() throws Exception {
    String msg = "pwteil1" + Command.DELIMITER + "pwteil2" + Command.DELIMITER;
    command.put(Command.SHUTDOWN, msg);
    assertEquals(command, session.parseCmdFromString("SHUTDOWN" + Command.DELIMITER + msg));
  }

  @Test
  void shutdownDeny0Param() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("SHUTDOWN"));
  }

  @Test
  void shutdownDenyEmptyParam() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("SHUTDOWN" + Command.DELIMITER + ""));
  }

  @Test
  void lowercaseAccept() throws Exception {
    command.put(Command.LOWERCASE, "text");
    assertEquals(command, session.parseCmdFromString("LOWERCASE" + Command.DELIMITER + "text"));
  }

  @Test
  void lowercaseAcceptDelimiter() throws Exception {
    String msg = "tEiL1" + Command.DELIMITER + "TeIl2" + Command.DELIMITER;
    command.put(Command.LOWERCASE, msg);
    assertEquals(command, session.parseCmdFromString("LOWERCASE" + Command.DELIMITER + msg));
  }

  @Test
  void lowercaseDeny0Param() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("LOWERCASE"));
  }

  @Test
  void lowercaseDenyEmptyParam() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("LOWERCASE" + Command.DELIMITER + ""));
  }

  @Test
  void uppercaseAccept() throws Exception {
    command.put(Command.UPPERCASE, "text");
    assertEquals(command, session.parseCmdFromString("UPPERCASE" + Command.DELIMITER + "text"));
  }

  @Test
  void uppercaseAcceptDelimiter() throws Exception {
    String msg = "tEiL1" + Command.DELIMITER + "TeIl2" + Command.DELIMITER;
    command.put(Command.UPPERCASE, msg);
    assertEquals(command, session.parseCmdFromString("UPPERCASE" + Command.DELIMITER + msg));
  }

  @Test
  void uppercaseDeny0Params() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("UPPERCASE"));
  }

  @Test
  void uppercaseDenyEmptyParam() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("UPPERCASE" + Command.DELIMITER + ""));
  }

  @Test
  void reverseAccept() throws Exception {
    command.put(Command.REVERSE, "text");
    assertEquals(command, session.parseCmdFromString("REVERSE" + Command.DELIMITER + "text"));
  }

  @Test
  void reverseAcceptDelimiter() throws Exception {
    String msg = "tEiL1" + Command.DELIMITER + "TeIl2" + Command.DELIMITER;
    command.put(Command.REVERSE, msg);
    assertEquals(command, session.parseCmdFromString("REVERSE" + Command.DELIMITER + msg));
  }

  @Test
  void reverseDeny0ParamNot() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("REVERSE"));
  }

  @Test
  void reverseDenyEmptyParam() {
    assertThrows(SyntaxError.class, () -> session.parseCmdFromString("REVERSE" + Command.DELIMITER + ""));
  }
}

Session:

package dev;

import dev.util.ServerExceptions;
import dev.util.ServerValidator;
import dev.util.StringHelper;

import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;

final class Session implements Runnable {

  private final Socket socket;
  private DataInputStream inputStream;
  private DataOutputStream outputStream;
  private boolean logoutTriggered;

  private String threadName;

  private void setThreadName(String threadName) { this.threadName = threadName; }

  private String getThreadName() { return this.threadName; }

  Session(Socket socket) {
    this.socket = socket;
    this.logoutTriggered = false;
  }

  private void endSession() { logoutTriggered = true; }

  private void shutdownServer(String pw) {
    if (Server.attemptShutdown(pw)) {
      try {
        sendMsg("Server wird bald heruntergefahren");
      } catch (IOException e) { /* _ */
      }
    } else {
      try {
        sendMsg("Falsches Passwort: Verbindung wird getrennt, Server läuft weiter");
      } catch (IOException e) { /* _ */
      }
    }
    endSession();
  }

  // Wäre vielleicht besser in einem Builder oder im ServerValidator aufgehoben
  Map<Command, String> parseCmdFromString(String text) throws ServerExceptions.SyntaxError, ServerExceptions.UnknownCommand {

    final Map<Command, String> cmd = new HashMap<>(1);
    String[] input = text.split(Command.DELIMITER, 2);

    Command cmdEnum;
    try {
      cmdEnum = Command.valueOf(input[0]);
    } catch (IllegalArgumentException e) {
      throw new ServerExceptions.UnknownCommand();
    }

    // if-else würde hier reichen, es gibt hier im Grunde nur 2 Fälle
    switch (cmdEnum) {
    case BYE:
      if (input.length != 1) {
        throw new ServerExceptions.SyntaxError("BYE nur ohne Parameter erlaubt");
      }
      cmd.put(cmdEnum, null);
      break;
    case LOWERCASE:
      if (input.length != 2 || input[1].isEmpty()) {
        throw new ServerExceptions.SyntaxError("LOWERCASE: Text benötigt");
      }
    case REVERSE:
      if (input.length != 2 || input[1].isEmpty()) {
        throw new ServerExceptions.SyntaxError("REVERSE: Text benötigt");
      }
    case SHUTDOWN:
      if (input.length != 2 || input[1].isEmpty()) {
        throw new ServerExceptions.SyntaxError("SHUTDOWN: Passwort benötigt");
      }
    case UPPERCASE:
      if (input.length != 2 || input[1].isEmpty()) {
        throw new ServerExceptions.SyntaxError("UPPERCASE: Text benötigt");
      }
    default:
      if (!ServerValidator.PARAM_VALID(cmdEnum, input[1])) {
        throw new ServerExceptions.SyntaxError(
            String.format("Ungültiger Parameter, %s benötigt Text und erlaubt maximal %d Bytes", cmdEnum.name(), ServerValidator.maxParamBytesInCmd(cmdEnum)));
      }

      cmd.put(cmdEnum, input[1]);
    }
    return cmd;
  }

  private String processCmd(Map<Command, String> cmd) throws ServerExceptions.SyntaxError, ServerExceptions.UnknownCommand {
    for (Map.Entry<Command, String> cmdEntry : cmd.entrySet()) {
      switch (cmdEntry.getKey()) {
      case BYE:
        endSession();
        return "OK BYE";
      case SHUTDOWN:
        shutdownServer(cmdEntry.getValue());
        return "OK BYE";
      case LOWERCASE:
        return "OK " + StringHelper.toLowerCase(cmdEntry.getValue());
      case UPPERCASE:
        return "OK " + StringHelper.toUpperCase(cmdEntry.getValue());
      case REVERSE:
        return "OK " + StringHelper.reverseString(cmdEntry.getValue());
      default:
        break;
      }
    }
    throw new ServerExceptions.UnknownCommand();
  }

  private void processRequest(String request) throws IOException {
    try {
      Map<Command, String> parsedRequest = parseCmdFromString(request);
      sendMsg(processCmd(parsedRequest));
    } catch (ServerExceptions.UnknownCommand | ServerExceptions.SyntaxError e) {
      sendMsg(e.getMessage());
    }
  }

  private void greetClient() throws IOException { sendMsg(Command.getCmdInfos()); }

  private void sendMsg(final String msg) throws IOException {
    byte[] msgBytes = msg.getBytes(StandardCharsets.UTF_8);

    for (byte b : msgBytes) {
      outputStream.write(b);
    }
    outputStream.write('\n');
    outputStream.flush();
  }

  private String receiveMsg() throws IOException, ServerExceptions.SyntaxError {
    byte[] inputBuffer = new byte[ServerValidator.BYTES_MAX];

    int offset;
    for (offset = 0; offset < ServerValidator.BYTES_MAX; offset++) {
      byte b = inputStream.readByte();
      inputBuffer[offset] = b;

      if (b == '\n')
        break;

      if (offset >= ServerValidator.BYTES_MAX - 1) {
        System.out.printf(Locale.GERMANY, "[%s] Nachricht zu lang (%s @ %s)\n", Server.currentTime(), getThreadName(), socket.getRemoteSocketAddress());
        throw new ServerExceptions.SyntaxError("Nachricht zu lang oder endet nicht mit \\n!");
      }
    }

    return new String(inputBuffer, 0, offset, StandardCharsets.UTF_8);
  }

  private void initSession(String threadName) throws IOException {
    setThreadName(threadName);

    System.out.printf(Locale.GERMANY, "[%s] Sitzung gestartet (%s @ %s)\n", Server.currentTime(), getThreadName(), socket.getRemoteSocketAddress());

    inputStream = new DataInputStream(socket.getInputStream());
    outputStream = new DataOutputStream(socket.getOutputStream());

    greetClient();
  }

  @Override
  public void run() {
    try {
      initSession(Thread.currentThread().getName());

      Thread clientMessages = new Thread(() -> {
        try {
          while (!Thread.currentThread().isInterrupted()) {
            String msg = receiveMsg();
            System.out.printf("[%s] %s: %s\n", Server.currentTime(), Thread.currentThread().getName(), msg);
            Server.updateHeartbeat(System.currentTimeMillis());
            processRequest(msg);
          }
        } catch (IOException e) {
          System.out.printf("[%s] %s: Verbindung getrennt\n", Server.currentTime(), getThreadName());
          endSession();
        } catch (ServerExceptions.SyntaxError ex) {
          try {
            sendMsg(ex.getMessage());
            sendMsg("Sitzung geschlossen. Neu verbinden");
          } catch (IOException couldNotSendMsg) { /* _ */
          } finally {
            endSession();
          }
        }
      });

      clientMessages.start();

      while (true) {
        if (logoutTriggered) {
          if (clientMessages.isAlive()) {
            clientMessages.interrupt();
          }
          if (!socket.isClosed()) {
            socket.shutdownOutput();
            socket.shutdownInput();
            socket.close();
          }
          return;
        }

        try {
          Thread.sleep(Server.UPDATE_INTERVAL_IN_MS);
        } catch (InterruptedException e) {
          return;
        }
      }
    } catch (IOException e) {
      System.err.println("Kann Sitzung nicht initialisieren");
      e.printStackTrace();
    }
  }
}

ServerValidator

ServerValidatorTest:

package dev.util;

import org.junit.jupiter.api.Test;

import static dev.Command.*;
import static dev.util.ServerValidator.*;
import static dev.util.StringHelper.generateSingleByteString;
import static org.junit.jupiter.api.Assertions.*;

class ServerValidatorTest {
  private static final int DELIMITER_BYTES = StringHelper.countBytesInString(DELIMITER);

  @Test
  void portAcceptValidPort() {
    assertTrue(isPortValid("50000"));
  }

  @Test
  void portAcceptLeading0() {
    assertTrue(isPortValid("050000"));
  }

  @Test
  void portDenyNotANumber() {
    assertFalse(isPortValid("KEINPORT"));
  }

  @Test
  void portDenyTooSmall() {
    assertFalse(isPortValid(String.valueOf(ServerValidator.PORT_MIN - 1)));
  }

  @Test
  void portDenyTooBig() {
    assertFalse(isPortValid(String.valueOf(ServerValidator.PORT_MAX + 1)));
  }

  @Test
  void pwAcceptValidPw() {
    assertTrue(isPwValid("passwort"));
  }

  @Test
  void pwAcceptSpecialChars() {
    assertTrue(isPwValid("_'@€§$%&/()=?\\\"µäöüß!-.é`´?"));
  }

  @Test
  void pwDenyEmpty() {
    assertFalse(isPwValid(""));
  }

  @Test
  void pwDenyNewline() {
    assertFalse(isPwValid("asd\nsd"));
  }

  /* Ich denke bei allen Aufrufen von generateSingleByteString('<CHAR>', <ANZAHL>) wäre es besser:
        - "<Zeichen>".repeat(<ANZAHL>) von String::repeat zu benutzen
        - den Wert einmalig als langen static final String fest zu legen, und dann Substring der nötigen Länge zu holen
        - Wert aus Random mit fixem Seed zu generieren, falls auch mehr Eingaben getestet werden sollen
     */

  @Test
  void pwAcceptMaxLength() {
    assertTrue(isPwValid(generateSingleByteString('a', maxParamBytesInCmd(SHUTDOWN))));
  }

  @Test
  void pwDenyTooLong() {
    assertFalse(isPwValid(generateSingleByteString('A', maxParamBytesInCmd(SHUTDOWN) + 1)));
  }

  /* Ich denke, alle folgenden maxBytes-Tests könnten parametrisiert/dynamisch statt per Hand getestet werden,
    da die Werte von festen Regeln abhängen:
        - ob Parameter erlaubt sind (wenn nicht, dann maxBytes = 0)
        - wie lang der Befehl + Delimiter ist
        - wie viele Bytes insgesamt pro Befehl zulässig sind (hier 255)
     */

  @Test
  void maxBytesBye() {
    assertEquals(0, maxParamBytesInCmd(BYE));
  }

  @Test
  void maxBytesByeDenyParams() {
    assertFalse(PARAM_VALID(BYE, "test"));
  }

  @Test
  void maxBytesShutdown() {
    assertEquals(BYTES_MAX - "SHUTDOWN".length() - DELIMITER_BYTES, maxParamBytesInCmd(SHUTDOWN));
  }

  @Test
  void maxBytesShutdownAcceptMaxLength() {
    assertTrue(PARAM_VALID(SHUTDOWN, generateSingleByteString('a', maxParamBytesInCmd(SHUTDOWN))));
  }

  @Test
  void maxBytesShutdownDenyTooLong() {
    assertFalse(PARAM_VALID(SHUTDOWN, generateSingleByteString('a', maxParamBytesInCmd(SHUTDOWN) + 1)));
  }

  @Test
  void maxBytesLowercase() {
    assertEquals(BYTES_MAX - "LOWERCASE".length() - DELIMITER_BYTES, maxParamBytesInCmd(LOWERCASE));
  }

  @Test
  void maxBytesLowercaseAcceptMaxLength() {
    assertTrue(PARAM_VALID(LOWERCASE, generateSingleByteString('a', maxParamBytesInCmd(LOWERCASE))));
  }

  @Test
  void maxBytesLowercaseDenyTooLong() {
    assertFalse(PARAM_VALID(LOWERCASE, generateSingleByteString('a', maxParamBytesInCmd(LOWERCASE) + 1)));
  }

  @Test
  void maxBytesUppercase() {
    assertEquals(BYTES_MAX - "UPPERCASE".length() - DELIMITER_BYTES, maxParamBytesInCmd(UPPERCASE));
  }

  @Test
  void maxBytesUppercaseAcceptMaxLength() {
    assertTrue(PARAM_VALID(UPPERCASE, generateSingleByteString('a', maxParamBytesInCmd(UPPERCASE))));
  }

  @Test
  void maxBytesUppercaseDenyTooLong() {
    assertFalse(PARAM_VALID(UPPERCASE, generateSingleByteString('a', maxParamBytesInCmd(UPPERCASE) + 1)));
  }

  @Test
  void maxBytesReverse() {
    assertEquals(BYTES_MAX - "REVERSE".length() - DELIMITER_BYTES, maxParamBytesInCmd(REVERSE));
  }

  @Test
  void maxBytesReverseAcceptMaxLength() {
    assertTrue(PARAM_VALID(REVERSE, generateSingleByteString('a', maxParamBytesInCmd(REVERSE))));
  }

  @Test
  void maxBytesReverseDenyTooLong() {
    assertFalse(PARAM_VALID(REVERSE, generateSingleByteString('a', maxParamBytesInCmd(REVERSE) + 1)));
  }
}

ServerValidator:

package dev.util;

import dev.Command;

import java.util.Arrays;
import java.util.function.Predicate;

public final class ServerValidator {

  static final int PORT_MIN = 0;
  static final int PORT_MAX = 65535;
  public static final int BYTES_MAX = 255;
  private static final int[] PORTS_RESERVED = new int[] {0};

  public static boolean isPortValid(String portString) {
    try {
      int port = Integer.parseInt(portString);
      return portValidator.test(port);
    } catch (NumberFormatException e) {
      return false;
    }
  }

  private static final Predicate<Integer> portValidator = port -> {
    if (port < PORT_MIN)
      return false;
    if (port > PORT_MAX)
      return false;
    return Arrays.stream(PORTS_RESERVED).noneMatch(i -> i == port);
  };

  public static boolean isPwValid(String pw) { return pwValidator.test(pw); }

  private static final Predicate<String> pwValidator = pw -> {
    if (pw.isEmpty())
      return false;
    if (pw.contains("\n"))
      return false;
    return StringHelper.countBytesInString(pw) <= maxParamBytesInCmd(Command.SHUTDOWN);
  };

  public static boolean PARAM_VALID(Command cmd, String parameter) {
    if (parameter == null || parameter.isEmpty())
      return false;
    if (cmd.equals(Command.BYE))
      return false;

    return StringHelper.countBytesInString(parameter) <= maxParamBytesInCmd(cmd);
  }

  public static int maxParamBytesInCmd(Command cmd) {
    if (cmd.equals(Command.BYE))
      return 0;
    return BYTES_MAX - StringHelper.countBytesInString(cmd.name()) - StringHelper.countBytesInString(Command.DELIMITER);
  }
}

StringHelper

StringHelperTest:

package dev.util;

import org.junit.jupiter.api.Test;

import static dev.util.ServerExceptions.SyntaxError;
import static dev.util.StringHelper.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

class StringHelperTest {

  @Test
  void lowercaseFromLowercase() throws Exception {
    assertEquals("abc", toLowerCase("abc"));
  }

  @Test
  void lowercaseNoEmptyString() {
    assertThrows(SyntaxError.class, () -> toLowerCase(""));
  }

  @Test
  void lowercaseNoNullString() {
    assertThrows(SyntaxError.class, () -> toLowerCase(null));
  }

  @Test
  void lowercaseFrom1ASCII() throws Exception {
    assertEquals("z", toLowerCase("Z"));
  }

  @Test
  void lowercaseFromAlphabet() throws Exception {
    assertEquals("abcdefghijklmnopqrstuvwxyz", toLowerCase("ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
  }

  @Test
  void lowercaseFromSpecialChar() throws Exception {
    assertEquals("!", toLowerCase("!"));
  }

  @Test
  void lowercaseFromMixedCase() throws Exception {
    assertEquals("abcdefg", toLowerCase("AbCdEfG"));
  }

  @Test
  void lowercaseFromMixedChars() throws Exception {
    assertEquals("a!b?c§d&e", toLowerCase("A!B?C§D&E"));
  }

  @Test
  void lowercaseFromSingleUmlaut() throws Exception {
    assertEquals("ä", toLowerCase("Ä"));
  }

  @Test
  void lowercaseFromUmlauts() throws Exception {
    assertEquals("äöü", toLowerCase("ÄÖÜ"));
  }

  @Test
  void lowercaseFromLigature() throws Exception {
    assertEquals("ß", toLowerCase("ß"));
  }

  @Test
  void uppercaseFromUppercase() throws Exception {
    assertEquals("ABC", toUpperCase("ABC"));
  }

  @Test
  void uppercaseNoEmptyString() {
    assertThrows(SyntaxError.class, () -> toUpperCase(""));
  }

  @Test
  void uppercaseNoNullString() {
    assertThrows(SyntaxError.class, () -> toUpperCase(null));
  }

  @Test
  void uppercaseFromSingleASCII() throws Exception {
    assertEquals("Z", toUpperCase("z"));
  }

  @Test
  void uppercaseFromASCIIAlphabet() throws Exception {
    assertEquals("ABCDEFGHIJKLMNOPQRSTUVWXYZ", toUpperCase("abcdefghijklmnopqrstuvwxyz"));
  }

  @Test
  void uppercaseFromSpecialChar() throws Exception {
    assertEquals("!", toUpperCase("!"));
  }

  @Test
  void uppercaseFromMixedCase() throws Exception {
    assertEquals("ABCDEFG", toUpperCase("AbCdEfG"));
  }

  @Test
  void uppercaseFromMixedChars() throws Exception {
    assertEquals("A!B?C§D&E", toUpperCase("a!b?c§d&e"));
  }

  @Test
  void uppercaseFromSingleUmlaut() throws Exception {
    assertEquals("Ä", toUpperCase("ä"));
  }

  @Test
  void uppercaseFromUmlauts() throws Exception {
    assertEquals("ÄÖÜ", toUpperCase("äöü"));
  }

  @Test
  void uppercaseFromLigature() throws Exception {
    assertEquals("GROSS", toUpperCase("groß"));
  }

  @Test
  void reverseSingleChar() throws Exception {
    assertEquals("a", reverseString("a"));
  }

  @Test
  void reverseNoEmptyString() {
    assertThrows(SyntaxError.class, () -> reverseString(""));
  }

  @Test
  void reverseNoNullString() {
    assertThrows(SyntaxError.class, () -> reverseString(null));
  }

  @Test
  void reverseEvenNumberOfLetters() throws Exception {
    assertEquals("dcba", reverseString("abcd"));
  }

  @Test
  void reverseUnevenNumberOfLetters() throws Exception {
    assertEquals("edcba", reverseString("abcde"));
  }

  @Test
  void reverseMixedChars() throws Exception {
    assertEquals("E&D§C?B!A", reverseString("A!B?C§D&E"));
  }

  @Test
  void countBytesInStringMaxLength() {
    assertEquals(255, countBytesInString(generateSingleByteString('x', 255)));
  }

  @Test
  void countBytesInEmptyString() {
    assertEquals(0, countBytesInString(""));
  }

  @Test
  void countBytesInNullString() {
    assertEquals(0, countBytesInString(null));
  }

  @Test
  void countBytesInControlChars() {
    assertEquals(3, countBytesInString("\n\n\n"));
  }

  @Test
  void count2ByteChars() {
    assertEquals(2 * 3, countBytesInString("äöü"));
  }

  @Test
  void count3ByteChars() {
    assertEquals(3 * 3, countBytesInString("☺☻♥"));
  }

  @Test
  void count4ByteChars() {
    assertEquals(4 * 3, countBytesInString("𩸽𩸽𩸽"));
  }

  @Test
  void countBytesMixedSizeChars() {
    assertEquals(1 + 2 + 3 + 4, countBytesInString("aä☺𩸽"));
  }
}

StringHelper:

package dev.util;

import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Locale;

public class StringHelper {

  public static String toLowerCase(String text) throws ServerExceptions.SyntaxError {
    if (text == null || text.isEmpty())
      throw new ServerExceptions.SyntaxError("Leere Zeichenkette für LOWERCASE");
    return text.toLowerCase(Locale.GERMAN);
  }

  public static String toUpperCase(String text) throws ServerExceptions.SyntaxError {
    if (text == null || text.isEmpty())
      throw new ServerExceptions.SyntaxError("Leere Zeichenkette für UPPERCASE");
    return text.toUpperCase(Locale.GERMAN);
  }

  public static String reverseString(String text) throws ServerExceptions.SyntaxError {
    if (text == null || text.isEmpty())
      throw new ServerExceptions.SyntaxError("Leere Zeichenkette für REVERSE");
    if (text.length() == 1)
      return text;
    return new StringBuilder(text).reverse().toString();
  }

  static int countBytesInString(String input) {
    if (input == null)
      return 0;
    return input.getBytes(StandardCharsets.UTF_8).length;
  }

  // Das hier wird wohl nur von Tests benutzt, also sollte es auch in den Tests stecken
  static String generateSingleByteString(char character, int sizeInBytes) {
    if (sizeInBytes < 1 || String.valueOf(character).getBytes(StandardCharsets.UTF_8).length > 1)
      return "";

    byte[] testBytes = new byte[sizeInBytes];
    Arrays.fill(testBytes, String.valueOf(character).getBytes(StandardCharsets.UTF_8)[0]);

    return new String(testBytes, StandardCharsets.UTF_8);
  }
}

Sonstiges

ServerExceptions:

package dev.util;
public class ServerExceptions extends Exception {
  private ServerExceptions() { /* _ */
  }

  private ServerExceptions(String message) { super("ERROR " + message); }

  public static final class SyntaxError extends ServerExceptions {
    public SyntaxError(String message) { super("SYNTAX ERROR " + message); }
  }

  public static class UnknownCommand extends Exception {
    public UnknownCommand() { super("UNKNOWN COMMAND"); }
  }
}

Command:

package dev;

import dev.util.ServerValidator;

public enum Command {
  BYE("BYE", "Sitzung beenden", null),
  LOWERCASE("LOWERCASE", "Zeichenkette in Kleinbuchstaben umwandeln", String.class),
  REVERSE("REVERSE", "Zeichenkette umkehren", String.class),
  SHUTDOWN("SHUTDOWN", "Sitzung beenden und Server herunterfahren", String.class),
  UPPERCASE("UPPERCASE", "Zeichenkette in Großbuchstaben umwandeln", String.class);

  private final String textValue;
  private final String description;
  private final Class paramType;

  String getDescription() { return description; }

  String getParamTypText() { return paramType == null ? null : paramType.getSimpleName(); }

  String getTextValue() { return textValue; }

  public static final String DELIMITER = " ";

  Command(String value, String description, Class paramType) {
    this.textValue = value;
    this.description = description;
    this.paramType = paramType;
  }

  static String getCmdInfos() {
    StringBuilder builder = new StringBuilder();

    builder.append(String.format("\n%10s\t%10s\n", "Befehl", "Parameter")).append("----------------------------------------------\n");

    for (Command cmd : Command.values()) {
      int maxBytes = ServerValidator.maxParamBytesInCmd(cmd);

      builder.append(String.format("%10s\t <%s%s>\n", cmd.getTextValue(), cmd.getParamTypText(), maxBytes == 0 ? "" : String.format(" (max. %d bytes)", maxBytes)))
          .append(String.format("%10s\t %s", "", cmd.getDescription()))
          .append("\n\n");
    }
    builder.append("----------------------------------------------\n");

    return builder.toString();
  }
}

Server:

package dev;

import dev.util.ServerValidator;

import java.io.DataOutputStream;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketException;
import java.nio.charset.StandardCharsets;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

public final class Server implements Runnable {

  private static final int CLIENTS_MAX = 3;
  private static final String PW_DEFAULT = "geheim";
  private static final int PORT_DEFAULT = 50_000;

  private static final int TIMEOUT_IN_MS = 30_000;
  static final long UPDATE_INTERVAL_IN_MS = 200L;
  private static final long DELAY_IN_MS = 225L;

  private static Server instance = null;

  static boolean attemptShutdown(String pw) {
    if (instance == null)
      return false;

    if (instance.isValidPw(pw)) {
      instance.sessionPool.shutdown();
      if (Server.instance.sessionPool.getActiveCount() == 0) {
        Server.instance.sessionPool.shutdownNow();
      }
      return true;
    } else {
      return false;
    }
  }

  static void updateHeartbeat(long currentTimeMillis) { Server.instance.latestHeartbeat = Math.max(currentTimeMillis, Server.instance.latestHeartbeat); }

  private boolean isValidPw(String pw) { return pw.equals(getPw()); }

  static String currentTime() { return SimpleDateFormat.getTimeInstance().format(new Date()); }

  private ThreadPoolExecutor sessionPool;
  private long latestHeartbeat;
  private final String pw;
  private final int port;

  private ServerSocket welcomeSocket;

  private int getPort() { return port; }

  private String getPw() { return pw; }

  Server() {
    this(PORT_DEFAULT, PW_DEFAULT);
    System.out.println("* Starte Server mit Standard-Parametern *");
  }

  Server(int port, String pw) {
    this.port = port;
    this.pw = pw;
    Server.instance = this;
  }

  private void denyConnection(String reason) {
    try {
      denyConnection(welcomeSocket.accept(), reason);
    } catch (IOException ex) { /* _ */
    }
  }

  private void denyConnection(Socket socket, String reason) {
    if (socket == null) {
      denyConnection(reason); // öhh, das ruft eine Methode auf, die wiederum diese Methode aufruft.. gefährlich
      return;
    }

    try (final DataOutputStream outputStream = new DataOutputStream(socket.getOutputStream())) {
      System.out.printf("[%s] Sitzung verweigert für: %s\n", currentTime(), socket.getRemoteSocketAddress());

      byte[] msgBytes = String.format("Verbindung abgelehnt: %s", reason).getBytes(StandardCharsets.UTF_8);
      for (byte b : msgBytes) {
        if (b == '\n' || b == '\r')
          break;
        outputStream.write(b);
      }
      outputStream.write('\n');

      outputStream.flush();
    } catch (IOException ex) { /* _ */
    }
  }

  private void startServer() throws IOException {
    this.welcomeSocket = new ServerSocket();
    welcomeSocket.setReceiveBufferSize(ServerValidator.BYTES_MAX);
    welcomeSocket.bind(new InetSocketAddress(getPort()), 1);
    this.sessionPool = new ThreadPoolExecutor(CLIENTS_MAX, CLIENTS_MAX, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(CLIENTS_MAX));

    System.out.printf("Server gestartet. Port: %d, PW: %s\n", getPort(), getPw());
    System.out.printf("Erlaubte aktive Verbindungen: %d; Erlaubte Warteschlangengröße: %d\n", sessionPool.getCorePoolSize(),
                      sessionPool.getMaximumPoolSize() - sessionPool.getCorePoolSize());
  }

  @Override
  public void run() {
    Thread.currentThread().setName(this.toString());

    try {
      startServer();

      Thread welcomeThread = new Thread(() -> {
        while (true) {
          if (sessionPool.getActiveCount() >= CLIENTS_MAX || sessionPool.isTerminating()) {
            denyConnection("Keine freien Plätze, später versuchen");
          }

          if (sessionPool.getActiveCount() < CLIENTS_MAX && !sessionPool.isTerminating()) {
            Socket socket = null;
            try {
              Thread.sleep(DELAY_IN_MS);
              socket = new Socket();
              socket.setReceiveBufferSize(ServerValidator.BYTES_MAX);
              socket.setSendBufferSize(ServerValidator.BYTES_MAX);
              socket = welcomeSocket.accept();
              Session session = new Session(socket);
              sessionPool.execute(session);
            } catch (SocketException ignore) {
              denyConnection(socket, "Kann keine Verbindung herstellen");
            } catch (RejectedExecutionException ignore) {
              denyConnection(socket, "keine freien Plätze");
            } catch (IOException e) {
              e.printStackTrace();
            } catch (InterruptedException ignore) {
              return;
            }
          }
        }
      });

      welcomeThread.start();

      while (true) {
        if (sessionPool.isTerminated()) {
          if (welcomeThread.isAlive()) {
            welcomeThread.interrupt();
          }

          System.out.println("Keine aktiven Clients mehr, Server wird sofort heruntergefahren...");
          sessionPool.shutdownNow();
          System.exit(0);
        }

        if (sessionPool.isTerminating() || sessionPool.isShutdown()) {
          if (welcomeThread.isAlive() || !welcomeThread.isInterrupted()) {
            System.out.println("Schließe Anmelde-Socket. Es werden keine neuen Verbindungen zugelassen.");
            welcomeThread.interrupt();
          }

          System.out.println("Warte auf Timeout der verbundenen Clients...");
          boolean terminated = false;

          while (!terminated) {
            try {
              terminated = sessionPool.awaitTermination(UPDATE_INTERVAL_IN_MS, TimeUnit.MILLISECONDS);
              if ((System.currentTimeMillis() - latestHeartbeat) > TIMEOUT_IN_MS) {
                if (welcomeThread.isAlive())
                  welcomeThread.interrupt();

                System.out.println("Timeout, Beenden wird erzwungen");
                sessionPool.shutdownNow();
                System.exit(0);
              }

            } catch (InterruptedException e) { /* _ */
            }
          }

          System.out.println("Keine aktiven Sitzungen mehr, Server wird jetzt heruntergefahren...");
          sessionPool.shutdownNow();
          System.exit(0);
        }

        try {
          Thread.sleep(UPDATE_INTERVAL_IN_MS);
        } catch (InterruptedException e) {
          e.printStackTrace();
        }
      }
    } catch (IOException e) {
      e.printStackTrace();
      System.exit(-1);
    }
  }

  @Override
  public String toString() {
    return String.format("Server @ Port %d", getPort());
  }
}

Überlegungen

Wo mir direkt Sachen auffielen habe ich Kommentare im Code hinterlassen
Das war im Original noch JUnit4, habe es in Junit5 umgewandelt, damit es aktueller ist
die statischen Importe für die Tests würde ich normalerweise nicht in dieser Menge benutzen, weil man damit durcheinander kommen könnte
hab da viel per Hand getestet habe; das hätte ich automatisieren können (bietet sich bei der Struktur des Enums gut an)
mit JUnit5 kann man glaube ich ganz gut parametrisiert testen, aber das hab ich mir noch nicht näher angesehen (auch mit JUnit4 wären Schleifen + Callbacks möglich gewesen, statt der vielen manuellen Tests)
wenn ich SessionTest so sehe, scheint die Session-Klasse Aufgaben zu übernehmen, die in ServerValidator oder einer eigenen Klasse besser aufgehoben wären (wenn man den Klassen-Aufbau so lassen will)

Diskussion Unit-Tests für Parametervalidierung - Codereview

Mat

Aktives Mitglied

Anhänge