Skip to content
Open
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
34 changes: 0 additions & 34 deletions .gitignore

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,82 @@
package org.hyperskill.database.clientside;

import com.beust.jcommander.JCommander;
import com.beust.jcommander.Parameter;
import com.google.gson.Gson;
import org.hyperskill.database.common.Request;
import org.hyperskill.database.common.Response;
import org.hyperskill.database.common.Util;

import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.net.Socket;

public class Client {
public static void main(String[] args) {
System.out.println("Hello World!");
@Parameter(names = {"-host", "-ip"}, description = "IP address")
String host;

@Parameter(names = {"-port"}, description = "Port", validateWith = PositiveInteger.class)
int port;

/*
@Parameter(names = "-t", description = "Command", validateWith = Commands.class)
String command;

@Parameter(names = "-m", description = "Text to save")
String text;

@Parameter(names = "-i", description = "Index")
String index;
*/

@Parameter(names = "-in", description = "Input file")
String inputFile;

@Parameter(names = "-out", description = "Output file")
String outputFile;

public static void main(String... argv) {
Client client = new Client();
JCommander.newBuilder().addObject(client).build().parse(argv);

if (argv.length < 6) {
System.err.println(
"Usage: java -jar client.jar -ip <hostname> -port <port> -in <filename> [-out <filename>]");
System.exit(1);
}

try (
Socket socket = new Socket(client.host, client.port);
DataInputStream input = new DataInputStream(socket.getInputStream());
DataOutputStream output = new DataOutputStream(socket.getOutputStream())
) {
System.out.println("ifile" + client.inputFile);

Request request = Util.read(client.inputFile);
Gson gson = new Gson();
String out = gson.toJson(request);
output.writeUTF(out);
System.out.println("Sent: " + out);
String receivedMsg = input.readUTF(); // response message
System.out.println("Received: " + receivedMsg);
Response response = gson.fromJson(receivedMsg, Response.class);
if (response.isOk()) {
System.out.println("Request was successful");
if (client.outputFile != null && request.getType()== Request.TYPE.GET){
System.out.println("Value saved to " + client.outputFile);
Util.write(response, client.outputFile);
}
} else {
System.out.println("Request was not successful");
}
} catch (IOException e) {
e.printStackTrace();
}
}
}





Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.hyperskill.database.clientside;

import com.beust.jcommander.IParameterValidator;
import com.beust.jcommander.ParameterException;

public class Commands implements IParameterValidator {

Choose a reason for hiding this comment

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

Better name it CommandValidator, cause just Commands is ambiguous
The name of a class should reflect on what it does

public void validate(String name, String value)
throws ParameterException {
value = value.trim().toLowerCase();

Choose a reason for hiding this comment

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

It's a bad practice to reassign input parameter
Better treat and mark them as final
More and more languages do that by default

if (!(value.equals("set") || value.equals("get") || value.equals("delete"))) {

Choose a reason for hiding this comment

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

That's fine, except for potential NPE, which I described in Client class
But could be simplified by creating a Set of allowed commands and checking if passed value is in this set
The set could also be used in exception message to create a string of allowed commands allowed.stream().collect(Collectors.joining("/"))

throw new ParameterException("Parameter " + name + " should be set/get/delete (found " + value + ")");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.hyperskill.database.clientside;

import com.beust.jcommander.IParameterValidator;
import com.beust.jcommander.ParameterException;

public class PositiveInteger implements IParameterValidator {

Choose a reason for hiding this comment

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

Same with naming as with Commands

public void validate(String name, String value)
throws ParameterException {
int n = Integer.parseInt(value);
if (n < 0) {
throw new ParameterException("Parameter " + name + " should be positive (found " + value + ")");
}
}
}
49 changes: 49 additions & 0 deletions common/src/main/java/org/hyperskill/database/common/Request.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package org.hyperskill.database.common;

import java.util.Objects;

public class Request {
public enum TYPE {SET, GET, DELETE}

Choose a reason for hiding this comment

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

For now it's okay, cause you use it only inside the Request class, but if you'd try to pass TYPE as a parameter somewhere else, you'll have to create an instance of 'Request' to access the enum. To avoid it make it static

Also, enum is just another class, and so follows the same naming convention (upper camel case, instead of all-caps). The values of enum are constants, and so they are written in caps, but not the enum itself

private TYPE type;
private String key;
private String value;

public Request(String type, String key, String value) throws IllegalArgumentException {

Choose a reason for hiding this comment

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

Why not use TYPE enum as type parameter type instead of String
Usage of String here only introduces potential failure and complicates logic where you additionally need to parse it

Objects.requireNonNull(type);
Objects.requireNonNull(key);

this.type = validateCommand(type);

if (this.type == TYPE.SET && value == null) {
throw new IllegalArgumentException("Set command requires value, got null");
}
this.key = key;
this.value = value;
}

public TYPE getType() {
return type;
}

public String getKey() {
return key;
}

public String getValue() {
return value;
}

public static TYPE validateCommand(String word) {

Choose a reason for hiding this comment

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

It's not validation but parsing
Also, it could be done with Enum::valueOf method, which will also throw IllegalArgumentException on unknown parameter

switch (word.trim().toLowerCase()) {
case "set":
return TYPE.SET;
case "get":
return TYPE.GET;
case "delete":
return TYPE.DELETE;
default:
throw new IllegalArgumentException(word + " is not allowed command");
}

}
}
48 changes: 48 additions & 0 deletions common/src/main/java/org/hyperskill/database/common/Response.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.hyperskill.database.common;

public class Response {
public enum TYPE {OK, FAIL}

Choose a reason for hiding this comment

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

Same here as in Request


public boolean isOk() {
if (this.type == TYPE.OK) {

Choose a reason for hiding this comment

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

Again, when your method returns boolean, you can just return boolean expression used in if clause

return true;
}
return false;
}

public TYPE getType() {

Choose a reason for hiding this comment

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

Better have class parameters at the top of its definition. This way you and everyone else can clearly see what this class is constructed of
Your whole structure here is upside-down
It usually goes as follows:
– parameters
– code blocks
– constructors
– getters/setters
– methods
– utility

return type;
}

private TYPE type;

public String getValue() {
return value;
}

public String getReason() {
return reason;
}

private String value;
private String reason;

public Response(String type, String value, String reason) {
switch (type.trim().toLowerCase()) {
case "ok":
this.type = TYPE.OK;
break;
case "fail":
this.type = TYPE.FAIL;
break;
default:
throw new IllegalArgumentException(type + " is not allowed command");
}

if (this.type == TYPE.FAIL && reason == null) {
throw new IllegalArgumentException("FAIL response requires reason, got null");
}
this.reason = reason;
this.value = value;
}
}
58 changes: 58 additions & 0 deletions common/src/main/java/org/hyperskill/database/common/Util.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package org.hyperskill.database.common;

import java.io.*;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Objects;

public class Util {

Choose a reason for hiding this comment

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

Creating an "utils" class is tempting but actually is a bad practice. It tempts you to put unrelated stuff together into one class
Here you do IO operations, so the name should convey this

public static Request read(String filename) throws IOException, IllegalArgumentException {
Path file = Paths.get(filename);
String command = null;
String key = null;
StringBuilder value = new StringBuilder();
try (InputStream in = Files.newInputStream(file);

Choose a reason for hiding this comment

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

This whole thing could have been implemented in a couple of lines, using Files.lines(file)
But if you don't know about streams, it's ok

BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) {
String line = null;
int lineProcessing = 0;
while ((line = reader.readLine()) != null) {
switch (lineProcessing) {
case 0:
command = line;
lineProcessing++;
break;
case 1:
key = line;
lineProcessing++;
break;
case 2:
value.append(line);
lineProcessing++;
break;
default:
value.append(" ");
value.append(line);
lineProcessing++;
break;
}
}
}
return new Request(command, key, value.toString());
}

public static void write(Response response, String outFileName) {
Objects.requireNonNull(response, "Response can't be null in Util.write");
Objects.requireNonNull(outFileName, "outFileName can't be null in Util.write");
try (FileWriter fw = new FileWriter(outFileName, false);
BufferedWriter bw = new BufferedWriter(fw);
PrintWriter out = new PrintWriter(bw)) {
out.println(response.getType());
out.println(response.getValue());

} catch (IOException e) {
System.out.println(e.toString());
}
}
}
8 changes: 4 additions & 4 deletions server/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@
</properties>

<dependencies>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
</dependency>
<dependency>
<groupId>com.beust</groupId>
<artifactId>jcommander</artifactId>
Expand All @@ -30,6 +26,10 @@
<groupId>org.hyperskill.database</groupId>
<artifactId>common</artifactId>
</dependency>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.hyperskill.database.serverside;

import com.google.gson.JsonElement;
import org.hyperskill.database.common.Request;
import org.hyperskill.database.common.Response;

import java.util.Objects;

public class JsonProtocol {

}
Loading