Fixes #18302: Make possible to give a revision for directives in rule…

… and get correct technique files

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/Technique.scala

@@ -58,7 +58,7 @@ final case class TechniqueName(value: String) extends AnyVal with Ordered[Techni
  */
 final case class TechniqueId(name: TechniqueName, version: TechniqueVersion) extends Ordered[TechniqueId] {
   // intented for debug/log, not serialization
-  def show = serialize
+  def debugString = serialize
   // a technique
   def serialize = name.value + "/" + version.serialize
   def withDefaultRevId = TechniqueId(name, version.withDefaultRevId)

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/domain/TechniqueVersion.scala

@@ -62,7 +62,7 @@ final case class TechniqueVersion(version: Version, revId: Option[RevId] = None)
   def withDefaultRevId = TechniqueVersion(version)
 
   // intended for debug
-  def show = serialize
+  def debugString = serialize
   // for serialisation on path
   def serialize = revId.fold(
     version.toVersionString
@@ -82,7 +82,7 @@ object TechniqueVersion {
    * not have. So we must accept also leading number+":".
    */
   def authorizedChar = """([0-9]:)*[.0-9]+""".r.pattern
-  
+
   def parse(value: String): Either[String, TechniqueVersion] = {
     val (v, revId) = {
       val parts = value.split("\\+")

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/services/TechniqueLibraryUpdateNotification.scala

@@ -54,10 +54,10 @@ final case class Deleted(cat: TechniqueCategory) extends TechniqueCategoryModTyp
 final case class Moved(oldId: TechniqueCategoryId, newId: TechniqueCategoryId) extends TechniqueCategoryModType
 }
 
-sealed trait TechniqueVersionModType
-final case object VersionDeleted extends TechniqueVersionModType
-final case object VersionAdded extends TechniqueVersionModType
-final case object VersionUpdated extends TechniqueVersionModType
+sealed trait TechniqueVersionModType { def name: String }
+final case object VersionDeleted extends TechniqueVersionModType { val name = "deleted" }
+final case object VersionAdded extends TechniqueVersionModType { val name = "added" }
+final case object VersionUpdated extends TechniqueVersionModType { val name = "updated" }
 
 sealed trait TechniquesLibraryUpdateType {
   val techniqueName: TechniqueName

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/services/impl/GitTechniqueReader.scala

@@ -341,12 +341,12 @@ class GitTechniqueReader(
                          }
                          ids match {
                            case Nil =>
-                             logger.error(s"Metadata file ${techniqueDescriptorName} was not found for technique with id ${techniqueId.show}.")
+                             logger.error(s"Metadata file ${techniqueDescriptorName} was not found for technique with id ${techniqueId.debugString}.")
                              Option.empty[ObjectStream]
                            case h :: Nil =>
                              Some(repo.db.open(h).openStream)
                            case _ =>
-                             logger.error(s"There is more than one Technique with ID '${techniqueId.show}', what is forbidden. Please check if several categories have that Technique, and rename or delete the clones.")
+                             logger.error(s"There is more than one Technique with ID '${techniqueId.debugString}', what is forbidden. Please check if several categories have that Technique, and rename or delete the clones.")
                              Option.empty[ObjectStream]
                          }
                        } catch {
@@ -692,7 +692,7 @@ class GitTechniqueReader(
       catId match {
         case RootTechniqueCategoryId =>
           val cat = info.rootCategory.getOrElse(
-              throw new RuntimeException(s"Can not find the parent (root) category '${descriptorFile.getParent}' for technique '${techniqueId.show}'")
+              throw new RuntimeException(s"Can not find the parent (root) category '${descriptorFile.getParent}' for technique '${techniqueId.debugString}'")
           )
           info.rootCategory = Some(cat.copy(techniqueIds = cat.techniqueIds.union(Set(techniqueId) )))
           true
@@ -703,7 +703,7 @@ class GitTechniqueReader(
               info.subCategories(sid) = cat.copy(techniqueIds = cat.techniqueIds.union(Set(techniqueId) ))
               true
             case None =>
-              logger.error(s"Can not find the parent (root) category '${descriptorFile.getParent}' for technique '${techniqueId.show}'")
+              logger.error(s"Can not find the parent (root) category '${descriptorFile.getParent}' for technique '${techniqueId.debugString}'")
               false
           }
       }

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/services/impl/TechniqueRepositoryImpl.scala

@@ -110,7 +110,7 @@ class TechniqueRepositoryImpl(
           else {
             val details = modifiedPackages.values.map {
               case TechniqueDeleted(name, versions) => s"['${name.value}': deleted (${versions.mkString(", ")})]"
-              case TechniqueUpdated(name, mods)     => s"['${name.value}': updated (${mods.map(x => s"${x._1.show}: ${x._2}").mkString(", ")})]"
+              case TechniqueUpdated(name, mods)     => s"['${name.value}': updated (${mods.map(x => s"${x._1.debugString}: ${x._2.name}").mkString(", ")})]"
             }
 
             "found modified technique(s): " + details.mkString(", ")
@@ -296,5 +296,5 @@ class TechniqueRepositoryImpl(
     } yield {
       cat
     }
-  }.notOptional(s"The parent category for '${id.show}' was not found.")
+  }.notOptional(s"The parent category for '${id.debugString}' was not found.")
 }

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/xmlparsers/SectionSpecParser.scala

@@ -52,7 +52,7 @@ class SectionSpecParser(variableParser:VariableSpecParser) extends Loggable {
     val sections = policy \\ SECTIONS_ROOT
 
     (if (sections.size > 1) {
-      val err = s"In ${id.show} -> ${policyName} : Only one <sections> marker is allowed in the entire file"
+      val err = s"In ${id.debugString} -> ${policyName} : Only one <sections> marker is allowed in the entire file"
       logger.error(err)
       Left(LoadTechniqueError.Parsing(err).asInstanceOf[LoadTechniqueError])
     } else {
@@ -206,21 +206,21 @@ class SectionSpecParser(variableParser:VariableSpecParser) extends Loggable {
 
     def parseOneVariable(sectionName: String, node: Node): Either[LoadTechniqueError, (SectionChildSpec, List[SectionChildSpec])] = {
       variableParser.parseSectionVariableSpec(sectionName, node).leftMap(err =>
-        LoadTechniqueError.Chained(s"In ${id.show} -> ${policyName}, couldn't parse variable ${node}", err)
+        LoadTechniqueError.Chained(s"In ${id.debugString} -> ${policyName}, couldn't parse variable ${node}", err)
       )
     }
 
     def parseOneSection(node: Node, id: TechniqueId, policyName: String) : Either[LoadTechniqueError, SectionSpec] = {
       parseSection(node, id, policyName).leftMap(err =>
-        LoadTechniqueError.Chained(s"Couldn't parse Section in ${id.show} -> ${policyName} for XML: ${node}", err)
+        LoadTechniqueError.Chained(s"Couldn't parse Section in ${id.debugString} -> ${policyName} for XML: ${node}", err)
       )
     }
 
     (node.child.filter(child => !child.isEmpty && child.label != "#PCDATA").toList.traverse { child =>
       child.label match {
         case v if SectionVariableSpec.isVariable(v) => parseOneVariable(sectionName, child).map { case (a,b) => a :: b }
         case s if SectionSpec.isSection(s)          => parseOneSection(child,id,policyName).map(s => s :: Nil)
-        case x => Left(LoadTechniqueError.Parsing(s"Unexpected <${SECTIONS_ROOT}> child element in technique ${id.show}: ${x}"))
+        case x => Left(LoadTechniqueError.Parsing(s"Unexpected <${SECTIONS_ROOT}> child element in technique ${id.debugString}: ${x}"))
       }
     }).map( _.flatten)
   }

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/cfclerk/xmlparsers/TechniqueParser.scala

@@ -116,7 +116,7 @@ class TechniqueParser(
             _            <- { // all agent config types must be different
                               val duplicated = agentConfigs.map(_.agentType.id).groupBy(identity).collect { case (id, seq) if(seq.size > 1) => id }
                               if(duplicated.nonEmpty) {
-                                Left(LoadTechniqueError.Parsing(s"Error when parsing technique with ID '${id.show}': these agent configurations are declared " +
+                                Left(LoadTechniqueError.Parsing(s"Error when parsing technique with ID '${id.debugString}': these agent configurations are declared " +
                                                                 s"several times: '${duplicated.mkString("','")}' (note that <TMLS>, <BUNDLES> and <FILES> " +
                                                                 s"sections under root <TECHNIQUE> tag build a 'cfengine-community' agent configuration)"
                                 ))
@@ -127,7 +127,7 @@ class TechniqueParser(
 
             // System technique should not have run hooks, this is not supported:
             _ = if(isSystem && agentConfigs.exists( a => a.runHooks.nonEmpty ) ) {
-              logEffect.warn(s"System Technique with ID '${id.show}' has agent run hooks defined. This is not supported on system technique.")
+              logEffect.warn(s"System Technique with ID '${id.debugString}' has agent run hooks defined. This is not supported on system technique.")
             }
 
             // 4.3: does the technique support generation without directive merge (i.e mutli directive)
@@ -194,7 +194,7 @@ class TechniqueParser(
         AgentType.fromValue(name) match {
           case Right(agentType) => Some(agentType)
           case Left(err)        =>
-            val msg = s"Error when parsing technique with id '${id.show}', agent type='${name}' is not known and the corresponding config will be ignored: ${err.fullMsg}"
+            val msg = s"Error when parsing technique with id '${id.debugString}', agent type='${name}' is not known and the corresponding config will be ignored: ${err.fullMsg}"
             logEffect.warn(msg)
             None
         }
@@ -245,7 +245,7 @@ class TechniqueParser(
   private[this] def parseSysvarSpecs(xml: Node, id:TechniqueId) : Either[LoadTechniqueError, Set[SystemVariableSpec]] = {
     (xml \ SYSTEMVARS_ROOT \ SYSTEMVAR_NAME).toList.traverse { x =>
       systemVariableSpecService.get(x.text).leftMap(_ =>
-        LoadTechniqueError.Parsing(s"The system variable ${x.text} is not defined: perhaps the metadata.xml for technique '${id.show}' is not up to date")
+        LoadTechniqueError.Parsing(s"The system variable ${x.text} is not defined: perhaps the metadata.xml for technique '${id.debugString}' is not up to date")
       ).toValidatedNel
     }.fold(errs => Left(LoadTechniqueError.Accumulated(errs)), x => Right(x.toSet))
   }
@@ -303,8 +303,8 @@ class TechniqueParser(
               Right(TechniqueResourceIdByPath(fileToList(path.getParentFile), techniqueId.version.revId, name))
             } else {
               if(n.startsWith(RUDDER_CONFIGURATION_REPOSITORY)) { //most likely an user error, issue a warning
-                logEffect.warn(s"Resource named '${n}' for technique '${techniqueId.show}' starts with ${RUDDER_CONFIGURATION_REPOSITORY} which is not followed by a '/'. " +
-                    "If you meant to use a relative path from configuration-repository directory for the resource, it is an error.")
+                logEffect.warn(s"Resource named '${n}' for technique '${techniqueId.debugString}' starts with ${RUDDER_CONFIGURATION_REPOSITORY} which is not followed by a '/'. " +
+                               "If you meant to use a relative path from configuration-repository directory for the resource, it is an error.")
               }
               Right(TechniqueResourceIdByName(techniqueId, n))
             }
@@ -403,11 +403,11 @@ class TechniqueParser(
             RunHook.Parameter(pname, pvalue)
           }
          ))
-      }).leftMap(err => LoadTechniqueError.Parsing(s"Error: in technique '${id.show}', tried to parse a <${RUN_HOOKS}> xml, but XML is invalid: "+ err.fullMsg))
+      }).leftMap(err => LoadTechniqueError.Parsing(s"Error: in technique '${id.debugString}', tried to parse a <${RUN_HOOKS}> xml, but XML is invalid: " + err.fullMsg))
     }
 
     if(xml.label != RUN_HOOKS) {
-      Left(LoadTechniqueError.Parsing(s"Error in techni in technique '${id.show}', this is not a valid <${RUN_HOOKS}>: ${xml}"))
+      Left(LoadTechniqueError.Parsing(s"Error in techni in technique '${id.debugString}', this is not a valid <${RUN_HOOKS}>: ${xml}"))
     } else {
 
       // parse each direct children, but only proceed with PRE and POST.

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/batch/AutomaticReportLogger.scala

@@ -258,11 +258,11 @@ class AutomaticReportLogger(
           val n         = allNodes.get(report.nodeId).map(_.hostname).getOrElse("Unknown node")
           val rid       = report.ruleId.value
           val r         = rules.get(report.ruleId).map(_.name).getOrElse("Unknown rule")
-          val did       = report.directiveRId.show
+          val did       = report.directiveRId.debugString
           // TODO: do we need to store revId for directive (and rule and etc) or can we find it back from other part ?
           // for now, only head
           val (d,tn,tv) = directives.allDirectives.get(report.directiveRId) match {
-                            case Some((at, d)) => (d.name, at.techniqueName.value, d.techniqueVersion.show)
+                            case Some((at, d)) => (d.name, at.techniqueName.value, d.techniqueVersion.debugString)
                             case _ => ("Unknown directive", "Unknown technique id", "N/A")
                           }
           val c         = report.component

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/batch/CheckPolicyTemplateLibrary.scala

@@ -47,6 +47,7 @@ import com.normation.utils.StringUuidGenerator
 import com.normation.eventlog.ModificationId
 import com.normation.rudder.domain.logger.ScheduledJobLogger
 import net.liftweb.actor.SpecializedLiftActor
+import org.joda.time.format.ISODateTimeFormat
 
 final case class StartLibUpdate(actor: EventActor)
 
@@ -108,7 +109,7 @@ class CheckTechniqueLibrary(
         } // don't need an else part : we have to do nothing and the else return Unit
 
         logger.trace("***** Start a new update")
-        policyPackageUpdater.update(ModificationId(uuidGen.newUuid), actor, Some(s"Automatic batch update at ${DateTime.now}"))
+        policyPackageUpdater.update(ModificationId(uuidGen.newUuid), actor, Some(s"Automatic batch update at ${DateTime.now.toString(ISODateTimeFormat.basicDateTime())}"))
 
     }
   }

webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/batch/UpdateDynamicGroups.scala

@@ -48,7 +48,7 @@ import com.normation.eventlog.ModificationId
 import com.normation.utils.StringUuidGenerator
 import com.normation.inventory.domain.NodeId
 import com.normation.rudder.domain.logger.ScheduledJobLogger
-import org.joda.time.format.ISODateTimeFormat
+import com.normation.utils.Utils.DateToIsoString
 
 //Message to send to the updater manager to start a new update of all dynamic groups or get results
 sealed trait GroupUpdateMessage
@@ -166,7 +166,7 @@ class UpdateDynamicGroups(
         }
       } else {
         avoidedUpdate = avoidedUpdate + 1
-        logger.debug(s"No changes that can lead to a dynamic group update happened since ${lastUpdateTime.toString(ISODateTimeFormat.dateTime())} (total ${avoidedUpdate} times avoided)")
+        logger.debug(s"No changes that can lead to a dynamic group update happened since ${lastUpdateTime.toIsoStringNoMillis} (total ${avoidedUpdate} times avoided)")
       }
     }
 
@@ -223,8 +223,7 @@ class UpdateDynamicGroups(
         }
 
         //log some information
-        val format = ISODateTimeFormat.dateTimeNoMillis()
-        logger.debug(s"Dynamic group update in ${new Duration(end.getMillis - start.getMillis).toPeriod().toString} (started at ${start.toString(format)}, ended at ${end.toString(format)})")
+        logger.debug(s"Dynamic group update in ${new Duration(end.getMillis - start.getMillis).toPeriod().toString} (started at ${start.toIsoStringNoMillis}, ended at ${end.toIsoStringNoMillis})")
 
         for {
           (id,boxRes) <- results