Basic example of a java sse plugin #13

LinneaStovring-Nielsen · 2017-09-27T09:34:34Z

Status

[ ] Under development
[x] Waiting for code review
[x] Waiting for merge

Information

[ ] Contains breaking changes
[ ] Contains new API(s)
[x] Contains documentation
[ ] Contains test
[x] Contains examples

To-do list

[x] [The secure connection in the example is statically linked to OpenSSL and APR-lib. It is possible to do this dynamically, see https://github.com/grpc/grpc-java/blob/master/SECURITY.md. Have anyone done this? ]

qlikmats

In general, without having read the code in detail, I suggest trying to split the code over several classes/files, f.e. server (lifecycle) related code in one file, plugin impl. in another etc. if there is a way to do this that makes sense :)

qlikmats · 2017-09-27T12:57:48Z

examples/java/PluginServer.java

+                            .parseFrom(metadata.get(Metadata.Key.of("qlik-functionrequestheader-bin", BINARY_BYTE_MARSHALLER))); //Java 8
+                            logger.finest("Function request header.");
+                            logHeader(responseHeaders); //Java 8
+                            if(header.getFunctionId()==5) {


spaces surrounding e.g. assignment operator, comparison (<, >, == etc.). I'll leave this one comment, but there are several places throughout the file where this should be fixed.

qlikmats · 2017-09-27T13:04:40Z

examples/java/PluginServer.java

+        private ServerSideExtension.BundledRows evalScript(ServerSideExtension.ScriptRequestHeader header) {
+
+            logger.fine("evalScript called");
+            ServerSideExtension.BundledRows.Builder builder = ServerSideExtension.BundledRows.newBuilder();


A more descriptive name for builder would be nice, that indicates what is being built when you f.e. see builder.build()

qlikmats · 2017-09-27T13:06:39Z

examples/java/PluginServer.java

+            return builder.build();
+        }
+
+        private ServerSideExtension.BundledRows evalScript(ServerSideExtension.ScriptRequestHeader header, ServerSideExtension.BundledRows bundledRows) {


Seems like much code in common with previous method. Could it be refactored to avoid code duplication?

qlikmats · 2017-09-27T13:10:11Z

examples/java/PluginServer.java

+            }
+
+            //final ServerSideExtension.BundledRows.Builder builder = ServerSideExtension.BundledRows.newBuilder();
+            //&& header.getFunctionType()==ServerSideExtension.FunctionType.SCALAR


Remove code that is commented out.

qlikmats · 2017-09-28T08:38:57Z

examples/java/README.md

+The plugin is partly asynchronous since several method calls can be made to the plugin at the same time. But a `StreamObserver` (which sends data to and from the plugin) can only handle one call at a time, and they need 
+to be in a specific order, therefore the plugin cannot be fully asynchronous.
+
+## Prerequisites


Does this have to be done by the user? Can't we have this structure from the beginning, to make it easier?

qlikmats · 2017-09-28T08:55:50Z

examples/java/README.md

+**In a command prompt:** go to the grpc-java directory, run `gradlew.bat build` and `gradlew.bat install`. This installs grpc in your local maven repository (usually located at `C:/Users/<user>/.m2`).
+
+## Changing the default logging
+Move the `javapluginlogger.properties` file to the `my-app/target` directory (it needs to be in the same directory as the `my-app-1.0-SNAPSHOT.jar` file).


I don't know maven, but I suspect that this file could be moved automatically during the build?

qlikmats

I am not sure I remember enough Java, but isn't it best practice to use package declaration(s) and matching folder structure(s)? So, f.e. create a structure (under /src/main/java) like: com/qlik/sse and declare a package com.qlik.sse in PluginServer.java?

qlikmats · 2017-09-28T10:45:25Z

examples/java/README.md

+In this file you can edit the settings for the logging. There are different [levels](https://docs.oracle.com/javase/7/docs/api/java/util/logging/Level.html) of logging depending on how much 
+information you want (`OFF`, `SEVERE`, `WARNING`, `INFO`, `CONFIG`, `FINE`, `FINER`, `FINEST`, `ALL`).
+
+* `.level`: Sets the default level for all loggers, in this case they are set to info. 


I think a short comment about logging, with a reference to the logger documentation is sufficient.

qlikmats · 2017-09-28T10:50:26Z

examples/java/README.md

+Install Qlik Sense Desktop June release or later.
+To be able to connect to Sense you need to specify which port Sense should look for a plugin at. Go to `Documents/Qlik/Sense` and create or edit the `Settings.ini` file so that it includes 
+the line `SSEPlugin=javaPlugin,localhost:50071`. `javaPlugin` is the name of the plugin. This can be whatever you want but since the name of the plugin in the example app is set to `javaPlugin` the 
+example will not work if you set it to something else (unless you change the name of the plugin in every call to the plugin in the app). `50071` is the port. You should change this to `<Port number>` 


Keep it short, like: "'javaPlugin' is an arbitrary name, that needs to match the name used in Sense, when referencing the plugin."

qlikmats · 2017-09-28T10:51:14Z

examples/java/README.md

+in the `generated-sources` folder.
+
+### If you are having trouble...
+It is possible to install the protobuff and the grpc packages manually if they for some reason are not downloaded by maven when you run `mvn package`. If you need to do this, install the protocol buffer first


qlikmats · 2017-09-28T10:53:07Z

examples/java/README.md

+with input parameters if it is not an aggregation and if the script can be evaluated for one row of input parameters independently of the other rows of parameters. The scripts being evaluated however are JavaScript scripts 
+since there is no "eval" function in java.
+
+Synchronous/asynchronous:


For me, this section doesn't provide much useful info for the purpose of running an example plugin.

LinneaStovring-Nielsen · 2017-09-29T10:04:00Z

I have now split the code into three files and updated the documentation to reflect that. I have also (hopefully) fixed the problems pointed out by @qlikmats except for the part about storing packages as com/qlik/sse since maven automatically stores them in the local repository when you run mvn package.

qlikmats · 2017-10-02T09:40:48Z

examples/java/basic_example/src/main/java/Plugin/PluginServer.java

+                } catch (Exception e) {
+                    logger.log(Level.WARNING, "Invalid port, using default value: " + port);
+                }
+            } else if (args[i].equals("--pemDir")) {


What if --pemDir is given as e.g. "./some/dir"? Since it's concatenated with the .pem file name, the dir needs to have a trailing slash, right? See if there is some path utility library that can handle this, so dir can be given both with and without trailing slash.

I hadn't thought about that, fixed now.

qlikmats · 2017-10-02T12:04:57Z

examples/java/basic_example/src/main/java/Plugin/JavaPlugin.java

+             .setPluginVersion("v1.0.0")
+             .addFunctions(ServerSideExtension.FunctionDefinition.newBuilder()
+                 .setName("HelloWorld")
+                 .setFunctionId(0)


Define constants for function IDs since they are also switched upon elsewhere in the code.

qlikmats · 2017-10-02T12:05:43Z

examples/java/basic_example/src/main/java/Plugin/JavaPlugin.java

+                 switch(functionId) {
+                     case 2: responseObserver.onNext(bundledRowsBuilder.addRows(ServerSideExtension.Row.newBuilder().addDuals(ServerSideExtension.Dual.newBuilder().setNumData(sum(columnSum)))).build());
+                         break;
+                     case 3: responseObserver.onNext(bundledRowsBuilder.addRows(ServerSideExtension.Row.newBuilder().addDuals(ServerSideExtension.Dual.newBuilder().setStrData(stringBuilder.toString()))).build());


I think the lines are too long.

qlikmats · 2017-10-02T12:07:02Z

examples/java/basic_example/src/main/java/Plugin/JavaPlugin.java

+         };
+     }
+
+     private ServerSideExtension.BundledRows evalScript(ServerSideExtension.ScriptRequestHeader header, ServerSideExtension.BundledRows bundledRows) {


Too long line.

qlikmats · 2017-10-02T12:07:22Z

examples/java/basic_example/src/main/java/Plugin/JavaPlugin.java

+         return bundledRowsBuilder.build();
+     }
+
+     private boolean evalScript(String script, ServerSideExtension.BundledRows.Builder bundledRowsBuilder, ScriptEngine engine, ServerSideExtension.DataType returnType) {


Too long line.

qlikmats · 2017-10-02T12:08:07Z

examples/java/basic_example/src/main/java/Plugin/PluginServerInterceptor.java

+                    ServerSideExtension.FunctionRequestHeader header = ServerSideExtension.FunctionRequestHeader
+                    .parseFrom(metadata.get(Metadata.Key.of("qlik-functionrequestheader-bin", BINARY_BYTE_MARSHALLER))); //Java 8
+                    logger.finest("Function request header.");
+                    logHeader(responseHeaders); //Java 8


What does the "Java 8" comments mean?

That you need java 8 for this to work. Should I remove it or make the comment more clear?

Prerequisites already states Java 8 as a requirement, so I think it can be removed.

kjartan-qlik · 2017-10-02T09:17:57Z

examples/java/basic_example/src/main/proto/ServerSideExtension.proto

@@ -0,0 +1,131 @@
+/*


The proto file is already included in the repository as proto/ServerSideExtension.proto. Is it not possible for you to refer to that one instead of including another copy of it?

Both yes and no. As I have understood it, standard Maven practice is to include the .proto file like this (and if you add tests you add another copy of it). I can probably find another way to do it. Then it is a question about if it should be possible to download just the basic_example folder or if you need to download everything to run the example?

kjartan-qlik · 2017-10-02T09:18:47Z

examples/java/basic_example/README.md

+# Running the example
+
+## Prerequisites
+This example was created using maven, so the easiest way to run the example is to install [maven](http://maven.apache.org/ ). 


Maven is written with capital 'M'.

kjartan-qlik · 2017-10-02T09:21:23Z

examples/java/basic_example/README.md

+## Prerequisites
+This example was created using maven, so the easiest way to run the example is to install [maven](http://maven.apache.org/ ). 
+
+There are other ways to do this. If you don't use maven, what you need is the protocol buffer package and the grpc package (in that order) and the protoc 


What does 'this' refer to? Building the example?

kjartan-qlik · 2017-10-02T09:22:13Z

examples/java/basic_example/README.md

+## Generate the java plugin (and the classes from the .proto file)
+**In a command prompt:** go to the `basic_example` directory and run `mvn package`.
+
+Now, got to the `target` directory in the `basic_example` directory. In the `target` folder, you can find a jar file, that is the plugin. The code generated from the `.proto` file can be found 


'go to the..'

Add an sse java plugin to later be used as a basic example.

Add evaluate script to the plugin so that it can be used as a basic example showing how to do script evaluation in a java plugin.

Add more functions that can be executed to make the example more useful. Also add the pom.xml file to show which dependencies are needed.

Add functions for showing how the cache can be turned of to be consistent with other basic examples.

Add a properties file for the logging so that the plugin user can set how much logging will be done. Also add a secure conection so that a secure connection with Qlik Sense Server is possible.

Add documentaion so that it becomes easier for others to create their own sse java plugin. Some cleanup to make the code easier to read.

tobiaslindulf

add a .gitignore file in basic_example folder with the following content:

dependency-reduced-pom.xml
/target/*
!/target/javapluginlogger.properties

cjpersson · 2017-10-08T09:12:10Z

I was asked to review this (I guess) because we mainly work with Java and that we have made a prototype with a Java SSE. So here are my comments.

It is great to get an example to start with! It will save a lot of time building SSEs using Java.

One thing that is a bit awkward in Java when building SSEs is the way incoming metadata is handled compared to how grpc is integrated in other languages. It would be helpful to explain the purpose of the interceptor and the thread local storage used for metadata. That is a good insight that it takes some time to get when starting building Java SSEs. A comment in the code explaining this mechanism in PluginServerInterceptor and JavaPlugin would be good.

Folders and package names must match. Normally the structure used here should not compile. I guess the build script does something unconventional to make it compile.

Do not import classes in same package. They are automatically visible.

Use package names starting with a lower case letter. The correct thing to use here would be something like com.qlik.sse.basicExample but for an example it might be easier with folder hierarchies that are not that deep so qlik.basicExample is ok. That would imply that the source files would be in src/main/java/qlik/basicExample.

LinneaStovring-Nielsen · 2017-10-09T08:00:14Z

@cjpersson Thank you for your comments! On the metadata explanation, would it be better to have a more detailed explanation in the documentation or do you want that explanation in the code? or both?

qlikmats · 2017-10-11T08:49:24Z

examples/java/basic_example/src/main/java/com/qlik/sse/basicExample/PluginServer.java

@@ -0,0 +1,99 @@
+package com.qlik.sse.basicExample;


package names should be all lowercase (https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html)

qlikmats · 2017-10-11T08:49:59Z

examples/java/basic_example/src/main/java/com/qlik/sse/basicExample/JavaPlugin.java

@@ -0,0 +1,405 @@
+package com.qlik.sse.basicExample;


package names should be all lowercase (https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html)

qlikmats · 2017-10-11T08:50:19Z

...ples/java/basic_example/src/main/java/com/qlik/sse/basicExample/PluginServerInterceptor.java

@@ -0,0 +1,69 @@
+package com.qlik.sse.basicExample;


package names should be all lowercase (https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html)

qlikmats

Correct the package names when you get an opportunity.

Split the code so that it becomes easier to read and to make it easier to update it in the future. Update package names and edit files to follow Java standards.

qlikmats self-requested a review September 27, 2017 12:53

qlikmats reviewed Sep 27, 2017

View reviewed changes

qlikmats reviewed Sep 28, 2017

View reviewed changes

qlikmats reviewed Oct 2, 2017

View reviewed changes

kjartan-qlik suggested changes Oct 2, 2017

View reviewed changes

LinneaStovring-Nielsen and others added 6 commits October 5, 2017 12:50

Add first draft of unfinnished java plugin.

6b3a3a0

Add an sse java plugin to later be used as a basic example.

Add (unfinnished) evaluate script

659a123

Add evaluate script to the plugin so that it can be used as a basic example showing how to do script evaluation in a java plugin.

Add new functions and the pom.xml file

685a707

Add more functions that can be executed to make the example more useful. Also add the pom.xml file to show which dependencies are needed.

Add Cache/NoCache functions

c7d1230

Add functions for showing how the cache can be turned of to be consistent with other basic examples.

Add logging options and secure connection

29b1029

Add a properties file for the logging so that the plugin user can set how much logging will be done. Also add a secure conection so that a secure connection with Qlik Sense Server is possible.

Add more documentation and clean upp of code

11202e0

Add documentaion so that it becomes easier for others to create their own sse java plugin. Some cleanup to make the code easier to read.

tobiaslindulf reviewed Oct 5, 2017

View reviewed changes

kjartan-qlik approved these changes Oct 9, 2017

View reviewed changes

qlikmats reviewed Oct 11, 2017

View reviewed changes

qlikmats approved these changes Oct 11, 2017

View reviewed changes

Split the code into three separate files

85d203d

Split the code so that it becomes easier to read and to make it easier to update it in the future. Update package names and edit files to follow Java standards.

LinneaStovring-Nielsen merged commit ae64fde into qlik-oss:master Oct 18, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic example of a java sse plugin #13

Basic example of a java sse plugin #13

LinneaStovring-Nielsen commented Sep 27, 2017

qlikmats left a comment

qlikmats Sep 27, 2017

qlikmats Sep 27, 2017

qlikmats Sep 27, 2017

qlikmats Sep 27, 2017

qlikmats Sep 28, 2017

qlikmats Sep 28, 2017

qlikmats left a comment

qlikmats Sep 28, 2017

qlikmats Sep 28, 2017

qlikmats Sep 28, 2017

qlikmats Sep 28, 2017 •

edited

Loading

LinneaStovring-Nielsen commented Sep 29, 2017

qlikmats Oct 2, 2017

LinneaStovring-Nielsen Oct 2, 2017

qlikmats Oct 2, 2017

qlikmats Oct 2, 2017

qlikmats Oct 2, 2017

qlikmats Oct 2, 2017

qlikmats Oct 2, 2017

LinneaStovring-Nielsen Oct 2, 2017

qlikmats Oct 3, 2017

kjartan-qlik Oct 2, 2017

LinneaStovring-Nielsen Oct 2, 2017

kjartan-qlik Oct 2, 2017

kjartan-qlik Oct 2, 2017

kjartan-qlik Oct 2, 2017

tobiaslindulf left a comment

cjpersson commented Oct 8, 2017

LinneaStovring-Nielsen commented Oct 9, 2017

qlikmats Oct 11, 2017

qlikmats Oct 11, 2017

qlikmats Oct 11, 2017

qlikmats left a comment

Basic example of a java sse plugin #13

Basic example of a java sse plugin #13

Conversation

LinneaStovring-Nielsen commented Sep 27, 2017

Status

Information

To-do list

qlikmats 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

qlikmats 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

qlikmats Sep 28, 2017 • edited Loading

Choose a reason for hiding this comment

LinneaStovring-Nielsen commented Sep 29, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiaslindulf left a comment

Choose a reason for hiding this comment

cjpersson commented Oct 8, 2017

LinneaStovring-Nielsen commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qlikmats left a comment

Choose a reason for hiding this comment

qlikmats Sep 28, 2017 •

edited

Loading