diff --git a/icd-db/src/main/scala/csw/services/icd/db/IcdDb.scala b/icd-db/src/main/scala/csw/services/icd/db/IcdDb.scala index 759ccb81..5bc49d49 100644 --- a/icd-db/src/main/scala/csw/services/icd/db/IcdDb.scala +++ b/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, diff --git a/icd-db/src/main/scala/csw/services/icd/db/IcdDbPrinter.scala b/icd-db/src/main/scala/csw/services/icd/db/IcdDbPrinter.scala index 41349ff3..fee86064 100644 --- a/icd-db/src/main/scala/csw/services/icd/db/IcdDbPrinter.scala +++ b/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.") } } } diff --git a/icd-db/src/main/scala/csw/services/icd/db/IcdVersionManager.scala b/icd-db/src/main/scala/csw/services/icd/db/IcdVersionManager.scala index 6505c85f..8a387d8e 100644 --- a/icd-db/src/main/scala/csw/services/icd/db/IcdVersionManager.scala +++ b/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 } } diff --git a/icd-git/src/main/scala/csw/services/icd/github/IcdGit.scala b/icd-git/src/main/scala/csw/services/icd/github/IcdGit.scala index a2421b21..b3dd74b6 100644 --- a/icd-git/src/main/scala/csw/services/icd/github/IcdGit.scala +++ b/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) } } diff --git a/icd-git/src/main/scala/csw/services/icd/github/IcdGitManager.scala b/icd-git/src/main/scala/csw/services/icd/github/IcdGitManager.scala index 735906ff..039c2aa9 100644 --- a/icd-git/src/main/scala/csw/services/icd/github/IcdGitManager.scala +++ b/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,11 +493,14 @@ 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) { @@ -530,13 +508,12 @@ object IcdGitManager { 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)) } } } diff --git a/icd-git/src/test/scala/csw/services/icd/github/IcdGitManagerTests.scala b/icd-git/src/test/scala/csw/services/icd/github/IcdGitManagerTests.scala index 2d26b21a..46c1d3fc 100644 --- a/icd-git/src/test/scala/csw/services/icd/github/IcdGitManagerTests.scala +++ b/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") diff --git a/icd-web-server/app/controllers/Application.scala b/icd-web-server/app/controllers/Application.scala index 03aeb9c1..77c77b88 100644 --- a/icd-web-server/app/controllers/Application.scala +++ b/icd-web-server/app/controllers/Application.scala @@ -142,7 +142,7 @@ class Application @Inject()(env: Environment, addToken: CSRFAddToken, checkToken val tv = SubsystemAndVersion(icdVersion.target, Some(icdVersion.targetVersion)) val icds = db.versionManager.getIcdVersions(icdVersion.subsystem, icdVersion.target) if (!icds.toSet.contains(v)) - IcdGitManager.importIcdFiles(db, List(sv, tv), println(_)) + IcdGitManager.importIcdFiles(db, List(sv, tv), println(_), allIcdVersions) } } @@ -181,7 +181,6 @@ class Application @Inject()(env: Environment, addToken: CSRFAddToken, checkToken val out = new ByteArrayOutputStream() val compNames = compNamesOpt match { - case Some(s) => s.split(",").toList case None => db.versionManager.getComponentNames(subsystem, versionOpt) } diff --git a/icd/src/main/scala/csw/services/icd/StdName.scala b/icd/src/main/scala/csw/services/icd/StdName.scala index cf4f9a8f..a5c89a8d 100644 --- a/icd/src/main/scala/csw/services/icd/StdName.scala +++ b/icd/src/main/scala/csw/services/icd/StdName.scala @@ -26,7 +26,7 @@ object StdName { * True if the argument is a directory containing icd files with the standard names */ def isStdDir(d: File): Boolean = { - d.isDirectory // && d.listFiles().map(_.getName).toSet.intersect(StdName.stdSet).nonEmpty + d.isDirectory && !d.getName.endsWith(".git") } }