process-analytics / bpmn-layout-generators

Tools for generating missing BPMNDiagram elements in BPMN files
Apache License 2.0
37 stars 6 forks source link

[R package] Review package installation instruction #91

Closed tbouffard closed 3 years ago

tbouffard commented 3 years ago

package installation: the usage of a gh token is probably not needed

The 0.1.4 package documentation explains that a github auth token must be pass to install the package. I don't understand why we need the token as all resources used by the package are public. If this is required, please document why.

package installation failure

devtools::install_github("process-analytics/bpmn-layout-generators", ref="bpmnLayoutGeneratoR-0.1.4", subdir="R/bpmnLayoutGeneratoR")

The package requires JDK to be installed on the machine. Otherwise the rjava package installation failed

Example on Ubuntu 20.04

checking Java support in R... present:
interpreter : '/usr/lib/jvm/default-java/bin/java'
archiver    : '/usr/lib/jvm/default-java/bin/jar'
compiler    : '/usr/lib/jvm/default-java/bin/javac'
header prep.: ''
cpp flags   : '-I/usr/lib/jvm/default-java/include -I/usr/lib/jvm/default-java/include/linux'
java libs   : '-L/usr/lib/jvm/default-java/lib/server -ljvm'
checking whether Java run-time works... ./configure: line 3765: /usr/lib/jvm/default-java/bin/java: No such file or directory
no
configure: error: Java interpreter '/usr/lib/jvm/default-java/bin/java' does not work
ERROR: configuration failed for package ‘rJava’

rJava installation: http://rforge.net/rJava/ Java installation

Once JDK is installed, don't forget to run R CMD javareconf. See https://cran.r-project.org/doc/manuals/R-admin.html#Java-support

Usage example in the readme --> NullPointerException

Note: confirmed on Ubuntu 20.04 by @tbouffard and on Windows 10 Pro by @oanesini

With package 0.1.4, R 3.6.3, AdoptOpenJDK 11.0.11

    Error in .jcall("RJavaTools", "Ljava/lang/Object;", "invokeMethod", cl,  :
      java.lang.NullPointerException

Same issue when running

as we are using the Java API and not the CLI, we should add a try catch bock when calling the 'layout generation from csv' method and at least print the stack trace

http://rforge.net/doc/packages/rJava/Exceptions.html

 tryCatch( diagramAsString <- bpmnLayoutJava$generateLayoutFromCSV(flow_node_as_csv, sequence_flow_as_csv, type), Exception = function(e){
     e$printStackTrace() 
 } )

When using the stack trace, we got

java.lang.NullPointerException
    at io.process.analytics.tools.bpmn.generator.input.CSVtoBPMN.assignIncomingAndOutgoingReferences(CSVtoBPMN.java:113)
    at io.process.analytics.tools.bpmn.generator.input.CSVtoBPMN.readFromCSV(CSVtoBPMN.java:51)
    at io.process.analytics.tools.bpmn.generator.BPMNLayoutGenerator.generateLayoutFromCSV(BPMNLayoutGenerator.java:48)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at RJavaTools.invokeMethod(RJavaTools.java:386)

CSVtoBPMN.readFromCSV received string parameters as

nodes

"","id","node","type"
"1",1,"task1","task"
"2",2,"task2","task"

edges --> the issue is here, from and to id must be number, not string, because in nodes, they are defined as number. Both must match. We need to add robustness to the Java implementation, when the referenced id in from/to doesn't exist in nodes. See #95

"","id","from_id","to_id"
"1","1","1","2"