[CONSULT-1524] changed execution from member to method parameter by eugenweissbart · Pull Request #2 · hydra-billing/activiti-ext

eugenweissbart · 2018-05-14T13:00:26Z

No description provided.

… rewrite

sugarfree1 · 2018-05-14T14:17:12Z

build.xml

  </path>
  <taskdef name="groovyc" classname="org.codehaus.groovy.ant.Groovyc" classpathref="groovy.classpath" />
+-->
+  <property name="groovy.home" value="/usr/local/Cellar/groovy" />


Really? Cellar/groovy?

This is where it's located on my mac. Will revert in next commit.

sugarfree1 · 2018-05-14T14:23:34Z

src/bss/eventListeners/abstract_listener.groovy

    false
  }
-}
+}


Trailing new line

sugarfree1 · 2018-05-14T14:24:19Z

src/bss/eventListeners/event_logging.groovy

    switch (event) {
      case ActivitiActivityEventImpl:
-        detailedLog = getActivityLog()
+        detailedLog = "${event.getActivityId()} (${event.getActivityType()} - ${event.getActivityName()})"


You could still use that functions, but pass event as a param.

I don't find them much necessary as they just return a single string

They are pretty complicated. We use this functions to simplify code.

sugarfree1 · 2018-05-14T14:25:03Z

src/bss/executionListeners/abstract_listener.groovy

-  
-  def protected execute() {
+
+  def protected execute(execution) {


Don't you need to describe type of execution like:
DelegateExecution execution?

Good point!

sugarfree1 · 2018-05-14T14:25:20Z

src/bss/executionListeners/abstract_listener.groovy

+
+    logger
+  }
+}


Trailing new line

sugarfree1 · 2018-05-14T14:26:33Z

src/bss/http.groovy

  def HTTPRestProcessor(parameters) {
-    this.execution = parameters.execution
    this.httpClient = new RESTClient(parameters.baseUrl)
+    this.httpClient.ignoreSSLIssues()


Easy man, easy. What happened here?

Oops, this was for testing purposes (self-signed SSL cert error) and didnt go away, will fix

sugarfree1 · 2018-05-14T14:27:52Z

src/bss/http.groovy

+      logger = execution.getVariable("logger")
+    }
+
+    logger


This method could be shortened to something like:

def protected getLogger(DelegateExecution execution) { if (execution) { execution.getVariable("logger") } }

Will this actually return the logger?

sugarfree1 · 2018-05-14T14:28:17Z

src/bss/http.groovy

+
  def public sendRequest(params, String method) {
+    def execution = params.execution
+    params.remove('execution')


groovy's HTTPBuilder has this strange check that caused the code to fail if execution stays in params map.

sugarfree1 · 2018-05-14T14:29:04Z

src/homs/eventListeners/auto_save_order_data.groovy


 import org.activiti.latera.bss.eventListeners.AbstractListener
 import org.activiti.latera.bss.http.HTTPRestProcessor
+import org.activiti.engine.delegate.event.*


I think you use only ActivitiEvent from this package.

Good point!

sugarfree1 · 2018-05-14T14:30:13Z

src/homs/eventListeners/auto_save_order_data.groovy

+      def orderData = getOrderData(execution)
+      def orderDataBuffer = execution.getVariable('homsOrdDataBuffer')
+
+      if (orderData != orderDataBuffer) {


You don't need variable orderDataBuffer. Just compare directly to result of execution.getVariable.

Won't agree here because the variables are changed already, here we have a few options

use homsOrdDataBuffer

get variables from HOMS before comparsion (not good as will add one HTTP GET request each AutoSaveOrderData run (basically each ACTIVITI_COMPLETED event)

push variables seamlessly each ACTIVITI_COMPLETED (even worse)

Discussed, will replace orderDataBuffer with execution.getVariable('homsOrdDataBuffer')

* Changed groovy path in build.xml * Added trailing newlines where missing * Fixed execute method declaration in src/bss/executionListeners/abstract_listener.groovy * Fixed alignment in src/bss/executionListeners/init_logging.groovy * Removed unnecessary ignoreSSLIssues call in src/bss/http.groovy * Shortened getLogger declaration in src/bss/http.groovy * Fixed imports for src/homs/eventListeners/auto_save_order_data.groovy * Removed unnecessary orderDataBuffer variable in src/homs/eventListeners/auto_save_order_data.groovy

sugarfree1 · 2018-05-15T09:52:52Z

build.xml

  <property name="dst.build" location="${dst}/build" />
  <property name="dst.lib" location="${dst}/lib" />

+<!--


Just remove this code if you don't need it.

sugarfree1 · 2018-05-15T09:53:48Z

src/bss/eventListeners/abstract_listener.groovy

-    execute()
+    def execution = getExecution(event)
+
+    execute(execution, event)


Remove def execution and change this line to:

execute(getExecution(event), event)

sugarfree1 · 2018-05-15T09:55:23Z

src/bss/eventListeners/event_logging.groovy

    switch (event) {
      case ActivitiActivityEventImpl:
-        detailedLog = getActivityLog()
+        detailedLog = "${event.getActivityId()} (${event.getActivityType()} - ${event.getActivityName()})"


They are pretty complicated. We use this functions to simplify code.

sugarfree1 · 2018-05-15T09:57:07Z

src/bss/executionListeners/abstract_listener.groovy

  }
+
+  def protected getLogger(DelegateExecution execution) {
+    def logger = null


def protected getLogger(DelegateExecution execution) { if (execution) { execution.getVariable("logger") } }

I'm pretty sure it works the same.

sugarfree1 · 2018-05-15T09:58:33Z

src/homs/executionListeners/finish_order.groovy

-    def homsUser = getParameterValue('homsUser', execution)
-    def homsPassword = getParameterValue('homsPassword', execution)
+  def execute(execution) {
+      def homsUrl = execution.getVariable('homsUrl')


* Removed commented options in build.xml * Removed unneeded execution variable definition in src/bss/eventListeners/abstract_listener.groovy * Re-implemented detailed log functions in src/bss/eventListeners/event_logging.groovy * Shortened getLogger for src/bss/executionListeners/abstract_listener.groovy too * Fixed alignment in src/homs/executionListeners/finish_order.groovy

[CONSULT-1524] changed execution from member to method parameter; log…

1fa23d0

… rewrite

sugarfree1 suggested changes May 14, 2018

View reviewed changes

sugarfree1 suggested changes May 15, 2018

View reviewed changes

sugarfree1 approved these changes May 15, 2018

View reviewed changes

Fixed critical bug preventing listeners to execute

7808e6e

sugarfree1 approved these changes May 24, 2018

View reviewed changes

                   false
                 }
-              }
+              }

                
                    No newline at end of file

Conversation

eugenweissbart commented May 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sugarfree1 May 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sugarfree1 May 14, 2018 •

edited

Loading