Skip to content

Commit

Permalink
Google Code Style plugin and conformance OKAPI-809 (#894)
Browse files Browse the repository at this point in the history
This uses https://maven.apache.org/plugins/maven-checkstyle-plugin/ and https://checkstyle.sourceforge.io/google_style.html to fail the build if there is a code style violation.

Checkstyle's google_style.xml deviates from Google Java Style Guide:

Checkstyle requires a switch default for enums where Google Java Style Guide doesn't require it:
https://google.github.io/styleguide/javaguide.html#s4.8.4.3-switch-default
Indentation of lambdas can be wrong: https://github.com/checkstyle/checkstyle/issues?q=is%3Aopen+label%3Aindentation+lambda
Checkstyle has many other open issues regarding indentation: https://github.com/checkstyle/checkstyle/labels/indentation
Suppress false positives using https://checkstyle.sourceforge.io/config_filters.html#SuppressWarningsFilter, for example @SuppressWarnings("indentation").

These classes have been renamed (breaking change!):

CQLUtil → CqlUtil
URLDecoder → UrlDecoder
  • Loading branch information
adamdickmeiss authored Mar 31, 2020
1 parent c4cfce3 commit 6b4878c
Show file tree
Hide file tree
Showing 84 changed files with 3,338 additions and 2,092 deletions.
331 changes: 331 additions & 0 deletions checkstyle/folio_checks.xml

Large diffs are not rendered by default.

327 changes: 327 additions & 0 deletions checkstyle/google_checks.xml

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions okapi-common/src/main/java/org/folio/okapi/common/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ private Config() {
}

/**
* Inspect Check string property first in system; if not found OR empty,
* Returns string config info from properties and JSON config.
* Check string property first in system; if not found OR empty,
* inspect JSON configuration
* @param key property key (JSON key)
* @param def default value (may be null)
Expand All @@ -26,7 +27,8 @@ public static String getSysConf(String key, String def, JsonObject conf) {
}

/**
* Inspect Check boolean property first in system; if not found OR empty,
* Returns boolean config info from properties and JSON config.
* Check boolean property first in system; if not found OR empty,
* inspect JSON configuration
* @param key property key (JSON key)
* @param def default value (may be null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import org.z3950.zing.cql.CQLAndNode;
import org.z3950.zing.cql.CQLBooleanNode;
import org.z3950.zing.cql.CQLNode;
import org.z3950.zing.cql.CQLOrNode;
import org.z3950.zing.cql.CQLNotNode;
import org.z3950.zing.cql.CQLOrNode;
import org.z3950.zing.cql.CQLPrefix;
import org.z3950.zing.cql.CQLPrefixNode;
import org.z3950.zing.cql.CQLProxNode;
Expand All @@ -15,11 +15,18 @@
import org.z3950.zing.cql.Modifier;
import org.z3950.zing.cql.ModifierSet;

public class CQLUtil {
private CQLUtil() {
public class CqlUtil {
private CqlUtil() {
throw new IllegalStateException("CQLUtil");
}

/**
* Evaluate CQL tree with limit.
* @param vn1 The CQL query
* @param tn Term node that is used for comparison
* @param cmp Comparison handler
* @return true if query is not limited by tn+cmp; false if query is limited
*/
public static boolean eval(CQLNode vn1, CQLTermNode tn, Comparator<CQLTermNode> cmp) {
if (vn1 instanceof CQLBooleanNode) {
CQLBooleanNode n1 = (CQLBooleanNode) vn1;
Expand All @@ -32,7 +39,8 @@ public static boolean eval(CQLNode vn1, CQLTermNode tn, Comparator<CQLTermNode>
case NOT:
return eval(n1.getLeftOperand(), tn, cmp);
default:
throw new IllegalArgumentException("unknown operator for CQLBooleanNode: " + n1.getOperator());
throw new IllegalArgumentException("unknown operator for CQLBooleanNode: "
+ n1.getOperator());
}
} else if (vn1 instanceof CQLTermNode) {
CQLTermNode n1 = (CQLTermNode) vn1;
Expand All @@ -48,6 +56,13 @@ public static boolean eval(CQLNode vn1, CQLTermNode tn, Comparator<CQLTermNode>
}
}

/**
* Impose limit on CQL query.
* @param vn1 CQL query tree
* @param tn Term node that is used for comparison
* @param cmp Comparison handler
* @return simplified query or null if limit makes query empty
*/
public static CQLNode reducer(CQLNode vn1, CQLTermNode tn, Comparator<CQLTermNode> cmp) {
if (vn1 instanceof CQLBooleanNode) {
return reduceBoolean((CQLBooleanNode) vn1, tn, cmp);
Expand All @@ -65,8 +80,8 @@ public static CQLNode reducer(CQLNode vn1, CQLTermNode tn, Comparator<CQLTermNod
} else {
CQLSortNode sn = new CQLSortNode(n2);
List<ModifierSet> mods = n1.getSortIndexes();
for (ModifierSet mSet : mods) {
sn.addSortIndex(mSet);
for (ModifierSet mset : mods) {
sn.addSortIndex(mset);
}
return sn;
}
Expand All @@ -80,19 +95,22 @@ public static CQLNode reducer(CQLNode vn1, CQLTermNode tn, Comparator<CQLTermNod
return new CQLPrefixNode(prefix.getName(), prefix.getIdentifier(), n2);
}
} else {
throw new IllegalArgumentException("unknown type for CQLNode: " + vn1.toString());
throw new IllegalArgumentException("unknown type for CQLNode: "
+ vn1.toString());
}
}

private static CQLNode reduceBoolean(CQLBooleanNode n1, CQLTermNode tn, Comparator<CQLTermNode> cmp) {
private static CQLNode reduceBoolean(CQLBooleanNode n1, CQLTermNode tn,
Comparator<CQLTermNode> cmp) {

CQLNode n2 = null;
CQLNode left = reducer(n1.getLeftOperand(), tn, cmp);
CQLNode right = reducer(n1.getRightOperand(), tn, cmp);

ModifierSet mSet = new ModifierSet(n1.getOperator().toString().toLowerCase());
ModifierSet mset = new ModifierSet(n1.getOperator().toString().toLowerCase());
List<Modifier> mods = n1.getModifiers();
for (Modifier m : mods) {
mSet.addModifier(m.getType(), m.getComparison(), m.getValue());
mset.addModifier(m.getType(), m.getComparison(), m.getValue());
}
if (left == null) {
n2 = right;
Expand All @@ -102,24 +120,25 @@ private static CQLNode reduceBoolean(CQLBooleanNode n1, CQLTermNode tn, Comparat
switch (n1.getOperator()) {
case AND:
if (left != null && right != null) {
n2 = new CQLAndNode(left, right, mSet);
n2 = new CQLAndNode(left, right, mset);
}
break;
case OR:
if (left != null && right != null) {
n2 = new CQLOrNode(left, right, mSet);
n2 = new CQLOrNode(left, right, mset);
}
break;
case NOT:
if (left != null && right != null) {
n2 = new CQLNotNode(left, right, mSet);
n2 = new CQLNotNode(left, right, mset);
}
break;
case PROX:
if (left != null && right != null) {
n2 = new CQLProxNode(left, right, mSet);
n2 = new CQLProxNode(left, right, mset);
}
break;
default:
}
return n2;
}
Expand Down
54 changes: 23 additions & 31 deletions okapi-common/src/main/java/org/folio/okapi/common/ErrorType.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,31 @@
* Types of errors.
*/
public enum ErrorType {
/** Not really an error, but a success code */
OK,
/** Internal errors of any kind */
INTERNAL,
/** Not really an error, but a success code. */
OK(200),
/** Internal errors of any kind. */
INTERNAL(500),
/** Bad requests, etc. */
USER,
/** Stuff that is not there */
NOT_FOUND,
/** Any kind of auth or permission problem */
FORBIDDEN,
/** Error type for anything else */
ANY;
USER(400),
/** Stuff that is not there. */
NOT_FOUND(404),
/** Any kind of auth or permission problem. */
FORBIDDEN(403),
/** Error type for anything else. */
ANY(500);

private final int statusCode;

ErrorType(int code) {
statusCode = code;
}

/**
* Maps error type to HTTP status code.
* @param t error type
* @return HTTP status
*/
public static int httpCode(ErrorType t) {
int code = 500;
switch (t) {
case OK:
code = 200;
break;
case USER:
code = 400;
break;
case NOT_FOUND:
code = 404;
break;
case FORBIDDEN:
code = 403;
break;
case INTERNAL:
case ANY:
code = 500;
break;
}
return code;
return t.statusCode;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,36 @@ public class HttpClientLegacy {
private HttpClientLegacy() {
throw new IllegalStateException("HttpClientLegacy");
}

/**
* Send HTTP request with style ala Vert.x 3.
* @param client HTTP client
* @param method HTTP method
* @param socketAddress socket address
* @param url Full URL
* @param response response handler
* @return handler for HTTP request
*/
public static HttpClientRequest requestAbs(HttpClient client, HttpMethod method,
SocketAddress socketAddress, String url,
Handler<HttpClientResponse> response) {
SocketAddress socketAddress, String url,
Handler<HttpClientResponse> response) {
return client.requestAbs(method, socketAddress, url, hndlr -> {
if (hndlr.succeeded()) {
response.handle(hndlr.result());
}
});
}

/**
* Send HTTP request with style ala Vert.x 3.
* @param client HTTP client
* @param method HTTP method
* @param url Full URL
* @param response response handler
* @return handler for HTTP request
*/
public static HttpClientRequest requestAbs(HttpClient client, HttpMethod method,
String url, Handler<HttpClientResponse> response) {
String url, Handler<HttpClientResponse> response) {
return client.requestAbs(method, url, hndlr -> {
if (hndlr.succeeded()) {
response.handle(hndlr.result());
Expand All @@ -31,31 +49,64 @@ public static HttpClientRequest requestAbs(HttpClient client, HttpMethod method,
}


/**
* Send HTTP request with style ala Vert.x 3.
* @param client HTTP client
* @param method HTTP method
* @param port server port
* @param host server host
* @param response response handler
* @return handler for HTTP request
*/
public static HttpClientRequest request(HttpClient client, HttpMethod method, int port,
String host, String uri,
Handler<HttpClientResponse> response) {
String host, String uri,
Handler<HttpClientResponse> response) {
return client.request(method, port, host, uri, hndlr -> {
if (hndlr.succeeded()) {
response.handle(hndlr.result());
}
});
}

/**
* Send HTTP POST request with style ala Vert.x 3.
* @param client HTTP client
* @param port server port
* @param host server host
* @param response response handler
* @return handler for HTTP request
*/
public static HttpClientRequest post(HttpClient client, int port,
String host, String uri,
Handler<HttpClientResponse> response) {
String host, String uri,
Handler<HttpClientResponse> response) {
return request(client, HttpMethod.POST, port, host, uri, response);
}

/**
* Send HTTP GET request with style ala Vert.x 3.
* @param client HTTP client
* @param port server port
* @param host server host
* @param response response handler
* @return handler for HTTP request
*/
public static HttpClientRequest get(HttpClient client, int port,
String host, String uri,
Handler<HttpClientResponse> response) {
String host, String uri,
Handler<HttpClientResponse> response) {
return request(client, HttpMethod.GET, port, host, uri, response);
}

/**
* Send HTTP DELETE request with style ala Vert.x 3.
* @param client HTTP client
* @param port server port
* @param host server host
* @param response response handler
* @return handler for HTTP request
*/
public static HttpClientRequest delete(HttpClient client, int port,
String host, String uri,
Handler<HttpClientResponse> response) {
String host, String uri,
Handler<HttpClientResponse> response) {
return request(client, HttpMethod.DELETE, port, host, uri, response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ private HttpResponse() {

private static final Logger logger = OkapiLogger.get();

/**
* Produce HTTP response based on exception cause.
* @param ctx routing context from where HTTP response is generated
* @param t error type which maps to HTTP status
* @param cause cause for error
*/
public static void responseError(RoutingContext ctx, ErrorType t, Throwable cause) {
responseError(ctx, ErrorType.httpCode(t), cause);
}
Expand All @@ -24,6 +30,12 @@ private static void responseError(RoutingContext ctx, int code, Throwable cause)
responseError(ctx, code, cause.getMessage());
}

/**
* Produce HTTP response with status code and text/plain message.
* @param ctx routing context from where HTTP response is generated
* @param code status code
* @param msg message to be part of HTTP response
*/
public static void responseError(RoutingContext ctx, int code, String msg) {
String text = (msg == null) ? "(null)" : msg;
if (code < 200 || code >= 300) {
Expand All @@ -35,22 +47,30 @@ public static void responseError(RoutingContext ctx, int code, String msg) {
}
}

/**
* Produce HTTP response with status code and text/plain header.
* @param ctx routing context from where HTTP response is generated
* @param code status code
* @return HTTP server response
*/
public static HttpServerResponse responseText(RoutingContext ctx, int code) {
HttpServerResponse res = ctx.response();
if (!res.closed()) {
res
.setStatusCode(code)
.putHeader("Content-Type", "text/plain");
res.setStatusCode(code).putHeader("Content-Type", "text/plain");
}
return res;
}

/**
* Produce HTTP response with status code and application/json header.
* @param ctx routing context from where HTTP response is generated
* @param code status code
* @return HTTP server response
*/
public static HttpServerResponse responseJson(RoutingContext ctx, int code) {
HttpServerResponse res = ctx.response();
if (!res.closed()) {
res
.setStatusCode(code)
.putHeader("Content-Type", "application/json");
res.setStatusCode(code).putHeader("Content-Type", "application/json");
}
return res;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class MainLauncher extends Launcher {

public static void main(String[] args) {
System.setProperty("vertx.logger-delegate-factory-class-name",
"io.vertx.core.logging.Log4jLogDelegateFactory");
"io.vertx.core.logging.Log4jLogDelegateFactory");
MainLauncher m = new MainLauncher();
m.dispatch(args);
}
Expand Down
Loading

0 comments on commit 6b4878c

Please sign in to comment.