From eb8f48465ba69f25bd3b50dac76407e75e0d9ea3 Mon Sep 17 00:00:00 2001 From: Maciej Swiderski Date: Fri, 14 Jul 2017 13:03:00 +0200 Subject: [PATCH 1/2] JBPM-6182 - Add validation for Work Item handlers declarations in Deployment Descriptor Editor --- .../jbpm-wb-integration-backend/pom.xml | 5 + .../server/dd/DDEditorServiceImpl.java | 79 +++++++++++- .../server/dd/DDEditorServiceTest.java | 120 ++++++++++++++++++ 3 files changed, 202 insertions(+), 2 deletions(-) diff --git a/jbpm-wb-integration/jbpm-wb-integration-backend/pom.xml b/jbpm-wb-integration/jbpm-wb-integration-backend/pom.xml index 3b85f54334..cf9ab78879 100644 --- a/jbpm-wb-integration/jbpm-wb-integration-backend/pom.xml +++ b/jbpm-wb-integration/jbpm-wb-integration-backend/pom.xml @@ -200,6 +200,11 @@ kie-wb-common-library-api + + org.mvel + mvel2 + + org.kie diff --git a/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java b/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java index 92a90931b5..d1c4148336 100644 --- a/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java +++ b/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java @@ -22,6 +22,7 @@ import javax.enterprise.context.ApplicationScoped; import javax.inject.Inject; import javax.inject.Named; +import javax.lang.model.SourceVersion; import org.guvnor.common.services.backend.exceptions.ExceptionUtilities; import org.guvnor.common.services.backend.util.CommentedOptionFactory; @@ -41,6 +42,9 @@ import org.kie.internal.runtime.conf.PersistenceMode; import org.kie.internal.runtime.conf.RuntimeStrategy; import org.kie.workbench.common.services.backend.service.KieService; +import org.mvel2.CompileException; +import org.mvel2.MVEL; +import org.mvel2.ParserContext; import org.uberfire.backend.server.util.Paths; import org.uberfire.backend.vfs.Path; import org.uberfire.io.IOService; @@ -131,8 +135,20 @@ public List validate(Path path, DeploymentDescriptorModel content) { final List validationMessages = new ArrayList(); try { - unmarshal(path, - content).toXml(); + DeploymentDescriptor dd = unmarshal(path, content); + + // validate the content of the descriptor + + validationMessages.addAll(validateObjectModels(path, dd.getConfiguration())); + validationMessages.addAll(validateObjectModels(path, dd.getEnvironmentEntries())); + validationMessages.addAll(validateObjectModels(path, dd.getEventListeners())); + validationMessages.addAll(validateObjectModels(path, dd.getGlobals())); + validationMessages.addAll(validateObjectModels(path, dd.getMarshallingStrategies())); + validationMessages.addAll(validateObjectModels(path, dd.getTaskEventListeners())); + validationMessages.addAll(validateObjectModels(path, dd.getWorkItemHandlers())); + + // validate its structure + dd.toXml(); } catch (Exception e) { final ValidationMessage msg = new ValidationMessage(); msg.setPath(path); @@ -157,6 +173,65 @@ public String toSource(Path path, // helper methods + protected List validateObjectModels(Path path, List objectModels) { + + final List validationMessages = new ArrayList(); + + objectModels.forEach(model -> { + + String identifier = model.getIdentifier(); + + if (identifier == null || identifier.isEmpty()) { + validationMessages.add(newMessage(path, "Identifier cannot be empty for " + model.getIdentifier(), Level.ERROR)); + } + + String resolver = model.getResolver(); + if (resolver == null) { + validationMessages.add(newMessage(path, "No resolver selected for " + model.getIdentifier(), Level.ERROR)); + } + else if (resolver.equalsIgnoreCase(ItemObjectModel.MVEL_RESOLVER)) { + try { + ParserContext parserContext = new ParserContext(); + parserContext.setStrictTypeEnforcement( true ); + parserContext.setStrongTyping( true ); + MVEL.compileExpression(identifier, parserContext); + } catch (CompileException e) { + StringBuilder text = new StringBuilder(); + text.append("Could not compile mvel expression '" + model.getIdentifier() +"'.") + .append(" this can be due to invalid syntax of missing classes") + .append("-") + .append(e.getMessage()); + validationMessages.add(newMessage(path, text.toString(), Level.WARNING)); + } + } else if (resolver.equalsIgnoreCase(ItemObjectModel.REFLECTION_RESOLVER)) { + if (!SourceVersion.isName(identifier)) { + validationMessages.add(newMessage(path, "Identifier is not valid Java class which is required by reflection resolver " + model.getIdentifier(), Level.ERROR)); + } + } else { + validationMessages.add(newMessage(path, "Not valid resolver selected for " + model.getIdentifier(), Level.ERROR)); + } + + + if (model instanceof NamedObjectModel) { + String name = ((NamedObjectModel) model).getName(); + if (name == null || name.isEmpty()) { + validationMessages.add(newMessage(path, "Name cannot be empty for " + model.getIdentifier(), Level.ERROR)); + } + } + }); + + return validationMessages; + } + + protected ValidationMessage newMessage(Path path, String text, Level level) { + final ValidationMessage msg = new ValidationMessage(); + msg.setPath(path); + msg.setLevel(level); + msg.setText(text); + + return msg; + } + protected DeploymentDescriptorModel marshal(DeploymentDescriptor originDD) { DeploymentDescriptorModel ddModel = new DeploymentDescriptorModel(); ddModel.setPersistenceUnitName(originDD.getPersistenceUnit()); diff --git a/jbpm-wb-integration/jbpm-wb-integration-backend/src/test/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceTest.java b/jbpm-wb-integration/jbpm-wb-integration-backend/src/test/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceTest.java index ad97be8884..d90619d17d 100644 --- a/jbpm-wb-integration/jbpm-wb-integration-backend/src/test/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceTest.java +++ b/jbpm-wb-integration/jbpm-wb-integration-backend/src/test/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceTest.java @@ -19,10 +19,14 @@ import java.lang.reflect.Field; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Random; +import org.guvnor.common.services.shared.message.Level; +import org.guvnor.common.services.shared.validation.model.ValidationMessage; import org.jbpm.workbench.wi.dd.model.DeploymentDescriptorModel; +import org.jbpm.workbench.wi.dd.model.ItemObjectModel; import org.jgroups.util.UUID; import org.junit.Test; import org.kie.internal.runtime.conf.AuditMode; @@ -33,6 +37,8 @@ import org.kie.internal.runtime.conf.RuntimeStrategy; import org.kie.test.util.compare.ComparePair; +import static org.junit.Assert.*; + public class DDEditorServiceTest extends DDEditorServiceImpl { private static Random random = new Random(); @@ -136,4 +142,118 @@ public void marshalUnmarshalTest() throws Exception { "DeploymentDescriptorModel.overview") .compare(); } + + @Test + public void testValidateEmptyResolver() { + List models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel(null, + "item-name", + "handler-classname")}); + + List validationMessages = validateObjectModels(null, models); + assertEquals(1, validationMessages.size()); + + ValidationMessage error = validationMessages.get(0); + assertEquals(Level.ERROR, error.getLevel()); + assertTrue(error.getText().startsWith("No resolver selected")); + + models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel("", + "item-name", + "handler-classname")}); + + validationMessages = validateObjectModels(null, models); + assertEquals(1, validationMessages.size()); + + error = validationMessages.get(0); + assertEquals(Level.ERROR, error.getLevel()); + assertTrue(error.getText().startsWith("Not valid resolver selected")); + } + + @Test + public void testValidateReflectionResolver() { + List models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel(ItemObjectModel.REFLECTION_RESOLVER, + "item-name", + "java.lang.String")}); + + List validationMessages = validateObjectModels(null, models); + assertEquals(0, validationMessages.size()); + + } + + @Test + public void testValidateReflectionResolverInvalid() { + List models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel(ItemObjectModel.REFLECTION_RESOLVER, + "item-name", + "handler-classname")}); + + List validationMessages = validateObjectModels(null, models); + assertEquals(1, validationMessages.size()); + + ValidationMessage error = validationMessages.get(0); + assertEquals(Level.ERROR, error.getLevel()); + assertTrue(error.getText().startsWith("Identifier is not valid Java class which is required by reflection resolver")); + } + + @Test + public void testValidateReflectionResolverMissingIdentifier() { + List models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel(ItemObjectModel.REFLECTION_RESOLVER, + "item-name", + "")}); + + List validationMessages = validateObjectModels(null, models); + assertEquals(2, validationMessages.size()); + + ValidationMessage error = validationMessages.get(0); + assertEquals(Level.ERROR, error.getLevel()); + assertTrue(error.getText().startsWith("Identifier cannot be empty")); + error = validationMessages.get(1); + assertEquals(Level.ERROR, error.getLevel()); + assertTrue(error.getText().startsWith("Identifier is not valid Java class which is required by reflection resolver")); + } + + @Test + public void testValidateReflectionResolverNameEmpty() { + List models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel(ItemObjectModel.REFLECTION_RESOLVER, + "", + "java.lang.String")}); + + List validationMessages = validateObjectModels(null, models); + assertEquals(1, validationMessages.size()); + + ValidationMessage error = validationMessages.get(0); + assertEquals(Level.ERROR, error.getLevel()); + assertTrue(error.getText().startsWith("Name cannot be empty")); + } + + @Test + public void testValidateMvelResolver() { + List models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel(ItemObjectModel.MVEL_RESOLVER, + "item-name", + "new String()")}); + + List validationMessages = validateObjectModels(null, models); + assertEquals(0, validationMessages.size()); + + } + + @Test + public void testValidateMvelResolverInvalid() { + List models = Arrays.asList(new NamedObjectModel[]{ + new NamedObjectModel(ItemObjectModel.MVEL_RESOLVER, + "item-name", + "handler-classname")}); + + List validationMessages = validateObjectModels(null, models); + assertEquals(1, validationMessages.size()); + + ValidationMessage error = validationMessages.get(0); + assertEquals(Level.WARNING, error.getLevel()); + assertTrue(error.getText().startsWith("Could not compile mvel expression")); + } } From fbc30b8a400636621b7fd8796560dd31e201c60f Mon Sep 17 00:00:00 2001 From: Maciej Swiderski Date: Fri, 21 Jul 2017 09:36:47 +0200 Subject: [PATCH 2/2] JBPM-6215 - Improve deployment descriptor validation for Work Item handler's resolver type --- .../wi/dd/service/DDEditorService.java | 4 +++- .../server/dd/DDEditorServiceImpl.java | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/jbpm-wb-integration/jbpm-wb-integration-api/src/main/java/org/jbpm/workbench/wi/dd/service/DDEditorService.java b/jbpm-wb-integration/jbpm-wb-integration-api/src/main/java/org/jbpm/workbench/wi/dd/service/DDEditorService.java index dd740c31cf..1615eccda3 100644 --- a/jbpm-wb-integration/jbpm-wb-integration-api/src/main/java/org/jbpm/workbench/wi/dd/service/DDEditorService.java +++ b/jbpm-wb-integration/jbpm-wb-integration-api/src/main/java/org/jbpm/workbench/wi/dd/service/DDEditorService.java @@ -16,6 +16,7 @@ package org.jbpm.workbench.wi.dd.service; +import org.guvnor.common.services.project.builder.service.BuildValidationHelper; import org.guvnor.common.services.shared.file.SupportsUpdate; import org.guvnor.common.services.shared.validation.ValidationService; import org.jboss.errai.bus.server.annotations.Remote; @@ -25,7 +26,8 @@ import org.uberfire.ext.editor.commons.service.support.SupportsRead; @Remote -public interface DDEditorService extends ViewSourceService, +public interface DDEditorService extends BuildValidationHelper, + ViewSourceService, ValidationService, SupportsRead, SupportsUpdate { diff --git a/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java b/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java index d1c4148336..7f52f520c3 100644 --- a/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java +++ b/jbpm-wb-integration/jbpm-wb-integration-backend/src/main/java/org/jbpm/workbench/wi/backend/server/dd/DDEditorServiceImpl.java @@ -18,6 +18,7 @@ import java.io.InputStream; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import javax.enterprise.context.ApplicationScoped; import javax.inject.Inject; @@ -35,6 +36,7 @@ import org.jbpm.workbench.wi.dd.model.ItemObjectModel; import org.jbpm.workbench.wi.dd.model.Parameter; import org.jbpm.workbench.wi.dd.service.DDEditorService; +import org.jbpm.workbench.wi.dd.type.DDResourceTypeDefinition; import org.kie.internal.runtime.conf.AuditMode; import org.kie.internal.runtime.conf.DeploymentDescriptor; import org.kie.internal.runtime.conf.NamedObjectModel; @@ -66,6 +68,9 @@ public class DDEditorServiceImpl @Inject private CommentedOptionFactory commentedOptionFactory; + @Inject + private DDResourceTypeDefinition resourceTypeDefinition; + @Override public DeploymentDescriptorModel load(Path path) { return super.loadContent(path); @@ -445,4 +450,21 @@ public void createIfNotExists(Path path) { xmlDescriptor); } } + + @Override + public boolean accepts(Path path) { + return this.resourceTypeDefinition.accept(path); + } + + @Override + public List validate(Path path) { + try { + InputStream input = ioService.newInputStream(Paths.convert(path)); + DeploymentDescriptorModel ddModel = marshal(DeploymentDescriptorIO.fromXml(input)); + + return validate(path, ddModel); + } catch (Exception e) { + return Arrays.asList(newMessage(path, e.getMessage(), Level.ERROR)); + } + } }