Fixes #19170: Binary compatibility still broken by query

Normation · Apr 19, 2021 · e0a8d85 · e0a8d85
1 parent 7a87032
commit e0a8d85
Show file tree

Hide file tree

Showing 26 changed files with 146 additions and 124 deletions.
diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/RudderDit.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/RudderDit.scala
@@ -39,7 +39,6 @@ package com.normation.rudder.domain
 
 import com.normation.rudder.domain.policies._
 import com.normation.rudder.domain.archives._
-import com.normation.rudder.domain.queries.Query
 import com.unboundid.ldap.sdk._
 import com.normation.ldap.sdk._
 import com.normation.inventory.ldap.core._
@@ -55,6 +54,7 @@ import com.normation.rudder.domain.nodes.NodeGroupCategoryId
 import com.normation.rudder.api.ApiAccountId
 import com.normation.rudder.domain.appconfig.RudderWebPropertyName
 import com.normation.ldap.sdk.syntax._
+import com.normation.rudder.domain.queries.QueryTrait
 import com.normation.rudder.rule.category.RuleCategoryId
 
 
@@ -370,7 +370,7 @@ class RudderDit(val BASE_DN:DN) extends AbstractDit {
     /**
      * Create a new group entry
      */
-    def groupModel(uuid:String, parentDN:DN, name:String, description : String, query: Option[Query], isDynamic : Boolean, srvList : Set[NodeId], isEnabled : Boolean, isSystem : Boolean) : LDAPEntry = {
+    def groupModel(uuid:String, parentDN:DN, name:String, description : String, query: Option[QueryTrait], isDynamic : Boolean, srvList : Set[NodeId], isEnabled : Boolean, isSystem : Boolean) : LDAPEntry = {
       val mod = LDAPEntry(group.groupDN(uuid, parentDN))
       mod.resetValuesTo(A_OC, OC.objectClassNames(OC_RUDDER_NODE_GROUP).toSeq:_*)
       mod.resetValuesTo(A_NAME, name)

diff --git a/...urces/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/nodes/NodeGroup.scala b/...urces/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/nodes/NodeGroup.scala
@@ -40,7 +40,7 @@ package com.normation.rudder.domain.nodes
 import com.normation.inventory.domain.NodeId
 import com.normation.rudder.domain.queries.And
 import com.normation.rudder.domain.queries.CriterionLine
-import com.normation.rudder.domain.queries.Query
+import com.normation.rudder.domain.queries.QueryTrait
 import com.normation.rudder.domain.queries.SubGroupComparator
 
 /**
@@ -70,7 +70,7 @@ object NodeGroup {
   /*
    * Check the query part of subgroupiness.
    */
-  def isSubqueryQuery(subgroup: Query, group: Query, groupId: NodeGroupId): Boolean = {
+  def isSubqueryQuery(subgroup: QueryTrait, group: QueryTrait, groupId: NodeGroupId): Boolean = {
     // you need to refined query to be a subgroup
     (subgroup.composition == And) &&
     // not sure about that one: it seems that they should have the same type,
@@ -106,7 +106,7 @@ final case class NodeGroup(
   , name       : String
   , description: String
   , properties : List[GroupProperty]
-  , query      : Option[Query]
+  , query      : Option[QueryTrait]
   , isDynamic  : Boolean = true
   , serverList : Set[NodeId]
   , _isEnabled : Boolean

diff --git a/...s/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/nodes/NodeGroupDiff.scala b/...s/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/nodes/NodeGroupDiff.scala
@@ -39,8 +39,8 @@ package com.normation.rudder.domain.nodes
 
 import com.normation.inventory.domain.NodeId
 import com.normation.rudder.domain.policies.SimpleDiff
-import com.normation.rudder.domain.queries.Query
 import com.normation.rudder.domain.policies.TriggerDeploymentDiff
+import com.normation.rudder.domain.queries.QueryTrait
 
 /**
  * That file defines "diff" objects for NodeGroups (groups, etc).
@@ -71,7 +71,7 @@ final case class ModifyNodeGroupDiff(
   , modName       : Option[SimpleDiff[String]]              = None
   , modDescription: Option[SimpleDiff[String]]              = None
   , modProperties : Option[SimpleDiff[List[GroupProperty]]] = None
-  , modQuery      : Option[SimpleDiff[Option[Query]]]       = None
+  , modQuery      : Option[SimpleDiff[Option[QueryTrait]]]       = None
   , modIsDynamic  : Option[SimpleDiff[Boolean]]             = None
   , modNodeList   : Option[SimpleDiff[Set[NodeId]]]         = None
   , modIsActivated: Option[SimpleDiff[Boolean]]             = None

diff --git a/...ces/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala b/...ces/rudder/rudder-core/src/main/scala/com/normation/rudder/domain/queries/CmdbQuery.scala
@@ -947,22 +947,16 @@ object ResultTransformation {
   }
 }
 
-// This object and the apply method are necessary to allow backward compatibilty in 6.2.5 for plugin scale-out relay due to changes to apply method in https://issues.rudder.io/issues/19138
-// This can be removed in 7.0
-object Query{
-  def apply (
-    returnType : QueryReturnType  //"node" or "node and policy servers"
-    , composition: CriterionComposition
-    , criteria   : List[CriterionLine] //list of all criteria to be matched by returned values
-  ) : Query = Query(returnType, composition, ResultTransformation.Identity, criteria)
-}
 
-final case class Query(
-    returnType : QueryReturnType  //"node" or "node and policy servers"
-  , composition: CriterionComposition
-  , transform  : ResultTransformation
-  , criteria   : List[CriterionLine] //list of all criteria to be matched by returned values
-) {
+// This trait and the apply method are necessary to allow backward compatibilty in 6.2.5 for plugin scale-out relay due to changes to apply method in https://issues.rudder.io/issues/19138
+// This can be removed in 7.0, going back to a case class Query with 4 fields
+// The issue was that we added a new field to a case class and this breaks binary compatibility
+sealed trait QueryTrait {
+  def  returnType : QueryReturnType  //"node" or "node and policy servers"
+  def  composition: CriterionComposition
+  def  criteria   : List[CriterionLine] //list of all criteria to be matched by returned values
+  def  transform  : ResultTransformation
+
   override def toString() = s"{ returnType:'${returnType.value}' (${transform.value}) with '${composition.toString}' criteria [${
       criteria.map{x => s"${x.objectType.objectType}.${x.attribute.name} ${x.comparator.id} ${x.value}" }.mkString(" ; ")
     }] }"
@@ -996,12 +990,12 @@ final case class Query(
 
     override def equals(other:Any) : Boolean = {
       other match {
-        case Query(rt,comp,t,crit) => //criteria order does not matter
-          this.returnType == rt &&
-          this.composition == comp &&
-          this.transform == t &&
-          this.criteria.size == crit.size &&
-          this.criteria.forall(c1 => crit.exists(c2 => c1 == c2))
+        case a :  QueryTrait => //criteria order does not matter
+          this.returnType == a.returnType &&
+          this.composition == a.composition &&
+          this.transform == a.transform &&
+          this.criteria.size == a.criteria.size &&
+          this.criteria.forall(c1 => a.criteria.exists(c2 => c1 == c2))
           //we don't care if the cardinal of equals criterion is not the same on the two,
           //ie [c1,c2,c1] == [c2,c2,c1] yields true
         case _ => false
@@ -1010,3 +1004,27 @@ final case class Query(
 
     override def hashCode() = returnType.hashCode * 17 + composition.hashCode * 3 + transform.hashCode * 11 + criteria.toSet.hashCode * 7
 }
+
+final case class Query(
+    returnType : QueryReturnType  //"node" or "node and policy servers"
+  , composition: CriterionComposition
+  , criteria   : List[CriterionLine] //list of all criteria to be matched by returned values
+) extends QueryTrait {
+  override  def transform : ResultTransformation = ResultTransformation.Identity
+}
+
+object  Query {
+  def apply (
+      returnType : QueryReturnType  //"node" or "node and policy servers"
+    , composition: CriterionComposition
+    , transform  : ResultTransformation
+    , criteria   : List[CriterionLine] //list of all criteria to be matched by returned values
+  ) : NewQuery = NewQuery(returnType, composition, transform, criteria)
+}
+
+final case class NewQuery(
+    returnType : QueryReturnType  //"node" or "node and policy servers"
+  , composition: CriterionComposition
+  , transform  : ResultTransformation
+  , criteria   : List[CriterionLine] //list of all criteria to be matched by returned values
+) extends QueryTrait
diff --git a/...r-core/src/main/scala/com/normation/rudder/services/eventlog/EventLogDetailsService.scala b/...r-core/src/main/scala/com/normation/rudder/services/eventlog/EventLogDetailsService.scala
@@ -53,7 +53,6 @@ import com.normation.rudder.api._
 import com.normation.rudder.batch.CurrentDeploymentStatus
 import com.normation.rudder.domain.policies._
 import com.normation.rudder.domain.nodes._
-import com.normation.rudder.domain.queries.Query
 import com.normation.rudder.domain.eventlog._
 import com.normation.rudder.domain.workflows._
 import com.normation.rudder.domain.parameters._
@@ -74,6 +73,7 @@ import com.normation.inventory.domain.Certificate
 import com.normation.inventory.domain.KeyStatus
 import com.normation.inventory.domain.PublicKey
 import com.normation.inventory.domain.SecurityToken
+import com.normation.rudder.domain.queries.QueryTrait
 import com.typesafe.config.ConfigValue
 
 /**
@@ -450,7 +450,7 @@ class EventLogDetailsServiceImpl(
       displayName     <- (group \ "displayName").headOption.map( _.text ) ?~! ("Missing attribute 'displayName' in entry type nodeGroup : " + entry)
       name            <- getFromToString((group \ "name").headOption)
       description     <- getFromToString((group \ "description").headOption)
-      query           <- getFromTo[Option[Query]]((group \ "query").headOption, {s =>
+      query           <- getFromTo[Option[QueryTrait]]((group \ "query").headOption, {s =>
                           //check for <from><none></none></from> or the same with <to>, <none/>, etc
                           if( (s \ "none").isEmpty) cmdbQueryParser(s.text).map( Some(_) )
                           else Full(None)

diff --git a/...r/rudder-core/src/main/scala/com/normation/rudder/services/eventlog/EventLogFactory.scala b/...r/rudder-core/src/main/scala/com/normation/rudder/services/eventlog/EventLogFactory.scala
@@ -51,7 +51,7 @@ import com.normation.rudder.domain.eventlog._
 import com.normation.rudder.domain.nodes._
 import com.normation.rudder.domain.parameters._
 import com.normation.rudder.domain.policies._
-import com.normation.rudder.domain.queries.Query
+import com.normation.rudder.domain.queries.QueryTrait
 import com.normation.rudder.domain.workflows.WorkflowStepChange
 import com.normation.rudder.services.marshalling._
 import net.liftweb.util.Helpers._
@@ -551,7 +551,7 @@ class EventLogFactoryImpl(
         <displayName>{modifyDiff.name}</displayName>{
           modifyDiff.modName.map(x => SimpleDiff.stringToXml(<name/>, x) ) ++
           modifyDiff.modDescription.map(x => SimpleDiff.stringToXml(<description/>, x ) ) ++
-          modifyDiff.modQuery.map(x => SimpleDiff.toXml[Option[Query]](<query/>, x){ t =>
+          modifyDiff.modQuery.map(x => SimpleDiff.toXml[Option[QueryTrait]](<query/>, x){ t =>
             t match {
               case None => <none/>
               case Some(y) => Text(y.toJSONString)

diff --git a/...er/rudder-core/src/main/scala/com/normation/rudder/services/queries/CmdbQueryParser.scala b/...er/rudder-core/src/main/scala/com/normation/rudder/services/queries/CmdbQueryParser.scala
@@ -80,11 +80,11 @@ trait QueryLexer {
 }
 
 trait StringQueryParser {
-  def parse(query:StringQuery) : Box[Query]
+  def parse(query:StringQuery) : Box[QueryTrait]
 }
 
 trait CmdbQueryParser extends StringQueryParser with QueryLexer {
-  def apply(query:String) : Box[Query] = for {
+  def apply(query:String) : Box[QueryTrait] = for {
     sq <- lex(query)
     q <- parse(sq)
   } yield q
@@ -100,7 +100,7 @@ trait DefaultStringQueryParser extends StringQueryParser {
 
   def criterionObjects : Map[String,ObjectCriterion]
 
-  override def parse(query:StringQuery) : Box[Query] = {
+  override def parse(query:StringQuery) : Box[QueryTrait] = {
 
     for {
       comp  <- query.composition match {
@@ -116,7 +116,7 @@ trait DefaultStringQueryParser extends StringQueryParser {
              }
       lines <- sequence(query.criteria)(parseLine)
     } yield {
-      Query(query.returnType, comp, trans, lines.toList)
+      NewQuery(query.returnType, comp, trans, lines.toList)
     }
   }
 

diff --git a/...er/rudder-core/src/main/scala/com/normation/rudder/services/queries/DynGroupService.scala b/...er/rudder-core/src/main/scala/com/normation/rudder/services/queries/DynGroupService.scala
@@ -48,7 +48,6 @@ import com.normation.rudder.domain.nodes.NodeGroup
 import com.normation.rudder.domain.nodes.NodeGroupId
 import com.normation.rudder.domain.queries.CriterionLine
 import com.normation.rudder.domain.queries.Equals
-import com.normation.rudder.domain.queries.Query
 import com.normation.rudder.domain.RudderDit
 import com.normation.rudder.repository.ldap.LDAPEntityMapper
 import net.liftweb.common._
@@ -67,6 +66,9 @@ import zio.syntax._
 import com.normation.errors._
 import com.normation.rudder.domain.logger.ScheduledJobLoggerPure
 import com.normation.rudder.domain.logger.TimingDebugLoggerPure
+import com.normation.rudder.domain.queries.NewQuery
+import com.normation.rudder.domain.queries.Query
+import com.normation.rudder.domain.queries.QueryTrait
 
 /**
  * A service used to manage dynamic groups : find
@@ -182,7 +184,7 @@ class DynGroupServiceImpl(
 
 object CheckPendingNodeInDynGroups {
 
-  final case class DynGroup(id: NodeGroupId, dependencies: Set[NodeGroupId], testNodes: Set[NodeId], query: Query, includeNodes: Set[NodeId])
+  final case class DynGroup(id: NodeGroupId, dependencies: Set[NodeGroupId], testNodes: Set[NodeId], query: QueryTrait, includeNodes: Set[NodeId])
 
   // for debuging message
   implicit class DynGroupsToString(val gs: List[(NodeGroupId, Set[NodeId])]) extends AnyVal {
@@ -342,7 +344,7 @@ class CheckPendingNodeInDynGroups(
               (g, next :: q)
             }
           }
-          DynGroup(g.id, dep, nodeIds, query.copy(criteria = criteria), Set())
+          DynGroup(g.id, dep, nodeIds, query match {case q : Query => q.copy(criteria = criteria); case q : NewQuery => q.copy(criteria = criteria)}, Set())
         }
       } else {
         None

diff --git a/...rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala b/...rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala
@@ -165,7 +165,7 @@ class AcceptedNodesLDAPQueryProcessor(
    * @return
    */
   private[this] def queryAndChekNodeId(
-      query:Query,
+      query:QueryTrait,
       select:Seq[String],
       limitToNodeIds:Option[Seq[NodeId]]
   ) : Box[Seq[QueryResult]] = {
@@ -213,7 +213,7 @@ class AcceptedNodesLDAPQueryProcessor(
     }
   }
 
-  override def process(query:Query) : Box[Seq[NodeInfo]] = {
+  override def process(query:QueryTrait) : Box[Seq[NodeInfo]] = {
 
     //only keep the one of the form Full(...)
     queryAndChekNodeId(query, NodeInfoService.nodeInfoAttributes, None).map { seq => seq.flatMap {
@@ -227,7 +227,7 @@ class AcceptedNodesLDAPQueryProcessor(
     } }
   }
 
-  override def processOnlyId(query:Query) : Box[Seq[NodeId]] = {
+  override def processOnlyId(query:QueryTrait) : Box[Seq[NodeId]] = {
     //only keep the one of the form Full(...)
     queryAndChekNodeId(query, Seq(A_NODE_UUID), None).map { seq => seq.flatMap {
       case QueryResult(nodeEntry, _ , _) =>
@@ -250,7 +250,7 @@ class PendingNodesLDAPQueryChecker(
     val checker:InternalLDAPQueryProcessor
 ) extends QueryChecker {
 
-  override def check(query:Query, limitToNodeIds:Seq[NodeId]) : Box[Seq[NodeId]] = {
+  override def check(query:QueryTrait, limitToNodeIds:Seq[NodeId]) : Box[Seq[NodeId]] = {
     if(query.criteria.isEmpty) {
       LoggerFactory.getILoggerFactory.getLogger(Logger.loggerNameFor(classOf[InternalLDAPQueryProcessor])).debug(
         s"Checking a query with 0 criterium will always lead to 0 nodes: ${query}"
@@ -306,7 +306,7 @@ class InternalLDAPQueryProcessor(
    * implement process&check method
    */
   def internalQueryProcessor(
-      query         : Query
+      query         : QueryTrait
     , select        : Seq[String] = Seq()
     , limitToNodeIds: Option[Seq[NodeId]] = None
     , debugId       : Long = 0L
@@ -832,7 +832,7 @@ class InternalLDAPQueryProcessor(
    * After that point, we only know how to handle NODES, but
    * perhaps
    */
-  private def normalize(query:Query) : PureResult[LDAPNodeQuery] = {
+  private def normalize(query:QueryTrait) : PureResult[LDAPNodeQuery] = {
     sealed trait QueryFilter
     object QueryFilter {
       final case class Ldap    (dnType: DnType, objectType: String, filter: ExtendedFilter)                   extends QueryFilter
@@ -868,7 +868,7 @@ class InternalLDAPQueryProcessor(
       }
     }
 
-    def checkAndSplitFilterType(q: Query): PureResult[Seq[QueryFilter]] = {
+    def checkAndSplitFilterType(q: QueryTrait): PureResult[Seq[QueryFilter]] = {
       // Compute in one go all data for a filter, fails if one filter fails to build
       q.criteria.traverse { case crit@CriterionLine(ot, a, comp, value) =>
         // objectType may be overriden in the attribute (for node state).

diff --git a/...der/rudder-core/src/main/scala/com/normation/rudder/services/queries/QueryProcessor.scala b/...der/rudder-core/src/main/scala/com/normation/rudder/services/queries/QueryProcessor.scala
@@ -39,7 +39,7 @@ package com.normation.rudder.services.queries
 
 import com.normation.inventory.domain.NodeId
 import com.normation.rudder.domain.nodes.NodeInfo
-import com.normation.rudder.domain.queries.Query
+import com.normation.rudder.domain.queries.QueryTrait
 import net.liftweb.common.Box
 
 trait QueryProcessor {
@@ -50,13 +50,13 @@ trait QueryProcessor {
    * @param select - attributes to fetch in the ldap entry. If empty, all attributes are fetched
    * @return
    */
-  def process(query:Query) : Box[Seq[NodeInfo]]
+  def process(query:QueryTrait) : Box[Seq[NodeInfo]]
 
   /**
    * Only get node ids corresponding to that request, with minimal consistency check.
    * This method is useful to maximize performance (low memory, high throughout) for ex for dynamic groups.
    */
-  def processOnlyId(query:Query) : Box[Seq[NodeId]]
+  def processOnlyId(query:QueryTrait) : Box[Seq[NodeId]]
 }
 
 
@@ -74,6 +74,6 @@ trait QueryChecker {
    *   Full(seq) with seq being the list of nodeId which verify
    *   query.
    */
-  def check(query:Query, nodeIds:Seq[NodeId]) : Box[Seq[NodeId]]
+  def check(query:QueryTrait, nodeIds:Seq[NodeId]) : Box[Seq[NodeId]]
 
 }
diff --git a/...der/rudder-core/src/main/scala/com/normation/rudder/services/servers/NewNodeManager.scala b/...der/rudder-core/src/main/scala/com/normation/rudder/services/servers/NewNodeManager.scala
@@ -68,6 +68,7 @@ import com.normation.rudder.domain.policies.PolicyMode
 import com.normation.rudder.domain.queries.CriterionLine
 import com.normation.rudder.domain.queries.DitQueryData
 import com.normation.rudder.domain.queries.Equals
+import com.normation.rudder.domain.queries.NewQuery
 import com.normation.rudder.domain.queries.NodeReturnType
 import com.normation.rudder.domain.queries.Or
 import com.normation.rudder.domain.queries.Query
@@ -831,7 +832,7 @@ class AcceptHostnameAndIp(
     }
 
     for {
-      duplicatesH    <- queryProcessor.process(Query(NodeReturnType, Or, ResultTransformation.Identity, hostnameCriterion)).map { nodesInfo =>
+      duplicatesH    <- queryProcessor.process(NewQuery(NodeReturnType, Or, ResultTransformation.Identity, hostnameCriterion)).map { nodesInfo =>
                           //here, all nodes found are duplicate-in-being. They should be unique, but
                           //if not, we will don't group them that the duplicate appears in the list
                           nodesInfo.map( ni => ni.hostname)