Fixes #20851: Regroup report with their values, not with the expanded…

… value

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/reports/StatusReports.scala

@@ -342,6 +342,7 @@ final case class BlockStatusReport (
 }
 final case class ValueStatusReport  (
     componentName      : String
+  , unexpanded         : String
     //only one ComponentValueStatusReport by valuex.
   , componentValues    : List[ComponentValueStatusReport]
 ) extends  ComponentStatusReport {
@@ -375,17 +376,21 @@ final case class ValueStatusReport  (
  */
 object ComponentStatusReport extends Loggable {
 
-  def merge(components: Iterable[ComponentStatusReport]): List[ ComponentStatusReport] = {
+  def merge(components: Iterable[ComponentStatusReport]): List[ComponentStatusReport] = {
     components.groupBy( _.componentName).flatMap { case (cptName, reports) =>
-
       val valueComponents = reports.collect{ case c : ValueStatusReport => c }.toList match {
-        case Nil => None
+        case Nil => Nil
         case r =>
-          val newValues = ComponentValueStatusReport.merge(r.flatMap( _.componentValues))
-          Some(ValueStatusReport(cptName, newValues))
+          r.groupBy(_.unexpanded).toList.map {
+            case (unexpanded, r) =>
+
+              val newValues = ComponentValueStatusReport.merge(r.flatMap( _.componentValues))
+              ValueStatusReport(cptName, unexpanded, newValues)
+
+          }
       }
       val groupComponent= reports.collect{ case c : BlockStatusReport => c }.toList match {
-        case Nil => None
+        case Nil => Nil
         case r =>
           import ReportingLogic._
           val reportingLogic = r.map(_.reportingLogic).reduce(
@@ -396,18 +401,13 @@ object ComponentStatusReport extends Loggable {
               case (FocusReport(a), _)                                       => FocusReport(a)
             }
           )
-          Some(BlockStatusReport(cptName, reportingLogic, ComponentStatusReport.merge(r.flatMap(_.subComponents)).toList))
+          BlockStatusReport(cptName, reportingLogic, ComponentStatusReport.merge(r.flatMap(_.subComponents)).toList) :: Nil
       }
 
-      (valueComponents,groupComponent) match {
-        case (None       , None       ) => Nil
-        case (Some(v)    , None       ) => (cptName, v) :: Nil
-        case (None       , Some(v)    ) => (cptName, v) :: Nil
-        case (Some(value), Some(group)) => (cptName, group.copy(subComponents = value :: group.subComponents)) :: Nil
-      }
+      groupComponent ::: valueComponents
 
     }
-  }.values.toList
+  }.toList
 }
 
 /**
@@ -439,15 +439,11 @@ object ComponentValueStatusReport extends Loggable {
    * them by component *unexpanded* value
    */
   def merge(values: Iterable[ComponentValueStatusReport]): List[ ComponentValueStatusReport] = {
-    val pairs = values.groupBy(_.unexpandedComponentValue).map { case (unexpanded, values) =>
-      //the unexpanded value should be the same on all values.
-      //if not, report an error for devs
-
-          ComponentValueStatusReport(unexpanded, unexpanded, values.toList.flatMap(_.messages))
-
-
+    values.groupBy(_.componentValue).toList.flatMap { case (component, values) =>
+      values.groupBy(_.unexpandedComponentValue).toList.map { case (unexpanded, values) =>
+        ComponentValueStatusReport(component, unexpanded, values.toList.flatMap(_.messages))
+      }
     }
-    pairs.toList
   }
 }
 

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/policies/DeploymentService.scala

@@ -1523,7 +1523,6 @@ object RuleExpectedReportBuilder extends Loggable {
             // structure of componentId is "Component Name", List("Component Name", "current block", "parent block", "great parent block")
             val currentPath = section.name :: path
             val children = section.children.collect{case c : SectionSpec => c }.flatMap(c => sectionToExpectedReports(currentPath)(c)).toList
-            logger.warn(children)
             BlockExpectedReport(section.name, rule, children) :: Nil
 
         }

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/services/reports/ExecutionBatch.scala

@@ -46,15 +46,13 @@ import com.normation.rudder.domain.policies.DirectiveId
 import com.normation.rudder.domain.reports._
 import com.normation.rudder.reports._
 import com.normation.rudder.reports.execution.AgentRunId
-
 import net.liftweb.common.Loggable
 
 import java.util.regex.Pattern
 import com.normation.rudder.domain.policies.PolicyMode
 import com.normation.rudder.domain.reports.ReportType.BadPolicyMode
 import com.normation.rudder.reports.execution.AgentRunWithNodeConfig
 import com.normation.rudder.domain.policies.RuleId
-
 import org.apache.commons.lang3.StringUtils
 
 import scala.annotation.tailrec
@@ -899,7 +897,6 @@ final case class ContextForNoAnswer(
       // full compliance and "success" when on changes only - but that success
       // depends upon the policy mode
       missingReportStatus = missingReportType(modes.globalComplianceMode, policyMode)
-
       component  <- directive.components
 
     } yield {
@@ -909,6 +906,7 @@ final case class ContextForNoAnswer(
     val t2 = System.nanoTime
     timer.u1 += t2-t1
 
+
     /*
      * now we have three cases:
      * - expected component without reports => missing (modulo changes only interpretation)
@@ -1120,7 +1118,7 @@ final case class ContextForNoAnswer(
         , mergeInfo.configId
         , seq.groupBy(_.directiveId).map{ case (directiveId, reportsByDirectives) =>
           (directiveId, DirectiveStatusReport(directiveId, reportsByDirectives.groupBy(_.component).toList.map { case (component, reportsByComponents) =>
-            ValueStatusReport(component, reportsByComponents.groupBy(_.keyValue).toList.map { case (keyValue, reportsByComponent) =>
+            ValueStatusReport(component, component, reportsByComponents.groupBy(_.keyValue).toList.map { case (keyValue, reportsByComponent) =>
               ComponentValueStatusReport(keyValue, keyValue, reportsByComponent.map(r => MessageStatusReport(ReportType.Unexpected, r.message)).toList)
             })
             })
@@ -1135,7 +1133,7 @@ final case class ContextForNoAnswer(
   private[this] def buildUnexpectedDirectives(reports: Seq[Reports]): Seq[DirectiveStatusReport] = {
     reports.map { r =>
       DirectiveStatusReport(r.directiveId,
-        ValueStatusReport(r.component, ComponentValueStatusReport(r.keyValue, r.keyValue, MessageStatusReport(ReportType.Unexpected, r.message) :: Nil) :: Nil
+        ValueStatusReport(r.component, r.component, ComponentValueStatusReport(r.keyValue, r.keyValue, MessageStatusReport(ReportType.Unexpected, r.message) :: Nil) :: Nil
         ) :: Nil)
 
     }
@@ -1144,7 +1142,7 @@ final case class ContextForNoAnswer(
   private[reports] def componentExpectedReportToStatusReport (reportType: ReportType, c : ComponentExpectedReport) : ComponentStatusReport = {
        c match {
          case c : ValueExpectedReport =>
-              ValueStatusReport(c.componentName, c.componentsValues.map {
+              ValueStatusReport(c.componentName,c.componentName, c.componentsValues.map {
                 case ExpectedValueId(v,_) => (v,v)
                 case ExpectedValueMatch(v,u) => (v,u)
               }.map { case(v,u) =>
@@ -1322,7 +1320,7 @@ final case class ContextForNoAnswer(
 
         // 1. start with the easy case: expected reports with a report ID
 
-        val (matched,last_unexpected) =  matchId.foldLeft((List.empty[ValueStatusReport],filteredReports)) {
+        val (matched,last_unexpected) =  matchId.foldLeft((List.empty[ComponentStatusReport],filteredReports)) {
           case ((acc, reports), expectedValueId) =>
             val (matched, left) = reports.partition(_.reportId == expectedValueId.id)
 
@@ -1333,8 +1331,8 @@ final case class ContextForNoAnswer(
 
             val (ok, unexpected) = matched.foldLeft((List.empty[ResultReports], List.empty[ResultReports])) { case ((ok, unexp), next) =>
 
-              val componentOk = next.component == expectedComponent.componentName || matchCFEngineVars.pattern.matcher(expectedComponent.componentName).matches()
-              val valueOk = next.keyValue == expectedValueId.value || matchCFEngineVars.pattern.matcher(expectedValueId.value).matches()
+              val componentOk = next.component == expectedComponent.componentName || replaceCFEngineVars(expectedComponent.componentName).matcher(next.component).matches()
+              val valueOk = next.keyValue == expectedValueId.value || replaceCFEngineVars(expectedValueId.value).matcher(next.keyValue).matches()
 
               if( componentOk && valueOk ) {
                 (next :: ok, unexp)
@@ -1348,12 +1346,33 @@ final case class ContextForNoAnswer(
               }
             }
 
-            val expectedNumber = Math.max(ok.size, 1)
-            val resOk = buildComponentValueStatus(expectedValueId, ok, componentGotAtLeastOneReport, expectedNumber, noAnswerType, policyMode)
-            val resKo = unexpected.map(r => ComponentValueStatusReport(r.component, r.keyValue, MessageStatusReport(ReportType.Unexpected, r.message) :: Nil))
 
+            val okRes: List[ComponentStatusReport] = ok.groupBy(_.component).toList.map { case (component, r) =>
+              val cv = r.groupBy(_.keyValue).toList.map { case (key, r) =>
+                ComponentValueStatusReport(key, expectedValueId.value, r.map(_.toMessageStatusReport(policyMode)))
+              }
+              ValueStatusReport(component, expectedComponent.componentName, cv)
+            }
+
+
+            val missingRes: List[ComponentStatusReport] = if (ok.isEmpty) {
+              ValueStatusReport(expectedComponent.componentName, expectedComponent.componentName, ComponentValueStatusReport( expectedValueId.value,  expectedValueId.value,  MessageStatusReport(noAnswerType, "Missing report") :: Nil) :: Nil) :: Nil
+            } else {
+              Nil
+            }
+
+            val unexpectedRes : List[ComponentStatusReport] = unexpected.groupBy(_.component).toList.map { case (component, r) =>
+              val cv = r.groupBy(_.keyValue).toList.map { case (key, r) =>
+                ComponentValueStatusReport(key, expectedValueId.value, r.map(r => MessageStatusReport(ReportType.Unexpected, r.message)))
+              }
+              ValueStatusReport(component, expectedComponent.componentName, cv)
+            }
+
+
+
+            val koRes = missingRes ::: unexpectedRes
             // we still attach the unexpected with that component since they have its reportId
-            (ValueStatusReport(expectedComponent.componentName, resOk :: resKo) :: acc, left)
+            ( ComponentStatusReport.merge(okRes ::: koRes ::: acc), left)
         }
 
         // 2. now the complicated case - do what we used to do for expected report without a reportId
@@ -1414,6 +1433,7 @@ final case class ContextForNoAnswer(
        } else {
          ValueStatusReport(
            expectedComponent.componentName
+           , expectedComponent.componentName
            , ComponentValueStatusReport.merge(pairedReportStatus)
          ) :: matched
        }

webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/domain/reports/StatusReportTest.scala

@@ -342,7 +342,7 @@ class StatusReportTest extends Specification {
         case n :: r :: _ :: d :: c :: v :: uv :: t :: m :: Nil =>
           Some(RuleNodeStatusReport(n, r, None, None, Map(DirectiveId(DirectiveUid(d)) ->
             DirectiveStatusReport(d, List(
-              ValueStatusReport(c, List(
+              ValueStatusReport(c,c, List(
                 ComponentValueStatusReport(v, uv, List(
                     MessageStatusReport(toRT(t), ?(m))
                 ))

webapp/sources/rudder/rudder-core/src/test/scala/com/normation/rudder/services/reports/ExecutionBatchTest.scala

@@ -575,14 +575,14 @@ class ExecutionBatchTest extends Specification {
     "focus on 'component' and get its two reports" in {
       withBad.compliance === ComplianceLevel(repaired = 1, unexpected = 1)
     }
-    "with bad reports return two components" in {
-      withBad.componentValues.size === 2
+    "with bad reports return 4 values" in {
+      withBad.componentValues.size === 4
     }
     "with bad reports return a component with the key values foo with one unexpected and one repaired " in {
-      withBad.componentValues("foo").flatMap(_.messages.map(_.reportType)) ===  EnforceRepaired :: Unexpected :: Nil
+      withBad.componentValues("foo").flatMap(_.messages.map(_.reportType)) ===  Unexpected :: EnforceRepaired :: Nil
     }
     "with bad reports return a component with the key values bar which is a success " in {
-      withBad.componentValues("bar").flatMap(_.messages.map(_.reportType)) ===  Unexpected :: Missing :: Nil
+      withBad.componentValues("bar").flatMap(_.messages.map(_.reportType)) ===   Missing :: Unexpected :: Nil
     }
   }
 
@@ -1077,23 +1077,23 @@ class ExecutionBatchTest extends Specification {
     val withBad   = ExecutionBatch.checkExpectedComponentWithReports(expectedComponent, badReports, ReportType.Missing, PolicyMode.Enforce, strictUnexpectedInterpretation).head
 
     "return 2 components globally (because they work by reportId)" in {
-      withGood.componentValues.size === 2
+      withGood.componentValues.size === 4
     }
     "return a total with weight 4 (2x(2 each))" in {
       withGood.compliance === ComplianceLevel(success = 2, repaired = 2)
     }
     "components are grouped by reportId - which not optimal, we would like them grouped by ${user}" in {
-      ( withGood.componentValues("/bin/createUserScript ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
+      ( withGood.componentValues.filter(_.unexpandedComponentValue ==  "/bin/createUserScript ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
         List("""Success:"alice is correctly created"""", """Repaired:"bob is correctly created"""") ) ) and
-      ( withGood.componentValues("/bin/checkRightsOK ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
+      ( withGood.componentValues.filter(_.unexpandedComponentValue ==  "/bin/checkRightsOK ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
         List("""Success:"alice rights are correct"""", """Repaired:"bob rights are correct"""") ) )
     }
     "mallory can get green compliance with other things executed that what we though" in {
-      ( withBad.compliance === ComplianceLevel(success = 2)) and
-      ( withBad.componentValues("/bin/createUserScript ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
-        List("""Success:"mallory is correctly created"""") ) ) and
-      ( withBad.componentValues("/bin/checkRightsOK ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
-        List("""Success:"mallory rights are correct"""") ) )
+      ( withBad.compliance === ComplianceLevel(unexpected = 2, missing = 2)) and
+      ( withBad.componentValues.filter(_.unexpandedComponentValue == "/bin/createUserScript ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
+        List("""Unexpected:"mallory is correctly created"""", """Missing:"Missing report"""") ) ) and
+      ( withBad.componentValues.filter(_.unexpandedComponentValue == "/bin/checkRightsOK ${user}").flatMap(_.messages.map(_.debugString)) must containTheSameElementsAs(
+        List("""Unexpected:"mallory rights are correct"""", """Missing:"Missing report"""") ) )
 
     }
   }

webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/services/ComplianceData.scala

@@ -279,6 +279,7 @@ final case class BlockComplianceLine (
 
 final case class ValueComplianceLine (
     component   : String
+  , unexpanded  : String
   , compliance  : ComplianceLevel
   , details     : JsTableData[ComponentValueComplianceLine]
   , noExpand    : Boolean
@@ -287,6 +288,7 @@ final case class ValueComplianceLine (
   val json = {
     JsObj (
         ( "component"         -> escapeHTML(component)    )
+      , ( "unexpanded"        -> escapeHTML(unexpanded) )
       , ( "compliance"        -> jsCompliance(compliance) )
       , ( "compliancePercent" -> compliance.computePercent().compliance)
       , ( "details"           -> details.json             )
@@ -310,6 +312,7 @@ final case class ValueComplianceLine (
  */
 final case class ComponentValueComplianceLine (
     value       : String
+  , unexpandedValue : String
   , messages    : List[(String, String)]
   , compliance  : ComplianceLevel
   , status      : String
@@ -319,6 +322,7 @@ final case class ComponentValueComplianceLine (
   val json = {
     JsObj (
         ( "value"             -> escapeHTML(value) )
+      , ( "unexpanded"        -> escapeHTML(unexpandedValue) )
       , ( "status"            -> status )
       , ( "statusClass"       -> statusClass )
       , ( "messages"          -> JsArray(messages.map{ case(s, m) => JsObj(("status" -> s), ("value" -> escapeHTML(m)))}))
@@ -543,6 +547,7 @@ object ComplianceData extends Loggable {
 
       ValueComplianceLine(
           component.componentName
+        , component.unexpanded
         , component.compliance
         , values
         , noExpand
@@ -563,11 +568,13 @@ object ComplianceData extends Loggable {
     } yield {
       val severity = ReportType.getWorseType(value.messages.map( _.reportType)).severity
       val status = getDisplayStatusFromSeverity(severity)
-      val key = value.unexpandedComponentValue
+      val key = value.componentValue
+      println(key)
       val messages = value.messages.map(x => (x.reportType.severity, x.message.getOrElse("")))
 
       ComponentValueComplianceLine(
           key
+        , value.unexpandedComponentValue
         , messages
         , value.compliance
         , status

webapp/sources/rudder/rudder-web/src/main/webapp/javascript/rudder/rudder-datatable.js

@@ -930,6 +930,10 @@ function createComponentTable(isTopLevel, isNodeView, contextPath) {
         } else {
           $(nTd).addClass("noExpand");
         }
+        if( oData.unexpanded !== undefined && oData["unexpanded"] !== sData) {
+          var elem = $("<i class=\"fa fa-question-circle icon-info\" title=\"original value is "+ oData["unexpanded"]+"\"></i>")
+          $(nTd).append(elem);
+        }
       }
   } , {
       "sWidth": complianceWidth
@@ -983,6 +987,12 @@ function createNodeComponentValueTable(contextPath) {
       "sWidth": "20%"
     , "mDataProp": "value"
     , "sTitle": "Value"
+    , "fnCreatedCell" : function (nTd, sData, oData, iRow, iCol) {
+        if(oData["unexpanded"] !== sData) {
+          var elem = $("<i class=\"fa fa-question-circle icon-info\" title=\"original value is "+ oData["unexpanded"]+"\"></i>")
+          $(nTd).append(elem);
+        }
+      }
   } , {
       "sWidth": "62.4%"
     , "mDataProp": "messages"
@@ -1044,6 +1054,12 @@ function createRuleComponentValueTable (contextPath) {
       "sWidth": componentSize
     , "mDataProp": "value"
     , "sTitle": "Value"
+    , "fnCreatedCell" : function (nTd, sData, oData, iRow, iCol) {
+        if(oData["unexpanded"] !== sData) {
+          var elem = $("<i class=\"fa fa-question-circle icon-info\" title=\"original value is "+ oData["unexpanded"]+"\"></i>")
+          $(nTd).append(elem);
+        }
+      }
   } , {
         "sWidth": complianceWidth
       , "mDataProp": "compliancePercent"