Fixes #3887: Correctly handle machine inventory not in the same branch than the node one

rudder-core/src/main/scala/com/normation/rudder/services/nodes/NodeInfoService.scala

@@ -43,7 +43,6 @@ import com.normation.rudder.domain.NodeDit
 import net.liftweb.common._
 import net.liftweb.util.Helpers._
 import com.normation.rudder.domain.nodes.NodeInfo
-
 import com.normation.rudder.domain.RudderLDAPConstants._
 import com.normation.inventory.ldap.core.LDAPConstants._
 import com.normation.rudder.domain.Constants._
@@ -52,9 +51,27 @@ import com.normation.ldap.sdk._
 import BuildFilter._
 import com.normation.rudder.repository.ldap.LDAPEntityMapper
 import com.normation.utils.Control._
+import com.normation.inventory.ldap.core.InventoryMapper
+import com.normation.inventory.ldap.core.InventoryDitService
+
+/**
+ * A case class used to represent the minimal
+ * information needed to get a NodeInfo
+ */
+case class LDAPNodeInfo(
+    nodeEntry            : LDAPEntry
+  , nodeInventoryEntry   : LDAPEntry
+  , machineInventoryEntry: Option[LDAPEntry]
+)
+
 
 trait NodeInfoService {
 
+  /**
+   * Retrieve minimal information needed for the node info
+   */
+  def getLDAPNodeInfo(nodeId: NodeId) : Box[LDAPNodeInfo]
+
   /**
    * Return a NodeInfo from a NodeId. First check the ou=Node, then fetch the other data
    * @param nodeId
@@ -99,31 +116,47 @@ class NodeInfoServiceImpl(
     rudderDit:RudderDit,
     inventoryDit:InventoryDit,
     ldap:LDAPConnectionProvider[RoLDAPConnection],
-    ldapMapper:LDAPEntityMapper
+    ldapMapper:LDAPEntityMapper,
+    inventoryMapper:InventoryMapper,
+    inventoryDitService:InventoryDitService
 ) extends NodeInfoService with Loggable {
   import NodeInfoServiceImpl._
 
-  def getNodeInfo(nodeId: NodeId) : Box[NodeInfo] = {
+  def getLDAPNodeInfo(nodeId: NodeId) : Box[LDAPNodeInfo] = {
     logger.trace("Fetching node info for node id %s".format(nodeId.value))
     for {
       con <- ldap
       node <- con.get(nodeDit.NODES.NODE.dn(nodeId.value), nodeInfoAttributes:_*) ?~! "Node with ID '%s' was not found".format(nodeId.value)
-      nodeInfo <- for {
-                    server <- con.get(inventoryDit.NODES.NODE.dn(nodeId.value), nodeInfoAttributes:_*)  ?~! "Node info with ID '%s' was not found".format(nodeId.value)
-                    machine <- server(A_CONTAINER_DN) match {
-                      case Some(container) => inventoryDit.MACHINES.MACHINE.idFromDN(new DN(container)) match {
-                        case Full(machineId) =>  Full(con.get(inventoryDit.MACHINES.MACHINE.dn(machineId),Seq("*"):_*).toOption)
-                        case eb:EmptyBox => val msg = s"could not get machine from container ${container} : ${eb}"
-                          logger.error(msg)
-                          Failure(msg)
-                      }
-                      case None => logger.debug(s"no machine Inventory for ${server}")
-                        Full(None)
-                    }
-                    nodeInfo <- ldapMapper.convertEntriesToNodeInfos(node, server,machine)
-                  } yield {
-                    nodeInfo
-                  }
+      nodeInventory  <- con.get(inventoryDit.NODES.NODE.dn(nodeId.value), nodeInfoAttributes:_*)  ?~! "Node info with ID '%s' was not found".format(nodeId.value)
+      machineInventory <- {
+                            val machines = inventoryMapper.mapSeqStringToMachineIdAndStatus(nodeInventory.valuesFor(A_CONTAINER_DN))
+                            if(machines.size == 0) Full(None)
+                            else { //here, we only process the first machine
+                              if(logger.isDebugEnabled && machines.size > 1) {
+                                logger.debug("Node with id %s is attached to several container. Taking %s as the valid one, ignoring other (%s)".format(
+                                    nodeId.value, machines(0)._1.value, machines.tail.map( _._1).mkString(", "))
+                                )
+                              }
+                              val (machineId, status) = machines(0)
+                              val dit = inventoryDitService.getDit(status)
+
+                              Full(con.get(dit.MACHINES.MACHINE.dn(machineId),Seq("*"):_*).toOption)
+                            }
+                          }
+    } yield {
+      LDAPNodeInfo(node, nodeInventory, machineInventory)
+    }
+  }
+
+
+  def getNodeInfo(nodeId: NodeId) : Box[NodeInfo] = {
+    for {
+      ldapEntries <- getLDAPNodeInfo(nodeId)
+      nodeInfo    <- ldapMapper.convertEntriesToNodeInfos(
+                         ldapEntries.nodeEntry
+                       , ldapEntries.nodeInventoryEntry
+                       , ldapEntries.machineInventoryEntry
+                     )
     } yield {
       nodeInfo
     }

rudder-core/src/main/scala/com/normation/rudder/services/queries/LdapQueryProcessor.scala

@@ -52,6 +52,8 @@ import com.normation.rudder.domain.RudderLDAPConstants._
 import java.util.regex.Pattern
 import com.normation.utils.HashcodeCaching
 import net.liftweb.util.Helpers
+import com.normation.rudder.services.nodes.NodeInfoService
+import com.normation.rudder.services.nodes.LDAPNodeInfo
 
 
 
@@ -116,7 +118,8 @@ object DefaultRequestLimits extends RequestLimits(10,1000,10,1000)
 class AccepetedNodesLDAPQueryProcessor(
     nodeDit:NodeDit,
     inventoryDit:InventoryDit,
-    processor:InternalLDAPQueryProcessor
+    processor:InternalLDAPQueryProcessor,
+    nodeInfoService: NodeInfoService
 ) extends QueryProcessor with Loggable {
 
 
@@ -147,22 +150,9 @@ class AccepetedNodesLDAPQueryProcessor(
       val inNodes = (for {
         inventoryEntry <- inventoryEntries
         rdn <- inventoryEntry(A_NODE_UUID)
-        con <- processor.ldap
-
-        machine <- inventoryEntry(A_CONTAINER_DN) match {
-          case Some(container) => inventoryDit.MACHINES.MACHINE.idFromDN(new DN(container)) match {
-            case Full(machineId) =>  Full(con.get(inventoryDit.MACHINES.MACHINE.dn(machineId),Seq("*"):_*).toOption)
-            case eb:EmptyBox => val msg = s"could not get machine from container ${container} : ${eb}"
-              logger.error(msg)
-              Failure(msg)
-            }
-          case None => logger.debug(s"no machine Inventory for ${inventoryEntry}")
-            Full(None)
-        }
-
-        nodeEntry <- con.get(nodeDit.NODES.NODE.dn(rdn), Seq(SearchRequest.ALL_USER_ATTRIBUTES, A_OBJECT_CREATION_DATE):_*)
+        LDAPNodeInfo(nodeEntry, nodeInv, machineInv) <- nodeInfoService.getLDAPNodeInfo(NodeId(rdn))
       } yield {
-        QueryResult(nodeEntry,inventoryEntry,machine)
+        QueryResult(nodeEntry,nodeInv, machineInv)
       })
 
       if(logger.isDebugEnabled) {

rudder-core/src/test/scala/com/normation/rudder/services/nodes/DummyNodeInfoService.scala

@@ -78,4 +78,6 @@ class DummyNodeInfoService extends NodeInfoService {
   def getAllSystemNodeIds() : Box[Seq[NodeId]] = {
     Full(allNodes.filter(_.isPolicyServer).map(_.id))
   }
+
+  def getLDAPNodeInfo(nodeId:NodeId) : Box[LDAPNodeInfo] = ???
 }

rudder-core/src/test/scala/com/normation/rudder/services/queries/TestQueryProcessor.scala

@@ -50,6 +50,7 @@ import com.normation.ldap.sdk._
 import com.normation.inventory.ldap.core._
 import com.normation.inventory.domain.NodeId
 import com.normation.utils.HashcodeCaching
+import com.normation.rudder.services.nodes.NodeInfoServiceImpl
 
 /*
  * Test query parsing.
@@ -88,17 +89,25 @@ class TestQueryProcessor extends Loggable {
 
   val DIT = new InventoryDit(new DN("ou=Accepted Inventories,ou=Inventories,cn=rudder-configuration"),new DN("ou=Inventories,cn=rudder-configuration"),"test")
 
+  val removedDIT = new InventoryDit(new DN("ou=Removed Inventories,ou=Inventories,cn=rudder-configuration"),new DN("ou=Inventories,cn=rudder-configuration"),"test")
+  val pendingDIT = new InventoryDit(new DN("ou=Pending Inventories,ou=Inventories,cn=rudder-configuration"),new DN("ou=Inventories,cn=rudder-configuration"),"test")
+  val ditService = new InventoryDitServiceImpl(pendingDIT, DIT, removedDIT)
   val nodeDit = new NodeDit(new DN("cn=rudder-configuration"))
+  val rudderDit = new RudderDit(new DN("cn=rudder-configuration"))
 
   val ditQueryData = new DitQueryData(DIT)
 
-  val ldapMapper = new LDAPEntityMapper(null, nodeDit, DIT, null)
+  val ldapMapper = new LDAPEntityMapper(rudderDit, nodeDit, DIT, null)
+  val inventoryMapper = new InventoryMapper(ditService, pendingDIT, DIT, removedDIT)
   val internalLDAPQueryProcessor = new InternalLDAPQueryProcessor(ldap,DIT,ditQueryData,ldapMapper)
 
+  val nodeInfoService = new NodeInfoServiceImpl(nodeDit, rudderDit, DIT, ldap, ldapMapper, inventoryMapper, ditService)
+
   val queryProcessor = new AccepetedNodesLDAPQueryProcessor(
       nodeDit,
       DIT,
-      internalLDAPQueryProcessor
+      internalLDAPQueryProcessor,
+      nodeInfoService
   )
 
   val parser = new CmdbQueryParser with

rudder-web/src/main/scala/bootstrap/liftweb/AppConfig.scala

@@ -588,7 +588,9 @@ object RudderConfig extends Loggable {
   private[this] lazy val queryProcessor = new AccepetedNodesLDAPQueryProcessor(
     nodeDitImpl,
     acceptedNodesDitImpl,
-    new InternalLDAPQueryProcessor(roLdap, acceptedNodesDitImpl, ditQueryDataImpl, ldapEntityMapper))
+    new InternalLDAPQueryProcessor(roLdap, acceptedNodesDitImpl, ditQueryDataImpl, ldapEntityMapper),
+    nodeInfoServiceImpl
+  )
 
   //we need a roLdap query checker for nodes in pending
   private[this] lazy val inventoryQueryChecker = new PendingNodesLDAPQueryChecker(new InternalLDAPQueryProcessor(roLdap, pendingNodesDitImpl, new DitQueryData(pendingNodesDitImpl), ldapEntityMapper))
@@ -936,7 +938,15 @@ object RudderConfig extends Loggable {
    , groupLibReadWriteMutex
   )
   private[this] lazy val eventLogDeploymentServiceImpl = new EventLogDeploymentService(logRepository, eventLogDetailsServiceImpl)
-  private[this] lazy val nodeInfoServiceImpl: NodeInfoService = new NodeInfoServiceImpl(nodeDitImpl, rudderDitImpl, acceptedNodesDitImpl, roLdap, ldapEntityMapper)
+  private[this] lazy val nodeInfoServiceImpl: NodeInfoService = new NodeInfoServiceImpl(
+      nodeDitImpl
+    , rudderDitImpl
+    , acceptedNodesDitImpl
+    , roLdap
+    , ldapEntityMapper
+    , inventoryMapper
+    , inventoryDitService
+  )
   private[this] lazy val dependencyAndDeletionServiceImpl: DependencyAndDeletionService = new DependencyAndDeletionServiceImpl(
         roLdap
       , rudderDitImpl