uPortal-Project / uPortal

Enterprise open source portal built by and for the higher education community.
https://www.apereo.org/projects/uportal
Apache License 2.0
270 stars 273 forks source link

PAGS ValueMissingTester throws ClassCastException for any non-String value #1261

Open drewwills opened 6 years ago

drewwills commented 6 years ago

Describe the bug

In ancient times user attributes always had to be Strings. That hasn't been the case for a long time, but the ValueMissingTester missed the memo.

To Reproduce

    <test-group>
      <test>
        <attribute-name>communicationPreferencesLastUpdateMillis</attribute-name>
        <tester-class>org.apereo.portal.groups.pags.testers.ValueMissingTester</tester-class>
        <test-value></test-value>
      </test>
    </test-group>

Stack Trace

ERROR [uP-PortletExec-82-admin-RENDER-[user-administration]] o.a.p.g.p.d.EntityPersonAttributesGroupStore 2018-07-10 18:14:30,180 - Exception acquiring attributes for member EntityImpl (org.apereo.portal.security.IPerson) student while checking if group EntityGroupImpl (pags.Must Update Communication Preferences) Must Update Communication Preferences contains this member.
java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.String
    at org.apereo.portal.groups.pags.testers.ValueMissingTester.test(ValueMissingTester.java:43)
    at org.apereo.portal.groups.pags.TestGroup.test(TestGroup.java:36)
    at org.apereo.portal.groups.pags.PagsGroup.test(PagsGroup.java:79)
    at org.apereo.portal.groups.pags.PagsGroup.contains(PagsGroup.java:72)
    at org.apereo.portal.groups.pags.dao.EntityPersonAttributesGroupStore.contains(EntityPersonAttributesGroupStore.java:145)
    at org.apereo.portal.groups.pags.dao.EntityPersonAttributesGroupStore.findParentGroupsForEntity(EntityPersonAttributesGroupStore.java:257)
    at org.apereo.portal.groups.pags.dao.EntityPersonAttributesGroupStore.findParentGroups(EntityPersonAttributesGroupStore.java:236)
    at org.apereo.portal.groups.ReferenceIndividualGroupService.findParentGroups(ReferenceIndividualGroupService.java:178)
    at org.apereo.portal.groups.ReferenceCompositeGroupService.findParentGroups(ReferenceCompositeGroupService.java:59)
    at org.apereo.portal.groups.GroupMemberImpl.buildParentGroupsSet(GroupMemberImpl.java:105)
    at org.apereo.portal.groups.GroupMemberImpl.getParentGroups(GroupMemberImpl.java:90)
    at org.apereo.portal.groups.GroupMemberImpl.isMemberOf(GroupMemberImpl.java:155)
    at org.apereo.portal.groups.GroupMemberImpl.isDeepMemberOf(GroupMemberImpl.java:138)
    at org.apereo.portal.groups.pags.testers.AdHocGroupTester.test(AdHocGroupTester.java:104)
    at org.apereo.portal.groups.pags.TestGroup.test(TestGroup.java:36)
    at org.apereo.portal.groups.pags.PagsGroup.test(PagsGroup.java:79)
    at org.apereo.portal.groups.pags.PagsGroup.contains(PagsGroup.java:72)
    at org.apereo.portal.groups.pags.dao.EntityPersonAttributesGroupStore.contains(EntityPersonAttributesGroupStore.java:145)
    at org.apereo.portal.groups.pags.dao.EntityPersonAttributesGroupStore.findParentGroupsForEntity(EntityPersonAttributesGroupStore.java:257)
    at org.apereo.portal.groups.pags.dao.EntityPersonAttributesGroupStore.findParentGroups(EntityPersonAttributesGroupStore.java:236)
    at org.apereo.portal.groups.ReferenceIndividualGroupService.findParentGroups(ReferenceIndividualGroupService.java:178)
    at org.apereo.portal.groups.ReferenceCompositeGroupService.findParentGroups(ReferenceCompositeGroupService.java:59)
    at org.apereo.portal.groups.GroupMemberImpl.buildParentGroupsSet(GroupMemberImpl.java:105)
    at org.apereo.portal.groups.GroupMemberImpl.getParentGroups(GroupMemberImpl.java:90)
    at org.apereo.portal.groups.GroupMemberImpl.primGetAncestorGroups(GroupMemberImpl.java:175)
    at org.apereo.portal.groups.GroupMemberImpl.getAncestorGroups(GroupMemberImpl.java:74)
    at org.apereo.portal.portlets.account.UserAccountHelper.getParentGroups(UserAccountHelper.java:184)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.springframework.expression.spel.support.ReflectiveMethodExecutor.execute(ReflectiveMethodExecutor.java:117)
    at org.springframework.expression.spel.ast.MethodReference.getValueInternal(MethodReference.java:134)
    at org.springframework.expression.spel.ast.MethodReference.access$000(MethodReference.java:52)
    at org.springframework.expression.spel.ast.MethodReference$MethodValueRef.getValue(MethodReference.java:377)
    at org.springframework.expression.spel.ast.CompoundExpression.getValueInternal(CompoundExpression.java:88)
    at org.springframework.expression.spel.ast.SpelNodeImpl.getTypedValue(SpelNodeImpl.java:132)
    at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:297)
    at org.springframework.binding.expression.spel.SpringELExpression.getValue(SpringELExpression.java:84)
    at org.springframework.webflow.action.SetAction.doExecute(SetAction.java:55)
    at org.springframework.webflow.action.AbstractAction.execute(AbstractAction.java:188)
    at org.springframework.webflow.execution.AnnotatedAction.execute(AnnotatedAction.java:145)
    at org.springframework.webflow.execution.ActionExecutor.execute(ActionExecutor.java:51)
    at org.springframework.webflow.engine.ActionList.execute(ActionList.java:154)
    at org.springframework.webflow.engine.State.enter(State.java:193)
    at org.springframework.webflow.engine.Transition.execute(Transition.java:228)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.execute(FlowExecutionImpl.java:395)
    at org.springframework.webflow.engine.impl.RequestControlContextImpl.execute(RequestControlContextImpl.java:214)
    at org.springframework.webflow.engine.TransitionableState.handleEvent(TransitionableState.java:116)
    at org.springframework.webflow.engine.SubflowState.handleEvent(SubflowState.java:116)
    at org.springframework.webflow.engine.Flow.handleEvent(Flow.java:547)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.handleEvent(FlowExecutionImpl.java:390)
    at org.springframework.webflow.engine.impl.RequestControlContextImpl.handleEvent(RequestControlContextImpl.java:210)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.endActiveFlowSession(FlowExecutionImpl.java:414)
    at org.springframework.webflow.engine.impl.RequestControlContextImpl.endActiveFlowSession(RequestControlContextImpl.java:238)
    at org.springframework.webflow.engine.EndState.doEnter(EndState.java:107)
    at org.springframework.webflow.engine.State.enter(State.java:194)
    at org.springframework.webflow.engine.Transition.execute(Transition.java:228)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.execute(FlowExecutionImpl.java:395)
    at org.springframework.webflow.engine.impl.RequestControlContextImpl.execute(RequestControlContextImpl.java:214)
    at org.springframework.webflow.engine.TransitionableState.handleEvent(TransitionableState.java:116)
    at org.springframework.webflow.engine.Flow.handleEvent(Flow.java:547)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.handleEvent(FlowExecutionImpl.java:390)
    at org.springframework.webflow.engine.impl.RequestControlContextImpl.handleEvent(RequestControlContextImpl.java:210)
    at org.springframework.webflow.engine.ActionState.doEnter(ActionState.java:105)
    at org.springframework.webflow.engine.State.enter(State.java:194)
    at org.springframework.webflow.engine.Transition.execute(Transition.java:228)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.execute(FlowExecutionImpl.java:395)
    at org.springframework.webflow.engine.impl.RequestControlContextImpl.execute(RequestControlContextImpl.java:214)
    at org.springframework.webflow.engine.TransitionableState.handleEvent(TransitionableState.java:116)
    at org.springframework.webflow.engine.Flow.handleEvent(Flow.java:547)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.handleEvent(FlowExecutionImpl.java:390)
    at org.springframework.webflow.engine.impl.RequestControlContextImpl.handleEvent(RequestControlContextImpl.java:210)
    at org.springframework.webflow.engine.ViewState.handleEvent(ViewState.java:231)
    at org.springframework.webflow.engine.ViewState.resume(ViewState.java:195)
    at org.springframework.webflow.engine.Flow.resume(Flow.java:537)
    at org.springframework.webflow.engine.impl.FlowExecutionImpl.resume(FlowExecutionImpl.java:259)
    at org.springframework.webflow.executor.FlowExecutorImpl.resumeExecution(FlowExecutorImpl.java:169)
    at org.springframework.webflow.mvc.portlet.FlowHandlerAdapter.resumeFlowRender(FlowHandlerAdapter.java:299)
    at org.springframework.webflow.mvc.portlet.FlowHandlerAdapter.handleRender(FlowHandlerAdapter.java:128)
    at org.springframework.web.portlet.DispatcherPortlet.doRenderService(DispatcherPortlet.java:764)
    at org.springframework.web.portlet.FrameworkPortlet.processRequest(FrameworkPortlet.java:537)
    at org.springframework.web.portlet.FrameworkPortlet.doDispatch(FrameworkPortlet.java:483)
    at javax.portlet.GenericPortlet.render(GenericPortlet.java:251)
    at org.apereo.portal.portlet.container.FilterChainImpl.doFilter(FilterChainImpl.java:179)
    at org.apereo.portal.portlet.container.FilterChainImpl.processFilter(FilterChainImpl.java:95)
    at org.apereo.portal.portlet.container.FilterManagerImpl.processFilter(FilterManagerImpl.java:114)
    at org.apache.pluto.container.driver.PortletServlet.dispatch(PortletServlet.java:340)
    at org.apache.pluto.container.driver.PortletServlet.doGet(PortletServlet.java:261)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:635)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:742)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:728)
    at org.apache.catalina.core.ApplicationDispatcher.doInclude(ApplicationDispatcher.java:591)
    at org.apache.catalina.core.ApplicationDispatcher.include(ApplicationDispatcher.java:527)
    at org.apache.pluto.driver.container.DefaultPortletInvokerService.invoke(DefaultPortletInvokerService.java:233)
    at org.apache.pluto.driver.container.DefaultPortletInvokerService.render(DefaultPortletInvokerService.java:117)
    at sun.reflect.GeneratedMethodAccessor371.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:333)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:190)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
    at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint.proceed(MethodInvocationProceedingJoinPoint.java:84)
    at org.apereo.portal.portlet.dao.jpa.ThreadContextClassLoaderAspect.doThreadContextClassLoaderUpdate(ThreadContextClassLoaderAspect.java:63)
    at sun.reflect.GeneratedMethodAccessor215.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:627)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:616)
    at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:70)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:213)
    at com.sun.proxy.$Proxy474.render(Unknown Source)
    at org.apache.pluto.container.impl.PortletContainerImpl.doRender(PortletContainerImpl.java:157)
    at org.apereo.portal.portlet.rendering.PortletRendererImpl.doRender(PortletRendererImpl.java:456)
    at org.apereo.portal.portlet.rendering.PortletRendererImpl.doRenderMarkup(PortletRendererImpl.java:275)
    at org.apereo.portal.portlet.rendering.worker.PortletRenderExecutionWorker.callInternal(PortletRenderExecutionWorker.java:69)
    at org.apereo.portal.portlet.rendering.worker.PortletRenderExecutionWorker.callInternal(PortletRenderExecutionWorker.java:33)
    at org.apereo.portal.portlet.rendering.worker.PortletExecutionWorker$1.call(PortletExecutionWorker.java:185)
    at org.apereo.portal.portlet.rendering.worker.PortletExecutionWorker$ExecutionLifecycleCallable.call(PortletExecutionWorker.java:225)
    at org.apereo.portal.portlet.rendering.worker.PortletExecutionCallable.call(PortletExecutionCallable.java:113)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
sherazahmed83 commented 5 years ago

I was trying to run this code, with the help of @ChristianMurphy, I am able to run through gradlew check

But we observed only test cases in the package runs through "check" are the ones defined in Groovy Test, the ones defined in tag don't run. So running those test cases there may be some other way. Could you please share those steps.

drewwills commented 5 years ago

I'm not confident that the second criteria implemented by the ValueMissingTester -- "Tests whether the attribute is null or none of the values of the attribute equal the specified attribute value" -- provides any value to anyone. (I had no idea it was there until I encountered this error.)

I think we could make this task simpler by changing the class to check for null or an empty array. No type awareness required.

sherazahmed83 commented 5 years ago

`

@Override
public boolean test(IPerson person) {
    // Get the list of values for the attribute
    Object[] vals = person.getAttributeValues(getAttributeName());

    // No values, test passed
    if (vals == null) {
        return true;
    } else {
        // Loop through the values of the attribute, if one is equal
        // to the test case the test fails and returns false
        for (int i = 0; i < vals.length; i++) {
            Object obj = (Object) vals[i];

            if (obj != null) {
                String val = getStringCastedValue(obj);

                if (val.equalsIgnoreCase(testValue)) {
                    return false;
                }
            }
        }

        // None of the values equaled the test case, test passed
        return true;
    }
}

private String getStringCastedValue(Object obj) {
    if (obj instanceof String) {
        return (String) obj;
    } else if (obj instanceof Long || obj instanceof Double || obj instanceof Boolean || obj instanceof Byte
            || obj instanceof Character || obj instanceof Integer || obj instanceof Float || obj instanceof Short) {
        return String.valueOf(obj);
    }
    return "";
}

`

sherazahmed83 commented 5 years ago

The changed code methods are added here, it would be easy to merge only these 2 methods.

drewwills commented 5 years ago

A good start -- could you put it in a pull request so we can do line-by-line commenting?

sherazahmed83 commented 5 years ago

I am not able to create a Pull Request from the main branch, so I created my own repository from uPortal and created a branch from it and then created a Pull Request from that as below:

https://github.com/sherazahmed83/uPortal/pull/1