HTTP interface for tenant API. #481

sysexcontrol · 2018-02-06T17:32:13Z

Contained is the implementation of the http interface for the tenant API, including tests.

Not contained is the documentation - this should be the discussion base for the implementation.

Please do not review the first commit - it is just the squashed version of the AMQP implementation PR that was discussed already. The two commits on top are the ones for the HTTP and will be the only ones that will make it to master later.

sophokles73 · 2018-02-07T13:05:27Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

+                    response
+                            .putHeader(HttpHeaders.CONTENT_TYPE, HttpUtils.CONTENT_TYPE_JSON_UFT8)
+                            .putHeader(HttpHeaders.CONTENT_LENGTH, String.valueOf(msg.length()))
+                            .write(msg);


isn't this what setResponseBody() does?

sophokles73 · 2018-02-07T13:06:59Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

+        } catch (final DecodeException e) {
+            HttpUtils.badRequest(ctx, "body does not contain a valid JSON object");
+        }
+    }


this is the same code as doAddTenantJson. Maybe we can refactor the body extraction code into a separate method?

sophokles73 · 2018-02-07T13:08:36Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

+
+        doTenantAction(ctx, requestMsg, (status, tenantResult) -> {
+                response.setStatusCode(status);
+                if (status >=400) {


sophokles73

we seem to be duplicating test code over and over again. I have to admit that I do not like that at all because it makes maintenance harder and harder. We should start factoring out the CRUD HTTP client code. I would also actually like to move these tests to the integration test suite and have real unit tests here (without the need to start and bind a HTTP server)

sophokles73 · 2018-02-07T13:10:58Z

core/src/main/java/org/eclipse/hono/util/ProtocolAdapterConstants.java

+ * Constants used by protocol adapters of Hono.
+ *
+ */
+public final class ProtocolAdapterConstants {


can't we simply add these to Constants?

Yes, you are right.Will move them.

sophokles73 · 2018-02-07T13:11:39Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

+    public static JsonObject buildTenantPayload(final String tenantId) {
+        final JsonObject adapterDetailsHttp = new JsonObject().
+                put(TenantConstants.FIELD_ADAPTERS_TYPE, ProtocolAdapterConstants.TYPE_HTTP).
+                put(TenantConstants.FIELD_ADAPTERS_DEVICE_AUTHENTICATION_REQUIRED, "true").


use a boolean, not a string literal

sophokles73 · 2018-02-07T13:11:55Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

+        final JsonObject adapterDetailsHttp = new JsonObject().
+                put(TenantConstants.FIELD_ADAPTERS_TYPE, ProtocolAdapterConstants.TYPE_HTTP).
+                put(TenantConstants.FIELD_ADAPTERS_DEVICE_AUTHENTICATION_REQUIRED, "true").
+                put(TenantConstants.FIELD_ENABLED, "true");


use a boolean, not a string literal

sophokles73 · 2018-02-07T13:12:01Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

+                put(TenantConstants.FIELD_ENABLED, "true");
+        final JsonObject adapterDetailsMqtt = new JsonObject().
+                put(TenantConstants.FIELD_ADAPTERS_TYPE, ProtocolAdapterConstants.TYPE_MQTT).
+                put(TenantConstants.FIELD_ADAPTERS_DEVICE_AUTHENTICATION_REQUIRED, "true").


use a boolean, not a string literal

sophokles73 · 2018-02-07T13:12:06Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

+        final JsonObject adapterDetailsMqtt = new JsonObject().
+                put(TenantConstants.FIELD_ADAPTERS_TYPE, ProtocolAdapterConstants.TYPE_MQTT).
+                put(TenantConstants.FIELD_ADAPTERS_DEVICE_AUTHENTICATION_REQUIRED, "true").
+                put(TenantConstants.FIELD_ENABLED, "true");


use a boolean, not a string literal

sophokles73 · 2018-02-07T13:12:13Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

+                put(TenantConstants.FIELD_ENABLED, "true");
+        final JsonObject tenantPayload = new JsonObject().
+                put(TenantConstants.FIELD_TENANT_ID, tenantId).
+                put(TenantConstants.FIELD_ENABLED, "true").


use a boolean, not a string literal

sophokles73 · 2018-02-07T13:13:46Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

@@ -139,6 +165,18 @@ public static boolean testJsonObjectToBeContained(final JsonObject jsonObject, f
                            containResult.set(false);
                        }
                    }
+                } else if (entry.getValue() instanceof JsonArray) {
+                    if (!(jsonObject.getValue(entry.getKey()) instanceof JsonArray)) {


update the JavaDoc accordingly

👍 thanks for spotting.

sophokles73 · 2018-02-07T13:14:20Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

+                    return false;
+                }
+
+                if (testJsonObjectToBeContained((JsonObject)elemOfBiggerArray, (JsonObject)containedElem)) {


sysexcontrol · 2018-02-08T17:19:10Z

@sophokles73 : I pushed two new commits. The first one is a half rewrite of the TenantHttpEndpoint, which eliminates most code duplications existing before. Can you please have a look and tell me what you think about this way of coding it? I like it much better (although it still can be improved) - WDYT?

Not addressed the Test code duplications so far, would prefer to do this in another PR (after the release and after the merge of the Tenant API implementation).

sophokles73

I do like this approach a lot 👍
I thought about applying this style to the HTTP adapter in the past as well and I still think it would be worth refactoring it in this way. But that is something for 0.6 or 0.7, I guess :-)

sophokles73 · 2018-02-09T09:23:50Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

@@ -68,155 +69,104 @@ public String getName() {
    public void addRoutes(final Router router) {

        final String path = String.format("/%s", TenantConstants.TENANT_ENDPOINT);
+
+        final BodyHandler bodyHandler = BodyHandler.create();
+        bodyHandler.setBodyLimit(2048); // limit body size to 2kb


this should probably come from a config property

sophokles73 · 2018-02-09T09:24:49Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

    }

-    private void doTenantAction(final RoutingContext ctx, final JsonObject requestMsg, final BiConsumer<Integer, JsonObject> responseHandler) {
+    private void sendTenantAction(final RoutingContext ctx, final JsonObject requestMsg, final BiConsumer<Integer, JsonObject> responseHandler) {


we should be able to pull this up into AbstractHttpEndpoint, don't you think? There doesn't seem to be anything left that is tenant specific, is there?

sophokles73 · 2018-02-09T09:27:17Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

+            if (status >=400) {
+                setResponseBody(tenantResult, response);
+            } else if (sendResponseForStatus != null) {
+                if (sendResponseForStatus.test(status)) {


why not let another handler process the response? we can provide (and register) default handlers for the CRUD responses which can be overridden by sub-classes if appropriate

sophokles73 · 2018-02-09T09:28:47Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

+     * successful.
+     * <p>
+     * The payload is parsed to ensure it is valid JSON and is put to the RoutingContext ctx with the
+     * key {@link #KEY_REQUEST_BODY}.


this doesn't seem to be correct

sophokles73 · 2018-02-09T09:31:34Z

...s/device-registry/src/test/java/org/eclipse/hono/deviceregistry/DeviceRegistryTestUtils.java

-                put(TenantConstants.FIELD_ADAPTERS_TYPE, ProtocolAdapterConstants.TYPE_HTTP).
-                put(TenantConstants.FIELD_ADAPTERS_DEVICE_AUTHENTICATION_REQUIRED, "true").
-                put(TenantConstants.FIELD_ENABLED, "true");
+                put(TenantConstants.FIELD_ADAPTERS_TYPE, Constants.TYPE_HTTP).


we usually put the period on the new line, e.g.

new JsonObject() .put("key", value);

sysexcontrol · 2018-02-13T14:35:10Z

@sophokles73 : I pushed 2 new commits, one is a larger effort to further reduce the repeating code of the http endpoints (as already started before).
The registration and credentials endpoint are intentionally not fully refactored in here, but the base to do that afterwards is contained (and fully used for the tenant http endpoint).

Due to a necessary rebase, I needed to make a forced push (the last 2 commits are the new ones).

sophokles73 · 2018-02-13T15:33:42Z

core/src/main/java/org/eclipse/hono/util/RequestResponseApiConstants.java

@@ -43,6 +43,37 @@
    public static final String FIELD_PAYLOAD   = "payload";
    public static final String FIELD_TENANT_ID = "tenant-id";

+    /* request actions */
+    public enum Action {
+        ACTION_GET, ACTION_ADD, ACTION_UPDATE, ACTION_REMOVE, ACTION_UNKNOWN;


I do not believe that these are the same for all request/response APIs, are they?

Unfortunately: you are right. I haven't looked into the registration API, which defines more actions than this. With the Credentials API it would have been fitting (but only for now).

I will come up with a better solution, this has been too much generalized obviously...

sophokles73 · 2018-02-13T15:36:43Z

service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java

+            final RoutingContext ctx,
+            final RequestResponseApiConstants.Action action,
+            final Predicate<Integer> setResponseBodyForStatus,
+            final Function<Void,String> locationFormatter


I would instead use a Handler<HttpServerResponse> and let the handler modify the response in any way it sees fit, i.e. not limiting it to provide a location header value.

👍 much better indeed.
And this is solving another issue as well: the annoying action parameter, which was rather artificial in this signature.

sophokles73 · 2018-02-13T15:37:31Z

service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java

+     * @param requestMsg The JSON object to send via the event bus.
+     * @param responseHandler The handler to be invoked for the response received as answer from the event bus.
+     */
+    protected void sendAction(final RoutingContext ctx, final JsonObject requestMsg, final BiConsumer<Integer, JsonObject> responseHandler) {


is this method supposed to be overridden by subclasses?

No, will make it final.

sophokles73 · 2018-02-13T15:38:08Z

service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java

+     * @param ctx The routing context of the request.
+     * @return The tenantId retrieved from the request.
+     */
+    protected String getTenantParam(final RoutingContext ctx) {


sophokles73 · 2018-02-13T15:38:16Z

service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java

+     * @param ctx The routing context of the request.
+     * @return The deviceId retrieved from the request.
+     */
+    protected String getDeviceIdParam(final RoutingContext ctx) {


final?
document NullPointerException?

sophokles73 · 2018-02-13T15:39:01Z

service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java

+     *
+     * @param ctx The routing context of the request.
+     * @return The tenantId retrieved from the request.
+     */


document NullPointerException?

Please ignore in this PR, only consider commits on top of this. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

The payload is now checked for correct JSON. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

…class. This pushes the reduction of code in the always repeating http endpoint implementations a step further. Focussing on the tenant http endpoint, it is the base for rewriting (and reducing) the other endpoints as well. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

…action enum. The enum introduced for the tenant API actions is used in the credentials HTTP endpoint and service implementation as well. Any standard HTTP endpoint that supports GET,ADD,PUT,REMOVE can be based on this enum in the future. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

This allows for further reduction of code duplications in the HTTP endpoint implementations by introducing more and shorter lambdas instead. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sysexcontrol · 2018-02-14T15:13:11Z

@sophokles73 : thanks for the last comments, I (forced) pushed 2 new commits.
Since they are not in the right order in the github display, here are the relevant ones (all other are coming from the again necessary rebase and were not changed at all):
9cf619a
35abb9f

The enum action and the generic methods are applied to the credentials implementations as well, where it fits well. The Registration API is not touched (yet).
Can you have another look?

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sophokles73 · 2018-02-14T15:30:56Z

core/src/main/java/org/eclipse/hono/util/RequestResponseApiConstants.java

+     * Request standard actions to support the standard lifecycle.
+     * If more or other actions shall be used, an own enum type should be defined.
+    */
+    public enum StandardAction implements RequestResponseAction {


what is wrong with simply defining the actions that are defined for an API in the corresponding *Constants class?

Basically, not too much anymore, except for:
it would be completely identical for tenant and credentials API, and it would double about 30 code lines.
Since it often is probably the case that the HTTP APIs have get,add,update,delete, I have put it into the StandardAction.

Previously, there were generic methods operating on this type. Due to the introduced handlers this does not exist anymore, so the left reason to keep them here is purely the code duplication.

sophokles73 · 2018-02-14T15:42:47Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

@@ -75,21 +79,26 @@ public void addRoutes(final Router router) {
        router.post(path).handler(this::extractRequiredJsonPayload);
        router.post(path).handler(this::checkPayloadForTenantId);
        router.post(path).handler(ctx -> doTenantHttpRequest(ctx, StandardAction.ACTION_ADD,
-                status -> status == HttpURLConnection.HTTP_CREATED));
+                status -> status == HttpURLConnection.HTTP_CREATED,
+                tenantId -> response -> response.putHeader(HttpHeaders.LOCATION,


IMHO this does make the code very hard to read and, worse, very hard to adapt later. FMPOV it would be much cleaner to have one method per operation/action and then implement them accordingly.

I understand your point and will come up with a less condensed version of this code.

Previously, the direct use of lambdas of lambdas allowed for very short code, but the readability suffered. Now each HTTP method has it's own internal method again and the used predicates and location handler are explictly offered by the base class. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sysexcontrol · 2018-02-16T09:08:42Z

@sophokles73 : I refactored the TenantHttpEndpoint to a less condensed version, which IMHO improves readability and changeability a lot.

sophokles73 · 2018-02-16T09:35:05Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

@@ -13,17 +13,17 @@

 package org.eclipse.hono.service.tenant;

+import static java.net.HttpURLConnection.HTTP_CREATED;


please do not import the constants statically

sophokles73 · 2018-02-16T09:43:52Z

service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java

+     * @param statusToCheckFor The HTTP status, e.g. {@link HttpURLConnection#HTTP_OK}.
+     * @return The predicate.
+     */
+    protected final Predicate<Integer> getHttpStatusPredicate(final int statusToCheckFor) {


this obviously works but FMPOV it would improve readability a lot if we simply would put the lamba in the place where it is used instead of encapsulating this within a method. In the end, that's one of the reasons why lambdas have been introduced.
Compare

doTenantHttpRequest(ctx, tenantId, StandardAction.ACTION_ADD, getHttpStatusPredicate(HTTP_CREATED), getLocationHeaderHandler(location));

with

doTenantHttpRequest(ctx, tenantId, StandardAction.ACTION_ADD, status -> status == HTTP_CREATED, getLocationHeaderHandler(location));

How many bytes have we saved? Which version is easier to understand?
The same holds true for the getLocationHeaderHandler() method FMPOV.

Well, we fully agree on that - this is just like I wrote it before. Sometimes PR comments are not so clear...

The only difference with explicit getters for lambdas (or Handlers) is that they get a Javadoc and a (hopefully) good name, which sometimes can be helpful IMHO. In this case it is simple enough that this is not necessary.

Anyway, I will introduce the lambdas again and hope we both are satisfied then...

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sysexcontrol · 2018-02-16T14:11:01Z

@sophokles73 : I reintroduced the lambdas, hope we agree on this version :-) Thanks!

sophokles73 · 2018-02-19T08:20:54Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

    }

    private void updateTenant(final RoutingContext ctx) {

        final String tenantId = getTenantIdFromContext(ctx);

-        doTenantHttpRequest(ctx, tenantId, StandardAction.ACTION_UPDATE,null, null);
+        doTenantHttpRequest(ctx, tenantId, RequestResponseApiConstants.StandardAction.ACTION_UPDATE,null, null);


sophokles73 · 2018-02-19T08:21:03Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

    }

    private void removeTenant(final RoutingContext ctx) {

        final String tenantId = getTenantIdFromContext(ctx);

-        doTenantHttpRequest(ctx, tenantId, StandardAction.ACTION_REMOVE,null, null);
+        doTenantHttpRequest(ctx, tenantId, RequestResponseApiConstants.StandardAction.ACTION_REMOVE,null, null);


sophokles73 · 2018-02-19T08:26:41Z

service-base/src/main/java/org/eclipse/hono/service/tenant/TenantHttpEndpoint.java

    }

    private void updateTenant(final RoutingContext ctx) {

        final String tenantId = getTenantIdFromContext(ctx);

-        doTenantHttpRequest(ctx, tenantId, StandardAction.ACTION_UPDATE,null, null);
+        doTenantHttpRequest(ctx, tenantId, RequestResponseApiConstants.StandardAction.ACTION_UPDATE,null, null);


I think we should refer to the actions via the TenantConstants class (this is where they will end up anyway FMPOV)

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sysexcontrol · 2018-02-19T10:53:32Z

@sophokles73 : comments adressed, thx.

sysexcontrol · 2018-02-21T16:12:21Z

Commits combined and provided in PR [#501] for merge. This PR is closed instead.

sysexcontrol requested a review from sophokles73 February 6, 2018 17:32

sophokles73 reviewed Feb 7, 2018

View reviewed changes

sophokles73 requested changes Feb 7, 2018

View reviewed changes

sysexcontrol force-pushed the honoTenantApiImplHttpIfPr2 branch from 5b471ad to bb3eefa Compare February 8, 2018 17:25

sophokles73 reviewed Feb 9, 2018

View reviewed changes

sysexcontrol force-pushed the honoTenantApiImplHttpIfPr2 branch from 33c8b9d to e8cf7e0 Compare February 13, 2018 14:34

sophokles73 reviewed Feb 13, 2018

View reviewed changes

sysexcontrol added 9 commits February 14, 2018 16:03

[eclipse-hono#388] Squashed commit of all tenant AMQP commits.

b33329b

Please ignore in this PR, only consider commits on top of this. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

[eclipse-hono#388] Tenant API implementation (HTTP endpoints).

a57b0df

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

[eclipse-hono#388] Tenant API HTTP interface tests.

e68dbd8

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

[eclipse-hono#388] Condense and improve TenantHttpEndpoint code.

b522077

The payload is now checked for correct JSON. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

[eclipse-hono#388] Improvements from review.

12608c7

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

[eclipse-hono#388] Code style.

86fc5b0

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

[eclipse-hono#388] Introduced handler to postprocess HTTP responses.

35abb9f

This allows for further reduction of code duplications in the HTTP endpoint implementations by introducing more and shorter lambdas instead. Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sysexcontrol force-pushed the honoTenantApiImplHttpIfPr2 branch from 030b9b9 to 35abb9f Compare February 14, 2018 15:05

[eclipse-hono#388] Fixed bug with missing interface.

0a4393d

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sophokles73 reviewed Feb 14, 2018

View reviewed changes

sophokles73 reviewed Feb 16, 2018

View reviewed changes

[eclipse-hono#388] Use lambdas directly instead of getters.

7dab415

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sophokles73 requested changes Feb 19, 2018

View reviewed changes

[eclipse-hono#388] Formatting issues, correct constant usage.

bcbbe4b

Signed-off-by: Karsten Frank <Karsten.Frank@bosch-si.com>

sophokles73 approved these changes Feb 20, 2018

View reviewed changes

sysexcontrol mentioned this pull request Feb 21, 2018

Hono tenant api http endpoints #501

Merged

sysexcontrol closed this Feb 21, 2018

sysexcontrol deleted the honoTenantApiImplHttpIfPr2 branch March 6, 2018 16:11

		@@ -13,17 +13,17 @@

		package org.eclipse.hono.service.tenant;

		import static java.net.HttpURLConnection.HTTP_CREATED;

HTTP interface for tenant API. #481

HTTP interface for tenant API. #481

Conversation

sysexcontrol commented Feb 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophokles73 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysexcontrol commented Feb 8, 2018

sophokles73 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysexcontrol commented Feb 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysexcontrol commented Feb 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysexcontrol commented Feb 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysexcontrol commented Feb 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysexcontrol commented Feb 19, 2018

sysexcontrol commented Feb 21, 2018