Fixed issues ingesting model files

icd-db/src/main/scala/csw/services/icd/db/IcdDb.scala

@@ -1,7 +1,6 @@
 package csw.services.icd.db
 
 import java.io.File
-import java.nio.file.Files
 
 import com.mongodb.casbah.Imports._
 import com.typesafe.config.{ Config, ConfigFactory }
@@ -333,25 +332,26 @@ case class IcdDb(
       BaseModelParser(stdConfig.config).subsystem
   }
 
-  /**
-   * Imports a JSON file with ICD release information.
-   * The format of the file is the one generated by the [[IcdVersions]] class.
-   *
-   * @param inputFile a file in HOCON/JSON format matching the JSON schema in icds-schema.conf
-   */
-  def importIcds(inputFile: File): Unit = {
-    val icdVersions = IcdVersions.fromJson(new String(Files.readAllBytes(inputFile.toPath)))
-    importIcds(icdVersions)
-  }
+//  /**
+//   * Imports a JSON file with ICD release information.
+//   * The format of the file is the one generated by the [[IcdVersions]] class.
+//   *
+//   * @param inputFile a file in HOCON/JSON format matching the JSON schema in icds-schema.conf
+//   */
+//  def importIcds(inputFile: File): Unit = {
+//    val icdVersions = IcdVersions.fromJson(new String(Files.readAllBytes(inputFile.toPath)))
+//    importIcds(icdVersions)
+//  }
 
   /**
    * Imports the given ICD release information.
    *
    * @param icdVersions describes the ICD version
    */
-  def importIcds(icdVersions: IcdVersions): Unit = {
+  def importIcds(icdVersions: IcdVersions, feedback: String => Unit): Unit = {
     versionManager.removeIcdVersions(icdVersions.subsystems.head, icdVersions.subsystems(1))
     icdVersions.icds.foreach { icd =>
+      feedback(s"Ingesting ICD ${icdVersions.subsystems.head}-${icdVersions.subsystems(1)}-${icd.icdVersion}")
       versionManager.addIcdVersion(
         icd.icdVersion,
         icdVersions.subsystems.head, icd.versions.head,

icd-db/src/main/scala/csw/services/icd/db/IcdDbPrinter.scala

@@ -818,7 +818,7 @@ case class IcdDbPrinter(db: IcdDb) {
           case _ => println(s"Unsupported output format: Expected *.html or *.pdf")
         }
       case None =>
-        println("Please specify source and optionally target subsystems to print")
+        println(s"Failed to generate $file. You might need to run: 'icd-git --ingest' first to update the database.")
     }
   }
 }

icd-db/src/main/scala/csw/services/icd/db/IcdVersionManager.scala

@@ -415,7 +415,10 @@ case class IcdVersionManager(db: MongoDB, query: IcdDbQuery) {
         val allEntries = getEntries(versionInfo.parts)
         val entries = if (subsystemOnly) allEntries.take(1) else allEntries
         entries.map(Models(versionMap, _))
-      case None => Nil
+      case None =>
+        val v = versionOpt.map("-"+_).getOrElse("")
+        println(s"Error: $subsystem$v not found in the icd database.")
+        Nil
     }
   }
 

icd-git/src/main/scala/csw/services/icd/github/IcdGit.scala

@@ -12,25 +12,26 @@ object IcdGit extends App {
   // Default user name if none given
   private val defaultUser = System.getProperty("user.name")
 
+  // cache of API and ICD versions published on GitHub (to avoid cloning the same repo multiple times)
+  private val (allApiVersions, allIcdVersions) = IcdGitManager.getAllVersions
+
   /**
     * Command line options ("icd-git --help" prints a usage message with descriptions of all the options)
     */
-  private case class Options(
-                              list: Boolean = false,
-                              subsystems: List[SubsystemAndVersion] = Nil,
-                              icdVersion: Option[String] = None,
-                              interactive: Boolean = false,
-                              publish: Boolean = false,
-                              unpublish: Boolean = false,
-                              majorVersion: Boolean = false,
-                              user: Option[String] = None,
-                              password: Option[String] = None,
-                              comment: Option[String] = None,
-                              dbName: String = IcdDbDefaults.defaultDbName,
-                              host: String = IcdDbDefaults.defaultHost,
-                              port: Int = IcdDbDefaults.defaultPort,
-                              ingest: Boolean = false
-                            )
+  private case class Options(list: Boolean = false,
+                             subsystems: List[SubsystemAndVersion] = Nil,
+                             icdVersion: Option[String] = None,
+                             interactive: Boolean = false,
+                             publish: Boolean = false,
+                             unpublish: Boolean = false,
+                             majorVersion: Boolean = false,
+                             user: Option[String] = None,
+                             password: Option[String] = None,
+                             comment: Option[String] = None,
+                             dbName: String = IcdDbDefaults.defaultDbName,
+                             host: String = IcdDbDefaults.defaultHost,
+                             port: Int = IcdDbDefaults.defaultPort,
+                             ingest: Boolean = false)
 
   private def parseSubsystemsArg(s: String): List[SubsystemAndVersion] = {
     s.split(',').toList.map(SubsystemAndVersion(_)).sorted
@@ -139,7 +140,7 @@ object IcdGit extends App {
         if (s == null || s.isEmpty) sv
         else Option(s).map { subsys =>
           if (needsVersion) {
-            val versions = IcdGitManager.getSubsystemVersionNumbers(SubsystemAndVersion(subsys, None))
+            val versions = IcdGitManager.getSubsystemVersionNumbers(SubsystemAndVersion(subsys, None), allApiVersions)
             if (versions.isEmpty) error(s"No published versions of $subsys were found. Please use --publish option.")
             val version = if (versions.size == 1) versions.head
             else {
@@ -217,9 +218,8 @@ object IcdGit extends App {
   // --list option (print API and ICD versions for subsystem and target)
   private def list(options: Options): Unit = {
     if (options.subsystems.size == 2) {
-      // XXX TODO: Get all pairs of subsystems?
       for {
-        icdVersions <- IcdGitManager.list(options.subsystems)
+        icdVersions <- IcdGitManager.list(options.subsystems, allIcdVersions)
       } {
         icdVersions.icds.foreach { icd =>
           val a = s"${icdVersions.subsystems.head}-${icd.versions.head}"
@@ -229,7 +229,7 @@ object IcdGit extends App {
       }
     } else if (options.subsystems.nonEmpty) {
       // list the publish history for each of the given subsystems
-      options.subsystems.map(sv => IcdGitManager.getApiVersions(sv)).foreach {
+      options.subsystems.map(sv => IcdGitManager.getApiVersions(sv, allApiVersions)).foreach {
         _.foreach { a =>
           a.apis.foreach { api =>
             println(s"\nSubsystem ${a.subsystem}-${api.version}: created by ${api.user} on ${api.date}:\n${api.comment}\n")
@@ -315,7 +315,7 @@ object IcdGit extends App {
       case ex: Exception => error(s"Unable to drop the existing ICD database: $ex")
     }
 
-    IcdGitManager.ingest(db, options.subsystems, (s: String) => println(s))
+    IcdGitManager.ingest(db, options.subsystems, (s: String) => println(s), allApiVersions, allIcdVersions)
   }
 }
 

icd-git/src/main/scala/csw/services/icd/github/IcdGitManager.scala

@@ -125,8 +125,8 @@ object IcdGitManager {
   /**
     * Gets a list of version numbers for the given subsystem
     */
-  def getSubsystemVersionNumbers(sv: SubsystemAndVersion): List[String] = {
-    getApiVersions(sv).toList.flatMap(_.apis.map(_.version))
+  def getSubsystemVersionNumbers(sv: SubsystemAndVersion, allApiVersions: List[ApiVersions]): List[String] = {
+    getApiVersions(sv, allApiVersions).toList.flatMap(_.apis.map(_.version))
   }
 
   /**
@@ -138,18 +138,12 @@ object IcdGitManager {
 
   /**
     * Gets a list of information about the published versions of the given subsystem
+    *
+    * @param sv             indicates the subsystem
+    * @param allApiVersions cached list of all API versions (see getAllVersions)
     */
-  def getApiVersions(sv: SubsystemAndVersion): Option[ApiVersions] = {
-    // Checkout the main git repo in a temp dir
-    val gitWorkDir = Files.createTempDirectory("icds").toFile
-    try {
-      // Clone the repository containing the API version info files
-      val git = Git.cloneRepository.setDirectory(gitWorkDir).setURI(gitBaseUri).call
-      git.close()
-      getApiVersions(sv, gitWorkDir)
-    } finally {
-      deleteDirectoryRecursively(gitWorkDir)
-    }
+  def getApiVersions(sv: SubsystemAndVersion, allApiVersions: List[ApiVersions]): Option[ApiVersions] = {
+    allApiVersions.find(_.subsystem == sv.subsystem)
   }
 
   /**
@@ -193,32 +187,13 @@ object IcdGitManager {
     * Returns information about the ICD versions between the given subsystems (The order of
     * subsystems is not important)
     *
-    * @param subsystems the subsystems that make up the ICD
+    * @param subsystems     the subsystems that make up the ICD
+    * @param allIcdVersions cached list of ICD versions (from getAllVersions)
     * @return the ICD version info, if found
     */
-  def list(subsystems: List[SubsystemAndVersion]): Option[IcdVersions] = {
+  def list(subsystems: List[SubsystemAndVersion], allIcdVersions: List[IcdVersions]): Option[IcdVersions] = {
     if (subsystems.size != 2) error("Expected two subsystems that make up an ICD")
-    // sort by convention
-    val sorted = subsystems.sorted
-    val sv = sorted.head
-    val tv = sorted.tail.head
-    // Checkout the icds repo in a temp dir
-    val gitWorkDir = Files.createTempDirectory("icds").toFile
-    try {
-      val git = Git.cloneRepository.setDirectory(gitWorkDir).setURI(gitBaseUri).call
-      git.close()
-      val (file, _) = getIcdFile(sv.subsystem, tv.subsystem, gitWorkDir)
-      val path = Paths.get(file.getPath)
-
-      // Get the list of published ICDs for the subsystem and target from GitHub
-      if (!file.exists()) None
-      else {
-        // XXX TODO: filter on s.versionOpt and t.versionOpt
-        Some(IcdVersions.fromJson(new String(Files.readAllBytes(path))))
-      }
-    } finally {
-      deleteDirectoryRecursively(gitWorkDir)
-    }
+    allIcdVersions.find(i => i.subsystems == subsystems.sorted.map(_.subsystem))
   }
 
   /**
@@ -518,25 +493,27 @@ object IcdGitManager {
   /**
     * Ingests the given subsystems and ICD (or all subsystems and ICDs) into the ICD database.
     *
-    * @param db            the database to use
-    * @param subsystemList list of subsystems to ingest (two subsystems for an ICD, empty for all subsystems and ICDs)
-    * @param feedback      function to display messages while working
+    * @param db             the database to use
+    * @param subsystemList  list of subsystems to ingest (two subsystems for an ICD, empty for all subsystems and ICDs)
+    * @param feedback       function to display messages while working
+    * @param allApiVersions cached API version info (see getAllVersions)
+    * @param allIcdVersions cached ICD version info (see getAllVersions)
     */
-  def ingest(db: IcdDb, subsystemList: List[SubsystemAndVersion], feedback: String => Unit): Unit = {
+  def ingest(db: IcdDb, subsystemList: List[SubsystemAndVersion], feedback: String => Unit,
+             allApiVersions: List[ApiVersions], allIcdVersions: List[IcdVersions]): Unit = {
     // Get the list of subsystems to ingest
     val subsystems = {
       if (subsystemList.nonEmpty) {
         // sort by convention to avoid duplicate ICDs,
         feedback(s"Ingesting ${subsystemList.mkString(" and ")}")
         subsystemList.sorted
       } else {
-        feedback(s"Ingesting all known subsystems")
-        // XXX TODO: call getAllVersions and use the cached results instead of cloning the repo multiple times!
+        feedback(s"Ingesting all published subsystems from $gitBaseUri")
         allSubsystems.map(s => SubsystemAndVersion(s, None)).toList
       }
     }
-    subsystems.foreach(ingest(db, _, feedback))
-    importIcdFiles(db, subsystems, feedback)
+    subsystems.foreach(ingest(db, _, feedback, allApiVersions))
+    importIcdFiles(db, subsystems, feedback, allIcdVersions)
   }
 
   /**
@@ -547,13 +524,15 @@ object IcdGitManager {
     * @param sv       the subsystem and optional version
     * @param feedback optional feedback function
     */
-  def ingest(db: IcdDb, sv: SubsystemAndVersion, feedback: String => Unit): Unit = {
-    getApiVersions(sv) match {
+  def ingest(db: IcdDb, sv: SubsystemAndVersion, feedback: String => Unit, allApiVersions: List[ApiVersions]): Unit = {
+    getApiVersions(sv, allApiVersions) match {
       case Some(versionsFound) =>
         sv.versionOpt.foreach { v =>
           if (!versionsFound.apis.exists(_.version == v)) warning(s"No published version $v of ${sv.subsystem} was found in the repository")
         }
+
         def versionFilter(e: ApiEntry) = if (sv.versionOpt.isEmpty) true else sv.versionOpt.get == e.version
+
         val apiEntries = versionsFound.apis.filter(versionFilter)
         ingest(db, sv, apiEntries, feedback)
 
@@ -577,8 +556,8 @@ object IcdGitManager {
     val gitWorkDir = Files.createTempDirectory("icds").toFile
     try {
       val git = Git.cloneRepository.setDirectory(gitWorkDir).setURI(url).call()
-      apiEntries.foreach { e =>
-        feedback(s"Checking out ${sv.subsystem}-${e.version}")
+      apiEntries.reverse.foreach { e =>
+        feedback(s"Checking out ${sv.subsystem}-${e.version} (commit: ${e.commit})")
         git.checkout().setName(e.commit).call
         feedback(s"Ingesting ${sv.subsystem}-${e.version}")
         db.ingest(gitWorkDir)
@@ -592,28 +571,14 @@ object IcdGitManager {
   }
 
   // Imports the ICD release information for the two subsystems, or all subsystems
-  def importIcdFiles(db: IcdDb, subsystems: List[SubsystemAndVersion], feedback: String => Unit): Unit = {
+  def importIcdFiles(db: IcdDb, subsystems: List[SubsystemAndVersion],
+                     feedback: String => Unit, allIcdVersions: List[IcdVersions]): Unit = {
     val gitWorkDir = Files.createTempDirectory("icds").toFile
-    try {
-      val git = Git.cloneRepository.setDirectory(gitWorkDir).setURI(gitBaseUri).call
-      git.close()
-      // XXX Should look for pairs of subsystems?
-      if (subsystems.size == 2) {
-        // Import one ICD if subsystem and target options were given
-        val subsystem = subsystems.head.subsystem
-        val target = subsystems.tail.head.subsystem
-        val (file, _) = getIcdFile(subsystem, target, gitWorkDir)
-        if (file.exists()) db.importIcds(file)
-      } else {
-        // Import all ICD files
-        val dir = new File(gitWorkDir, gitIcdsDir)
-        if (dir.exists && dir.isDirectory) {
-          val files = dir.listFiles.filter(f => f.isFile && f.getName.endsWith(".json")).toList
-          files.foreach(file => db.importIcds(file))
-        }
-      }
-    } finally {
-      deleteDirectoryRecursively(gitWorkDir)
+    if (subsystems.size == 2) {
+      // Import one ICD if subsystem and target options were given
+      allIcdVersions.find(_.subsystems == subsystems.map(_.subsystem)).foreach(db.importIcds(_, feedback))
+    } else {
+      allIcdVersions.foreach(db.importIcds(_, feedback))
     }
   }
 }

icd-git/src/test/scala/csw/services/icd/github/IcdGitManagerTests.scala

@@ -11,13 +11,13 @@ import org.scalatest.{BeforeAndAfter, FunSuite}
 
 class IcdGitManagerTests extends FunSuite with BeforeAndAfter {
 
-  var repoDir: File = _
-  var git: Git = _
+  private var repoDir: File = _
+  private var git: Git = _
 
-  val user = System.getProperty("user.name")
-  val password = ""
-  val comment = "test comment"
-  val subsysList = List(SubsystemAndVersion("TEST", Some("1.0")), SubsystemAndVersion("TEST2", Some("1.0")))
+  private val user = System.getProperty("user.name")
+  private val password = ""
+  private val comment = "test comment"
+  private val subsysList = List(SubsystemAndVersion("TEST", Some("1.0")), SubsystemAndVersion("TEST2", Some("1.0")))
 
   // Use a dummy repo for the icd and api versions
   before {
@@ -34,8 +34,11 @@ class IcdGitManagerTests extends FunSuite with BeforeAndAfter {
   }
 
   test("Test publishing") {
+    // Note: Normally the return value from IcdGitManager.getAllVersions could be cached and reused for a while,
+    // but not here, since we are modifying the test repository with new versions.
+
     // List should return empty
-    assert(IcdGitManager.list(subsysList).isEmpty)
+    assert(IcdGitManager.list(subsysList, IcdGitManager.getAllVersions._2).isEmpty)
 
     // Publish API for TEST subsystem
     val i1 = IcdGitManager.publish("TEST", majorVersion = false, user, password, comment)
@@ -44,7 +47,7 @@ class IcdGitManagerTests extends FunSuite with BeforeAndAfter {
     assert(i1.user == user)
     assert(i1.subsystem == "TEST")
 
-    val apiVersionOpt = IcdGitManager.getApiVersions(subsysList.head)
+    val apiVersionOpt = IcdGitManager.getApiVersions(subsysList.head, IcdGitManager.getAllVersions._1)
     assert(apiVersionOpt.isDefined)
     val apiVersion = apiVersionOpt.get
     assert(apiVersion.subsystem == "TEST")
@@ -61,7 +64,7 @@ class IcdGitManagerTests extends FunSuite with BeforeAndAfter {
     assert(i2.user == user)
     assert(i2.subsystem == "TEST2")
 
-    val apiVersionOpt2 = IcdGitManager.getApiVersions(subsysList.tail.head)
+    val apiVersionOpt2 = IcdGitManager.getApiVersions(subsysList.tail.head, IcdGitManager.getAllVersions._1)
     assert(apiVersionOpt2.isDefined)
     val apiVersion2 = apiVersionOpt2.get
     assert(apiVersion2.subsystem == "TEST2")
@@ -77,7 +80,7 @@ class IcdGitManagerTests extends FunSuite with BeforeAndAfter {
     assert(i3.comment == comment + " 3")
     assert(i3.user == user)
 
-    val icdVersionsOpt = IcdGitManager.list(subsysList)
+    val icdVersionsOpt = IcdGitManager.list(subsysList, IcdGitManager.getAllVersions._2)
     assert(icdVersionsOpt.isDefined)
     val icdVersions = icdVersionsOpt.get
     assert(icdVersions.subsystems == List("TEST", "TEST2"))
@@ -95,7 +98,8 @@ class IcdGitManagerTests extends FunSuite with BeforeAndAfter {
     } catch {
       case ex: Exception => throw new RuntimeException("Unable to drop the existing ICD database", ex)
     }
-    IcdGitManager.ingest(db, subsysList, (s: String) => println(s))
+    val (apis, icds) = IcdGitManager.getAllVersions
+    IcdGitManager.ingest(db, subsysList, (s: String) => println(s), apis, icds)
     val icdNames = db.versionManager.getIcdNames
     assert(icdNames.size == 1)
     assert(icdNames.head.subsystem == "TEST")