Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactoring- Removed 3 design smells #856

Merged
merged 3 commits into from
Apr 2, 2024
Merged

Refactoring- Removed 3 design smells #856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24ff7c8
removed 3 design smells
Mar 30, 2024
65166fe
moved modelVersionTableToString to model rest service
Mar 31, 2024
9667c5d
fixed build errors after refactoring
Mar 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion imixs-workflow-core/src/main/java/org/imixs/workflow/BPMNRuleEngine.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this as we can have a valid implementation directly in the abstract WorkflowException class

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the method which was defined in interface modelManager and overriden in model service, now I have moved it to modelRestservice as per your suggestion, and it no longer a part of the modelManager hence not required to be overriden here.

Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ public void removeModel(String version) {
public Model getModelByWorkitem(ItemCollection workitem) throws ModelException {
return model;
}

}

}
10 changes: 10 additions & 0 deletions imixs-workflow-core/src/main/java/org/imixs/workflow/exceptions/PluginException.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets move this method into the abstract class WorkflowException

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the implementation of this method in both pluginException and validationException as the implementation is dependent on the params field of these classes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this now in the Issue #857 . This simplifies the code much more and we have the feature now for all Imixs Exception Types.
Thanks for you contribution!

Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,14 @@ protected void setErrorParameters(java.lang.Object[] aparams) {
this.params = aparams;
}

@Override
public String formatErrorMessageWithParameters(String message){
if (params != null && params.length > 0) {
for (int i = 0; i < params.length; i++) {
message = message.replace("{" + i + "}", params[i].toString());
}
}

return message;
}
}
3 changes: 3 additions & 0 deletions imixs-workflow-core/src/main/java/org/imixs/workflow/exceptions/WorkflowException.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just put the implementation here

Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,7 @@ public void setErrorCode(String errorCode) {
this.errorCode = errorCode;
}

public String formatErrorMessageWithParameters(String message){
return message;
}
}
9 changes: 3 additions & 6 deletions imixs-workflow-engine/src/main/java/org/imixs/workflow/engine/AsyncEventScheduler.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
import jakarta.ejb.TimerService;
import java.util.logging.Level;

import static org.imixs.workflow.engine.AsyncEventSchedulerConfig.*;

/**
* The AsyncEventScheduler starts a scheduler service to process async events in
* an asynchronous way by calling the AsyncEventService.
Expand All @@ -71,12 +73,7 @@
@Singleton
public class AsyncEventScheduler {

public static final String ASYNCEVENT_PROCESSOR_ENABLED = "asyncevent.processor.enabled";
public static final String ASYNCEVENT_PROCESSOR_INTERVAL = "asyncevent.processor.interval";
public static final String ASYNCEVENT_PROCESSOR_INITIALDELAY = "asyncevent.processor.initialdelay";
public static final String ASYNCEVENT_PROCESSOR_DEADLOCK = "asyncevent.processor.deadlock";

public static final String EVENTLOG_TOPIC_ASYNC_EVENT = "async.event";

// enabled
@Inject
Expand All @@ -95,7 +92,7 @@ public class AsyncEventScheduler {

// deadlock timeout interval in ms
@Inject
@ConfigProperty(name = AsyncEventScheduler.ASYNCEVENT_PROCESSOR_DEADLOCK, defaultValue = "60000")
@ConfigProperty(name = ASYNCEVENT_PROCESSOR_DEADLOCK, defaultValue = "60000")
long deadLockInterval;

private static final Logger logger = Logger.getLogger(AsyncEventScheduler.class.getName());
Expand Down
10 changes: 10 additions & 0 deletions imixs-workflow-engine/src/main/java/org/imixs/workflow/engine/AsyncEventSchedulerConfig.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.imixs.workflow.engine;

public class AsyncEventSchedulerConfig {
public static final String ASYNCEVENT_PROCESSOR_ENABLED = "asyncevent.processor.enabled";
public static final String ASYNCEVENT_PROCESSOR_INTERVAL = "asyncevent.processor.interval";
public static final String ASYNCEVENT_PROCESSOR_INITIALDELAY = "asyncevent.processor.initialdelay";
public static final String ASYNCEVENT_PROCESSOR_DEADLOCK = "asyncevent.processor.deadlock";

public static final String EVENTLOG_TOPIC_ASYNC_EVENT = "async.event";
}
9 changes: 6 additions & 3 deletions imixs-workflow-engine/src/main/java/org/imixs/workflow/engine/AsyncEventService.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@
import jakarta.ejb.TransactionAttributeType;
import jakarta.persistence.OptimisticLockException;

import static org.imixs.workflow.engine.AsyncEventSchedulerConfig.ASYNCEVENT_PROCESSOR_ENABLED;
import static org.imixs.workflow.engine.AsyncEventSchedulerConfig.EVENTLOG_TOPIC_ASYNC_EVENT;

/**
* The AsyncEventService can be used to process workflow events in an
* asynchronous batch process. The AsyncEventService lookup eventLog entries of
Expand Down Expand Up @@ -89,7 +92,7 @@ public class AsyncEventService {

// enabled
@Inject
@ConfigProperty(name = AsyncEventScheduler.ASYNCEVENT_PROCESSOR_ENABLED, defaultValue = "false")
@ConfigProperty(name = ASYNCEVENT_PROCESSOR_ENABLED, defaultValue = "false")
boolean enabled;

private static final Logger logger = Logger.getLogger(AsyncEventService.class.getName());
Expand Down Expand Up @@ -140,7 +143,7 @@ public void onProcess(@Observes ProcessingEvent processingEvent) throws ModelExc
asyncEventData.setItemValue("timeDuration", boundaryDuration);
asyncEventData.setItemValue(WorkflowKernel.TRANSACTIONID,
processingEvent.getDocument().getItemValueString(WorkflowKernel.TRANSACTIONID));
eventLogService.createEvent(AsyncEventScheduler.EVENTLOG_TOPIC_ASYNC_EVENT,
eventLogService.createEvent(EVENTLOG_TOPIC_ASYNC_EVENT,
processingEvent.getDocument().getUniqueID(), asyncEventData, cal);

}
Expand All @@ -166,7 +169,7 @@ public void processEventLog() {

// test for new event log entries by timeout...
List<EventLog> events = eventLogService.findEventsByTimeout(100,
AsyncEventScheduler.EVENTLOG_TOPIC_ASYNC_EVENT);
EVENTLOG_TOPIC_ASYNC_EVENT);

if (debug) {
logger.log(Level.FINEST, "......found {0} eventLog entries", events.size());
Expand Down
8 changes: 2 additions & 6 deletions imixs-workflow-engine/src/main/java/org/imixs/workflow/engine/ModelService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,8 @@

import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.text.SimpleDateFormat;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down
18 changes: 1 addition & 17 deletions imixs-workflow-faces/src/main/java/org/imixs/workflow/faces/util/ErrorHandler.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Original file line number Diff line number Diff line change
Expand Up @@ -155,23 +155,7 @@ public static void addErrorMessage(WorkflowException pe) {
}

// parse message for params
if (pe instanceof PluginException) {
PluginException p = (PluginException) pe;
if (p.getErrorParameters() != null && p.getErrorParameters().length > 0) {
for (int i = 0; i < p.getErrorParameters().length; i++) {
message = message.replace("{" + i + "}", p.getErrorParameters()[i].toString());
}
}
} else {
if (pe instanceof ValidationException) {
ValidationException p = (ValidationException) pe;
if (p.getErrorParameters() != null && p.getErrorParameters().length > 0) {
for (int i = 0; i < p.getErrorParameters().length; i++) {
message = message.replace("{" + i + "}", p.getErrorParameters()[i].toString());
}
}
}
}
pe.formatErrorMessageWithParameters(message);
FacesContext.getCurrentInstance().addMessage(null,
new FacesMessage(FacesMessage.SEVERITY_INFO, errorCode, message));

Expand Down
11 changes: 11 additions & 0 deletions imixs-workflow-faces/src/main/java/org/imixs/workflow/faces/util/ValidationException.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets move this method into the abstract class WorkflowException

Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,15 @@ protected void setErrorParameters(Object[] aparams) {
this.params = aparams;
}

@Override
public String formatErrorMessageWithParameters(String message){
if (params != null && params.length > 0) {
for (int i = 0; i < params.length; i++) {
message = message.replace("{" + i + "}", params[i].toString());
}
}

return message;
}

}
95 changes: 55 additions & 40 deletions imixs-workflow-jax-rs/src/main/java/org/imixs/workflow/jaxrs/ModelRestService.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your attention, but lets have the method modelVersionTableToString still in the ModelRestService as this is a very specific part of the REST API

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the function to modelRestService.

Original file line number Diff line number Diff line change
Expand Up @@ -146,51 +146,13 @@ private void printVersionTable(OutputStream out) {
try {
StringBuffer buffer = new StringBuffer();

List<String> modelVersionList = modelService.getVersions();

// compute rootContext:
String rootContext = servletRequest.getContextPath() + servletRequest.getServletPath();

buffer.append("<table>");
buffer.append("<tr><th>Version</th><th>Uploaded</th><th>Workflow Groups</th></tr>");
for (String modelVersion : modelVersionList) {

Model model = modelService.getModel(modelVersion);
ItemCollection modelEntity = modelService.loadModelEntity(modelVersion);

// now check groups...
List<String> groupList = model.getGroups();

buffer.append("<tr>");

if (modelEntity != null) {

buffer.append("<td><a href=\"" + rootContext + "/model/" + modelVersion + "/bpmn\">" + modelVersion
+ "</a></td>");

// print upload date...
if (modelEntity != null) {
Date dat = modelEntity.getItemValueDate("$Modified");
SimpleDateFormat formater = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
buffer.append("<td>" + formater.format(dat) + "</td>");
}
} else {
buffer.append("<td>" + modelVersion + "</td>");
buffer.append("<td> - </td>");
}

// Groups
buffer.append("<td>");
for (String group : groupList) {
// build a link for each group to get the Tasks

buffer.append("<a href=\"" + rootContext + "/model/" + modelVersion + "/groups/" + group + "\">"
+ group + "</a></br>");
}
buffer.append("</td>");
buffer.append("</tr>");

}
// append current model version table as a html string
buffer.append(modelVersionTableToString(rootContext));

buffer.append("</table>");
out.write(buffer.toString().getBytes());
Expand Down Expand Up @@ -495,4 +457,57 @@ public void postModel(XMLDataCollection ecol) {
putModel(ecol);
}

/**
* Returns the current model information in html format
* @param rootContext
* @return model version table as a html string
* @throws ModelException
*/
private String modelVersionTableToString(String rootContext) throws ModelException{
List<String> modelVersionList = modelService.getVersions();
StringBuffer buffer = new StringBuffer();

for (String modelVersion : modelVersionList) {

appendTagsToBuffer(modelVersion, rootContext, buffer);
}
return buffer.toString();
}

private void appendTagsToBuffer(String modelVersion, String rootContext, StringBuffer buffer) throws ModelException{
Model model = modelService.getModel(modelVersion);
ItemCollection modelEntity = modelService.loadModelEntity(modelVersion);

// now check groups...
List<String> groupList = model.getGroups();

buffer.append("<tr>");

if (modelEntity != null) {

buffer.append("<td><a href=\"" + rootContext + "/model/" + modelVersion + "/bpmn\">" + modelVersion
+ "</a></td>");

// print upload date...
if (modelEntity != null) {
Date dat = modelEntity.getItemValueDate("$Modified");
SimpleDateFormat formater = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
buffer.append("<td>" + formater.format(dat) + "</td>");
}
} else {
buffer.append("<td>" + modelVersion + "</td>");
buffer.append("<td> - </td>");
}

// Groups
buffer.append("<td>");
for (String group : groupList) {
// build a link for each group to get the Tasks

buffer.append("<a href=\"" + rootContext + "/model/" + modelVersion + "/groups/" + group + "\">"
+ group + "</a></br>");
}
buffer.append("</td>");
buffer.append("</tr>");
}
}
Loading