DROOLS-4249: Implement client-side marshalling #1306

yesamer · 2020-02-10T16:46:35Z

@jomarko @danielezonca
This PR introduces the client-side marshalling for SCESIM Kogito project (part of 3879 ticket)

https://issues.redhat.com/browse/DROOLS-4249

...scenario-simulation-editor-kogito-marshaller/src/main/resources/META-INF/ErraiApp.properties

...drools-wb-scenario-simulation-editor-kogito-marshaller/src/main/resources/META-INF/beans.xml

.../main/resources/org/drools/workbench/scenariosimulation/kogito/marshaller/js/SCESIMMainJs.js

jomarko

Is it OK that in target we have mix of classes and java files?
java

target/classes/org/drools/workbench/scenariosimulation/kogito/marshaller/jre/javax/xml/

class

target/classes/org/drools/workbench/scenariosimulation/kogito/marshaller/js/

jomarko · 2020-02-12T12:04:19Z

...wb-scenario-simulation-editor/drools-wb-scenario-simulation-editor-kogito-marshaller/pom.xml

+  <properties>
+    <jsinterop-annotations.version>1.0.2</jsinterop-annotations.version>
+    <jsinterop-base-version>1.0.0-beta-1</jsinterop-base-version>
+    <jsonix-scripts.version>3.0.0</jsonix-scripts.version>
+    <js.destination>${project.basedir}/src/main/resources/org/drools/workbench/scenariosimulation/kogito/marshaller/js</js.destination>
+    <gwt-jsonix-schema-compiler.version>1.1.0</gwt-jsonix-schema-compiler.version>
+    <gwt-interop-utils.version>0.3.0</gwt-interop-utils.version>
+    <maven-jaxb2-plugin.version>0.14.0</maven-jaxb2-plugin.version>
+  </properties>


@manstis @yesamer is it expected that the list of dependencies differ slightly from DMN? I do not mean versions.

https://github.com/kiegroup/kie-wb-common/blob/master/kie-wb-common-dmn/kie-wb-common-dmn-webapp-kogito-marshaller/pom.xml

@jomarko @manstis This is a good point, thank you to raise it. I'll analyze the differences. At first glance, a change in DMN side is required for gwt-jsonix-schema-compiler, in my understanding 1.1.0 is the last version generated by @gcardosi

Ok, I reported https://issues.redhat.com/browse/DROOLS-5057 for DMN

@jomarko

<plugin> <groupId>org.jvnet.jaxb2_commons</groupId> <artifactId>jaxb2-basics</artifactId> </plugin> <plugin> <groupId>org.jvnet.jaxb2_commons</groupId> <artifactId>jaxb2-namespace-prefix</artifactId> </plugin>

It seems that jaxb2-namespace-prefix version.jaxb2-basics are sub plugin used in maven-jaxb2-plugin together with gwt-jsonix-schema-compiler. IDK why these 2 plugins are required in DMN and NOT in SCESIM. @manstis do you have any idea?

IDK if they're required..

It could be they're dead baggage that can be tidied up (when @jomarko looks at the JIRA he reported).

@manstis Thank you! @jomarko probably another "cleanup" change required in DMN is the one spotted by @danielezonca: remove Errai.properties and beans.xml files which should be useless in this module.

jomarko · 2020-02-12T12:27:36Z

...org/drools/workbench/scenariosimulation/kogito/marshaller/jre/javax/xml/namespace/QName.java

+
+    @Override
+    public String toString() {
+        if (getNamespaceURI().equals(XMLConstants.NULL_NS_URI)) {


Could we please use Objects.equals also here?

@jomarko I don't see problems do to that, done!

jomarko · 2020-02-12T12:57:25Z

I am also wondering if we should test at least somehow this PR #1309

yesamer · 2020-02-12T13:56:23Z

Is it OK that in target we have mix of classes and java files?
java

target/classes/org/drools/workbench/scenariosimulation/kogito/marshaller/jre/javax/xml/

class

target/classes/org/drools/workbench/scenariosimulation/kogito/marshaller/js/

Considering these files are in resources folder and according to package-info of jre package:

/**
 * Container for GWT "super source" files where GWT does not provide an emulation of JRE classes.
 * See http://www.gwtproject.org/doc/latest/DevGuideOrganizingProjects.html
 */

It seems correct to me. I can dig more on it, anyway.

yesamer · 2020-02-12T14:48:31Z

I am also wondering if we should test at least somehow this PR #1309

@jomarko @manstis
About Unit Tests: I agree with you we need to add it. BUT, I would like to use the same approach used in DMN Editor. In my understating, kie-wb-common-dmn-webapp-kogito-marshaller doesn't contains ANY test. All unit tests related to that module are in kie-wb-common-dmn-webapp-kogito-marshaller-tests. Then, my idea is to follow the same approach on SCESIM, and to manage it in a separate ticket after I'll completed ticket 3879. WDYT?

manstis · 2020-02-13T09:28:47Z

@yesamer After extensive investigation writing tests for the marshaller in kie-wb-common-dmn-webapp-kogito-marshaller-tests was proving impossible (long story short: we need to run the real JavaScript which means we needed to use GWTTestCase however this uses a library that emulates a real browser and some of the implementation of browser objects returned different results to using a real browser.

Therefore the decision was made to extend @jomarko Selenium tests to also handle marshalling tests. The result is this PR...

yesamer · 2020-02-13T10:04:56Z

@yesamer After extensive investigation writing tests for the marshaller in kie-wb-common-dmn-webapp-kogito-marshaller-tests was proving impossible (long story short: we need to run the real JavaScript which means we needed to use GWTTestCase however this uses a library that emulates a real browser and some of the implementation of browser objects returned different results to using a real browser.

Therefore the decision was made to extend @jomarko Selenium tests to also handle marshalling tests. The result is this PR...

@manstis Yes, I remember part of the story and your mails about that topic. I didn't know kie-wb-common-dmn-webapp-kogito-marshaller-tests was planned to be totally removed. I expected that some kind of tests were possible, and in particular tests that checks JSInterops model classes are correctly generated (eg. #1309 wrote by @jomarko, which checks the number of generated model file is the one expected )
Considering this, I think the kind of test proposed by @jomarko are reasonable and are the only way to "unit test" the module.

manstis · 2020-02-13T10:11:36Z

@yesamer I could have the same type of test as @jomarko has written but note it is in drools-wb-scenario-simulation-editor-kogito-marshaller (and not a -kogito-marshaller-tests module!)

yesamer · 2020-02-13T10:19:39Z

@yesamer I could have the same type of test as @jomarko has written but note it is in drools-wb-scenario-simulation-editor-kogito-marshaller (and not a -kogito-marshaller-tests module!)

@manstis
Perfect, better use a common strategy. We'll do the same. Thanks!

yesamer · 2020-02-13T16:27:50Z

jenkins retest this please

jomarko · 2020-02-17T11:11:01Z

...-scenario-simulation-editor/drools-wb-scenario-simulation-editor-kogito-marshaller/README.md

+* **Callbacks**: Callbacks to be called during a marshall/unmarshall call. automatically generated by 
+_maven-jaxb2-plugin_ plugin in _js/callbacks_ package
+* **SCESIMMainJS**: JSInterop adapter to use for marshalling/unmarshalling, which refer to _SCESIMMainJS.js_ 
+its javascript representation. Thus u


@jomarko Completed.

jomarko · 2020-02-17T11:12:24Z

...wb-scenario-simulation-editor/drools-wb-scenario-simulation-editor-kogito-marshaller/pom.xml

+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <configuration>
+          <skip>true</skip>


shouldn't be disabled just for special folders?

@jomarko good point, done!

jomarko · 2020-02-17T11:14:34Z

.../main/resources/org/drools/workbench/scenariosimulation/kogito/marshaller/js/SCESIMMainJs.js

+/**
+ * @type {{marshall: MainJs.marshall, unmarshall: MainJs.unmarshall}}
+ */
+SCESIMMainJs = {


isn't this automatically generated? do we need to track it?

@jomarko That class is not automatically generated

sonarcloud · 2020-02-17T15:51:12Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
0.0% Duplication

yesamer · 2020-02-17T16:36:02Z

@danielezonca can you please merge it in master? Thanks

yesamer added 7 commits February 10, 2020 14:11

DROOLS-4249: Introducing kogito-marshaller module

9198e24

DROOLS-4249: Introducing kogito-marshaller module

fdf5512

DROOLS-4249: Introducing kogito-marshaller module

02d3980

DROOLS-4249: Introducing kogito-marshaller module

b9e6f43

DROOLS-4249: Introducing kogito-marshaller module

cc8de57

Merge remote-tracking branch 'origin/master' into DROOLS-4249

b0928a6

DROOLS-4249: Introducing kogito-marshaller module

49dd842

danielezonca reviewed Feb 11, 2020

View reviewed changes

DROOLS-4249: Required changes after Code Review

c8943b1

yesamer marked this pull request as ready for review February 11, 2020 13:03

yesamer requested a review from jomarko February 11, 2020 13:06

yesamer mentioned this pull request Feb 11, 2020

DROOLS-5037: Introducing client-kogito module for SCESIM #1307

Merged

DROOLS-3879: Updated project version

b08fad3

jomarko reviewed Feb 12, 2020

View reviewed changes

jomarko mentioned this pull request Feb 12, 2020

DROOLS-4249: Test classes generation was done #1309

Merged

DROOLS-4249: Required changes after Code Review

86bfbb2

DROOLS-4249: Required changes after Code Review

42a68df

danielezonca approved these changes Feb 13, 2020

View reviewed changes

yesamer requested a review from jomarko February 13, 2020 11:13

yesamer added 2 commits February 14, 2020 18:52

DROOLS-4249: Minor changes

903b8b2

Merge remote-tracking branch 'origin/master' into DROOLS-4249

8ec1e84

jomarko reviewed Feb 17, 2020

View reviewed changes

DROOLS-4249: Required changes after Code Review

4fdc2e0

yesamer requested a review from jomarko February 17, 2020 13:43

yesamer added 2 commits February 17, 2020 15:39

DROOLS-4249: Required changes after Code Review

cad4a3d

DROOLS-4249: Required changes after Code Review

378f83f

jomarko approved these changes Feb 17, 2020

View reviewed changes

DROOLS-4249: Minor changes

49f5b4f

danielezonca merged commit aace9e4 into kiegroup:master Feb 18, 2020

yesamer deleted the DROOLS-4249 branch February 18, 2020 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DROOLS-4249: Implement client-side marshalling #1306

DROOLS-4249: Implement client-side marshalling #1306

yesamer commented Feb 10, 2020 •

edited

jomarko left a comment

jomarko Feb 12, 2020

yesamer Feb 12, 2020 •

edited

jomarko Feb 12, 2020

yesamer Feb 12, 2020 •

edited

manstis Feb 12, 2020

yesamer Feb 12, 2020

jomarko Feb 12, 2020

yesamer Feb 12, 2020

jomarko commented Feb 12, 2020

yesamer commented Feb 12, 2020

yesamer commented Feb 12, 2020 •

edited

manstis commented Feb 13, 2020

yesamer commented Feb 13, 2020

manstis commented Feb 13, 2020

yesamer commented Feb 13, 2020 •

edited

yesamer commented Feb 13, 2020

jomarko Feb 17, 2020

yesamer Feb 17, 2020

jomarko Feb 17, 2020

yesamer Feb 17, 2020

jomarko Feb 17, 2020

yesamer Feb 17, 2020

sonarcloud bot commented Feb 17, 2020

yesamer commented Feb 17, 2020

DROOLS-4249: Implement client-side marshalling #1306

DROOLS-4249: Implement client-side marshalling #1306

Conversation

yesamer commented Feb 10, 2020 • edited

jomarko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yesamer Feb 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yesamer Feb 12, 2020 • edited

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

jomarko commented Feb 12, 2020

yesamer commented Feb 12, 2020

yesamer commented Feb 12, 2020 • edited

manstis commented Feb 13, 2020

yesamer commented Feb 13, 2020

manstis commented Feb 13, 2020

yesamer commented Feb 13, 2020 • edited

yesamer commented Feb 13, 2020

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

sonarcloud bot commented Feb 17, 2020

yesamer commented Feb 17, 2020

yesamer commented Feb 10, 2020 •

edited

yesamer Feb 12, 2020 •

edited

yesamer Feb 12, 2020 •

edited

yesamer commented Feb 12, 2020 •

edited

yesamer commented Feb 13, 2020 •

edited