Merge pull request #1 from moqui/master

Merged master branch

framework/build.gradle

@@ -136,6 +136,7 @@ dependencies {
     compile 'com.h2database:h2:1.4.199' // MPL 2.0, EPL 1.0
 
     // Jackson Databind (JSON, etc)
+    // NOTE: ran into ElasticSearch jar-hell issue on 'module-info' after update to 2.10.0, something weird there
     compile 'com.fasterxml.jackson.core:jackson-databind:2.9.9.2'
     compile 'com.fasterxml.jackson.dataformat:jackson-dataformat-cbor:2.9.9'
 

framework/data/MoquiInstallData.xml

@@ -30,6 +30,10 @@ along with this software (see the LICENSE.md file). If not, see
     <moqui.security.user.NotificationTopic topic="WebServletError" description="Web Servlet Error"
             typeString="danger" showAlert="Y" persistOnSend="Y" receiveNotifications="N" isPrivate="Y" emailTemplateId="NOTIFICATION"
             titleTemplate="Web Error ${errorCode?:''} (${username?:'no user'}) ${path?:''} ${message?:'N/A'}"/>
+    <moqui.security.user.NotificationTopic topic="ServiceJobError" description="Service Job Error"
+            typeString="danger" showAlert="Y" persistOnSend="Y" receiveNotifications="N" isPrivate="Y" emailTemplateId="NOTIFICATION"
+            titleTemplate="Job Error ${serviceCallRun.jobName?:''} [${serviceCallRun.jobRunId?:''}] ${serviceCallRun.errors?:'N/A'}"
+            linkTemplate="/vapps/system/ServiceJob/JobRuns/JobRunDetail?jobRunId=${serviceCallRun.jobRunId}"/>
 
     <!-- ========== Framework Scheduled ServiceJob Data ========== -->
     <!-- Handy cron strings: [0 0 2 * * ?] every night at 2:00 am, [0 0/15 * * * ?] every 15 minutes, [0 0/2 * * * ?] every 2 minutes -->

framework/entity/TestEntities.xml

@@ -38,9 +38,9 @@ along with this software (see the LICENSE.md file). If not, see
     </entity>
     <entity entity-name="Bar" package="moqui.test" sequence-bank-size="100">
         <field name="barId" type="id" is-pk="true" />
-        <field name="fooId" type="id"></field>
-        <field name="rank" type="number-integer"></field>
-        <field name="score" type="number-decimal"></field>
+        <field name="fooId" type="id"/>
+        <field name="rank" type="number-integer"/>
+        <field name="score" type="number-decimal"/>
     </entity>
     <view-entity entity-name="FooBar" package="moqui.test">
         <member-entity entity-alias="T1" entity-name="moqui.test.Foo" />
@@ -66,4 +66,8 @@ along with this software (see the LICENSE.md file). If not, see
         <field name="testCurrencyAmount" type="currency-amount"/>
         <field name="testCurrencyPrecise" type="currency-precise"/>
     </entity>
+    <entity entity-name="TestIntPk" package="moqui.test">
+        <field name="intId" type="number-integer" is-pk="true"/>
+        <field name="testMedium" type="text-medium"/>
+    </entity>
 </entities>

framework/src/main/groovy/org/moqui/impl/context/ArtifactExecutionFacadeImpl.groovy

@@ -90,7 +90,7 @@ class ArtifactExecutionFacadeImpl implements ArtifactExecutionFacade {
         if (!isPermitted(aeii, lastAeii, requiresAuthz, countTarpit, true, null)) {
             Deque<ArtifactExecutionInfo> curStack = getStack()
             StringBuilder warning = new StringBuilder()
-            warning.append("User ${eci.user.username ?: eci.user.userId} is not authorized for ${aeii.getActionDescription()} on ${aeii.getTypeDescription()} ${aeii.getName()}")
+            warning.append("User ${eci.user.username ?: eci.user.userId ?: '[No User]'} is not authorized for ${aeii.getActionDescription()} on ${aeii.getTypeDescription()} ${aeii.getName()}")
 
             ArtifactAuthorizationException e = new ArtifactAuthorizationException(warning.toString(), aeii, curStack)
             // end users see this message in vuet mode so better not to add all of this to the main message:

framework/src/main/groovy/org/moqui/impl/context/TransactionFacadeImpl.groovy

@@ -68,6 +68,8 @@ class TransactionFacadeImpl implements TransactionFacade {
 
             ut = transactionInternal.getUserTransaction()
             tm = transactionInternal.getTransactionManager()
+
+            logger.info("Internal transaction manager initialized: UserTransaction class ${ut?.class?.name}, TransactionManager class ${tm?.class?.name}")
         } else {
             throw new IllegalArgumentException("No transaction-jndi or transaction-internal elements found in Moqui Conf XML file")
         }
@@ -470,7 +472,7 @@ class TransactionFacadeImpl implements TransactionFacade {
                     if (causeThrowable != null) {
                         String causeString = causeThrowable.toString()
                         if (causeString.contains("org.eclipse.jetty.io.EofException")) {
-                            logger.warn("Transaction rollback. The rollback was originally caused by: ${causeMessage}\n${causeString}")
+                            logger.warn("Transaction set rollback only. The rollback was originally caused by: ${causeMessage}\n${causeString}")
                         } else {
                             logger.warn("Transaction set rollback only. The rollback was originally caused by: ${causeMessage}", causeThrowable)
                             logger.warn("Transaction set rollback only for [${causeMessage}]. Here is the current location: ", rbLocation)

framework/src/main/groovy/org/moqui/impl/context/WebFacadeImpl.groovy

@@ -958,6 +958,8 @@ class WebFacadeImpl implements WebFacade {
     void handleServiceRestCall(List<String> extraPathNameList) {
         ContextStack parmStack = (ContextStack) getParameters()
 
+        logger.info("Service REST for ${request.getMethod()} to ${request.getPathInfo()} headers ${request.headerNames.collect()} parameters ${getRequestParameters()}")
+
         // check for login, etc error messages
         if (eci.message.hasError()) {
             String errorsString = eci.message.errorsString

framework/src/main/groovy/org/moqui/impl/entity/EntityDataFeed.groovy

@@ -17,6 +17,7 @@ import groovy.transform.CompileStatic
 import org.moqui.entity.EntityCondition
 import org.moqui.entity.EntityException
 import org.moqui.entity.EntityList
+import org.moqui.entity.EntityNotFoundException
 import org.moqui.entity.EntityValue
 import org.moqui.impl.context.ExecutionContextFactoryImpl
 import org.moqui.impl.context.ExecutionContextImpl
@@ -119,7 +120,13 @@ class EntityDataFeed {
         if (isUpdate && oldValues == null) return
 
         // see if this should be added to the feed
-        ArrayList<DocumentEntityInfo> entityInfoList = getDataFeedEntityInfoList(ev.getEntityName())
+        ArrayList<DocumentEntityInfo> entityInfoList
+        try {
+            entityInfoList = getDataFeedEntityInfoList(ev.getEntityName())
+        } catch (Throwable t) {
+            logger.error("Error getting DataFeed entity info, not registering value for entity ${ev.getEntityName()}", t)
+            return
+        }
         // if (ev.getEntityName().endsWith(debugEntityName)) logger.warn("======= dataFeedCheckAndRegister entityInfoList size ${entityInfoList.size()}")
         if (entityInfoList.size() > 0) {
             // logger.warn("============== found registered entity [${ev.getEntityName()}] value: ${ev}")
@@ -241,6 +248,10 @@ class EntityDataFeed {
 
         for (String dataDocumentId in fullDataDocumentIdSet) {
             Map<String, DocumentEntityInfo> entityInfoMap = getDataDocumentEntityInfo(dataDocumentId)
+            if (entityInfoMap == null) {
+                logger.error("Invalid or missing DataDocument ${dataDocumentId}, ignoring for real time feed")
+                continue
+            }
             // got a Map for all entities in the document, now split them by entity and add to master list for the entity
             for (Map.Entry<String, DocumentEntityInfo> entityInfoMapEntry in entityInfoMap.entrySet()) {
                 String entityName = entityInfoMapEntry.getKey()
@@ -281,6 +292,10 @@ class EntityDataFeed {
         }
 
         String primaryEntityName = dataDocument.primaryEntityName
+        if (!efi.isEntityDefined(primaryEntityName)) {
+            logger.error("Could not find primary entity ${primaryEntityName} for DataDocument ${dataDocumentId}")
+            return null
+        }
         EntityDefinition primaryEd = efi.getEntityDefinition(primaryEntityName)
 
         Map<String, DocumentEntityInfo> entityInfoMap = [:]
@@ -614,7 +629,7 @@ class EntityDataFeed {
                     } else {
                         List<EntityCondition> condList = []
                         for (Map pkFieldValueMap in primaryPkFieldValues) {
-                            Map condAndMap = [:]
+                            Map<String, Object> condAndMap = new LinkedHashMap<String, Object>()
                             // if pk field is aliased used the alias name
                             for (String pkFieldName in primaryPkFieldNames)
                                 condAndMap.put(pkFieldAliasMap.get(pkFieldName), pkFieldValueMap.get(pkFieldName))

framework/src/main/groovy/org/moqui/impl/service/ServiceCallJobImpl.groovy

@@ -263,27 +263,47 @@ class ServiceCallJobImpl extends ServiceCallImpl implements ServiceCallJob {
                             messages:messages, hasError:(hasError ? 'Y' : 'N'), errors:errors] as Map<String, Object>)
                         .disableAuthz().call()
 
-                // if topic send NotificationMessage
-                if (topic) {
-                    NotificationMessage nm = threadEci.makeNotificationMessage().topic(topic)
-                    Map<String, Object> msgMap = new HashMap<>()
+                // notifications
+                Map<String, Object> msgMap = (Map<String, Object>) null
+                EntityList serviceJobUsers = (EntityList) null
+                if (topic || hasError) {
+                    msgMap = new HashMap<>()
                     msgMap.put("serviceCallRun", [jobName:jobName, description:jobDescription, jobRunId:jobRunId,
-                            endTime:nowTimestamp, messages:messages, hasError:hasError, errors:errors])
+                          endTime:nowTimestamp, messages:messages, hasError:hasError, errors:errors])
                     msgMap.put("parameters", parameters)
                     msgMap.put("results", results)
+
+                    serviceJobUsers = threadEci.entityFacade.find("moqui.service.job.ServiceJobUser")
+                            .condition("jobName", jobName).useCache(true).disableAuthz().list()
+                }
+
+                // if topic send NotificationMessage
+                if (topic) {
+                    NotificationMessage nm = threadEci.makeNotificationMessage().topic(topic)
                     nm.message(msgMap)
 
                     if (currentUserId) nm.userId(currentUserId)
-                    EntityList serviceJobUsers = threadEci.entity.find("moqui.service.job.ServiceJobUser")
-                            .condition("jobName", jobName).useCache(true).disableAuthz().list()
                     for (EntityValue serviceJobUser in serviceJobUsers)
-                        if (serviceJobUser.receiveNotifications != 'N')
-                            nm.userId((String) serviceJobUser.userId)
+                        if (serviceJobUser.receiveNotifications != 'N') nm.userId((String) serviceJobUser.userId)
 
                     nm.type(hasError ? NotificationMessage.danger : NotificationMessage.success)
                     nm.send()
                 }
 
+                // if hasError send general error notification
+                if (hasError) {
+                    NotificationMessage nm = threadEci.makeNotificationMessage().topic("ServiceJobError")
+                            .type(NotificationMessage.danger)
+                            .title('''Job Error ${serviceCallRun.jobName?:''} [${serviceCallRun.jobRunId?:''}] ${serviceCallRun.errors?:'N/A'}''')
+                            .message(msgMap)
+
+                    if (currentUserId) nm.userId(currentUserId)
+                    for (EntityValue serviceJobUser in serviceJobUsers)
+                        if (serviceJobUser.receiveNotifications != 'N') nm.userId((String) serviceJobUser.userId)
+
+                    nm.send()
+                }
+
                 return results
             } catch (Throwable t) {
                 logger.error("Error in service job handling", t)

framework/src/main/groovy/org/moqui/impl/service/runner/EntityAutoServiceRunner.groovy

@@ -144,7 +144,9 @@ class EntityAutoServiceRunner implements ServiceRunner {
 
             Object pkValue = parameters.get(singlePkField.name)
             if (!ObjectUtilities.isEmpty(pkValue)) {
-                newEntityValue.set(singlePkField.name, pkValue)
+                // convert from String if parameter type is String, PK field type may not be
+                if (pkValue instanceof CharSequence) newEntityValue.setString(singlePkField.name, pkValue.toString())
+                else newEntityValue.set(singlePkField.name, pkValue)
             } else {
                 // if it has a default value don't sequence the PK
                 if (singlePkField.defaultStr == null || singlePkField.defaultStr.isEmpty()) {

framework/src/main/groovy/org/moqui/impl/webapp/MoquiServlet.groovy

@@ -114,11 +114,27 @@ class MoquiServlet extends HttpServlet {
         MDC.remove("moqui_userId")
         MDC.remove("moqui_visitorId")
 
+        // make sure no transaction is active in thread
+        if (ecfi.transactionFacade.isTransactionInPlace()) {
+            logger.warn("In MoquiServlet.service there is already a transaction for thread [${Thread.currentThread().id}:${Thread.currentThread().name}], closing")
+            try {
+                ecfi.transactionFacade.destroyAllInThread()
+            } catch (Throwable t) {
+                logger.error("Error destroying transaction already in place in MoquiServlet.service", t)
+            }
+        }
+
+        // check for active ExecutionContext
         ExecutionContextImpl activeEc = ecfi.activeContext.get()
         if (activeEc != null) {
             logger.warn("In MoquiServlet.service there is already an ExecutionContext for user ${activeEc.user.username} (from ${activeEc.forThreadId}:${activeEc.forThreadName}) in this thread (${Thread.currentThread().id}:${Thread.currentThread().name}), destroying")
-            activeEc.destroy()
+            try {
+                activeEc.destroy()
+            } catch (Throwable t) {
+                logger.error("Error destroying ExecutionContext already in place in MoquiServlet.service", t)
+            }
         }
+        // get a new ExecutionContext
         ExecutionContextImpl ec = ecfi.getEci()
 
         /** NOTE to set render settings manually do something like this, but it is not necessary to set these things
@@ -210,7 +226,7 @@ class MoquiServlet extends HttpServlet {
 
         if (ecfi != null && errorCode == HttpServletResponse.SC_INTERNAL_SERVER_ERROR && !isBrokenPipe(origThrowable)) {
             ExecutionContextImpl ec = ecfi.getEci()
-            ec.makeNotificationMessage().topic("WebServletError").type(NotificationMessage.NotificationType.danger)
+            ec.makeNotificationMessage().topic("WebServletError").type(NotificationMessage.danger)
                     .title('''Web Error ${errorCode?:''} (${username?:'no user'}) ${path?:''} ${message?:'N/A'}''')
                     .message([errorCode:errorCode, errorType:errorType, message:message, exception:origThrowable?.toString(),
                         path:ec.web.getPathInfo(), parameters:ec.web.getRequestParameters(), username:ec.user.username] as Map<String, Object>)

framework/src/test/groovy/ServiceCrudImplicit.groovy

@@ -83,4 +83,17 @@ class ServiceCrudImplicit extends Specification {
         then:
         testEntityCheck.testMedium == "Test Name A"
     }
+
+    def "create and find TestIntPk 123 with service"() {
+        when:
+        // create with String for ID though is type number-integer, test single PK type conversion
+        ec.service.sync().name("create#moqui.test.TestIntPk").parameters([intId:"123", testMedium:"Test Name"]).call()
+        EntityValue testString = ec.entity.find("moqui.test.TestIntPk").condition([intId:"123"]).one()
+        EntityValue testInt = ec.entity.find("moqui.test.TestIntPk").condition([intId:123]).one()
+
+        then:
+        testString?.testMedium == "Test Name"
+        testInt?.testMedium == "Test Name"
+    }
+
 }

moqui-util/src/main/java/org/moqui/util/CollectionUtilities.java

@@ -112,17 +112,19 @@ public static void filterMapListByDate(List<Map> theList, String fromDateName, S
         filterMapListByDate(theList, fromDateName, thruDateName, compareStamp);
     }
 
-    /**
-     * Order list elements in place (modifies the list passed in), returns the list for convenience
-     */
+    /** Order list elements in place (modifies the list passed in), returns the list for convenience */
     public static List<Map<String, Object>> orderMapList(List<Map<String, Object>> theList, List<? extends CharSequence> fieldNames) {
+        return orderMapList(theList, fieldNames, null);
+    }
+    public static List<Map<String, Object>> orderMapList(List<Map<String, Object>> theList, List<? extends CharSequence> fieldNames, Boolean nullsLast) {
         if (fieldNames == null) throw new IllegalArgumentException("Cannot order List of Maps with null order by field list");
-        if (theList != null && fieldNames.size() > 0) theList.sort(new MapOrderByComparator(fieldNames));
+        if (theList != null && fieldNames.size() > 0) theList.sort(new MapOrderByComparator(fieldNames).nullsLast(nullsLast));
         return theList;
     }
 
     public static class MapOrderByComparator implements Comparator<Map> {
         String[] fieldNameArray;
+        Boolean nullsLast = null;
 
         public MapOrderByComparator(List<? extends CharSequence> fieldNameList) {
             ArrayList<String> fieldArrayList = new ArrayList<>();
@@ -143,6 +145,11 @@ public MapOrderByComparator(List<? extends CharSequence> fieldNameList) {
             // logger.warn("Order list by " + Arrays.asList(fieldNameArray));
         }
 
+        public MapOrderByComparator nullsLast(Boolean nl) {
+            nullsLast = nl;
+            return this;
+        }
+
         @SuppressWarnings("unchecked")
         @Override public int compare(Map map1, Map map2) {
             if (map1 == null) return -1;
@@ -161,14 +168,17 @@ public MapOrderByComparator(List<? extends CharSequence> fieldNameList) {
                     ignoreCase = true;
                     fieldName = fieldName.substring(1);
                 }
+
+                boolean nullsFirst = nullsLast != null ? !nullsLast.booleanValue() : ascending;
+
                 Comparable value1 = (Comparable) map1.get(fieldName);
                 Comparable value2 = (Comparable) map2.get(fieldName);
                 // NOTE: nulls go earlier in the list for ascending, later in the list for !ascending
                 if (value1 == null) {
-                    if (value2 != null) return ascending ? 1 : -1;
+                    if (value2 != null) return nullsFirst ? -1 : 1;
                 } else {
                     if (value2 == null) {
-                        return ascending ? -1 : 1;
+                        return nullsFirst ? 1 : -1;
                     } else {
                         if (ignoreCase && value1 instanceof String && value2 instanceof String) {
                             int comp = ((String) value1).compareToIgnoreCase((String) value2);