diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala index b7cefb777b2..97914e46a84 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/facts/nodes/NodeFactStorage.scala @@ -355,12 +355,15 @@ object GitNodeFactStorageImpl { * a special operation) */ class GitNodeFactStorageImpl( - override val gitRepo: GitRepositoryProvider, - groupOwner: Option[String], - actuallyCommit: Boolean -) extends NodeFactStorage with GitItemRepository with SerializeFacts[(NodeId, InventoryStatus), NodeFact] { + gitRepo: GitRepositoryProvider, + groupOwner: Option[String], + actuallyCommit: Boolean +) extends NodeFactStorage with SerializeFacts[(NodeId, InventoryStatus), NodeFact] { + + private val relativePath = "nodes" + + private val gitItemRepository: GitItemRepository = new GitItemRepository(gitRepo, relativePath) - override val relativePath = "nodes" override val entity: String = "node" override val fileFormat: String = "10" val committer = new PersonIdent("rudder-fact", "email not set") @@ -489,9 +492,9 @@ class GitNodeFactStorageImpl( case Some(go) => IOResult.attempt(file.setGroup(go)) } _ <- ZIO.when(actuallyCommit) { - commitAddFile( + gitItemRepository.commitAddFile( committer, - toGitPath(file.toJava), + gitItemRepository.toGitPath(file.toJava), s"Save inventory facts for ${merged.rudderSettings.status.name} node '${merged.fqdn}' (${merged.id.value})" ) } @@ -519,7 +522,11 @@ class GitNodeFactStorageImpl( } def delete(file: File) = { (if (actuallyCommit) { - commitRmFile(committer, toGitPath(file.toJava), s"Updating facts for node '${nodeId.value}': deleted") + gitItemRepository.commitRmFile( + committer, + gitItemRepository.toGitPath(file.toJava), + s"Updating facts for node '${nodeId.value}': deleted" + ) } else { IOResult.attempt(file.delete()) }).flatMap(_ => fileToNode(file)(attrs).map(Some(_)).catchAll(_ => None.succeed)).map { @@ -553,10 +560,10 @@ class GitNodeFactStorageImpl( // we need to overwrite IOResult.attempt(fromFile.moveTo(toFile)(File.CopyOptions(overwrite = true))) *> ZIO.when(actuallyCommit) { - commitMvDirectory( + gitItemRepository.commitMvDirectory( committer, - toGitPath(fromFile.toJava), - toGitPath(toFile.toJava), + gitItemRepository.toGitPath(fromFile.toJava), + gitItemRepository.toGitPath(toFile.toJava), s"Updating facts for node '${nodeId.value}' to status: ${to.name}" ) } *> fileToNode(toFile)(SelectFacts.none).map(Some(_)).catchAll(_ => None.succeed).map { diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/git/GitItemRepository.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/git/GitItemRepository.scala index cdeea12586e..b4bdd213353 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/git/GitItemRepository.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/git/GitItemRepository.scala @@ -56,17 +56,17 @@ import zio.* /** * Utility trait that factor out file commits. */ -trait GitItemRepository { +class GitItemRepository( + // the underlying git repository where items are saved + gitRepo: GitRepositoryProvider, + // relative path from git root directory where items for that implementation + // are saved. For example, rules are saved in `/var/rudder/config-repo/rules`, + // where `/var/rudder/config-repos` is the root directory provided by gitRepos, + // the relativePath is `rules` + relativePath: String +) { - // the underlying git repository where items are saved - def gitRepo: GitRepositoryProvider - // relative path from git root directory where items for that implementation - // are saved. For example, rules are saved in `/var/rudder/config-repo/rules`, - // where `/var/rudder/config-repos` is the root directory provided by gitRepos, - // the relativePath is `rules` - def relativePath: String - - lazy val getItemDirectory: File = { + final lazy val getItemDirectory: File = { /** * Create directory given in argument if does not exist, checking @@ -109,13 +109,13 @@ trait GitItemRepository { // better.files.File.pathAsString is normalized without an ending slash, an Git path are relative to "/" // *without* the leading slash. - def toGitPath(fsPath: File): String = fsPath.getPath.replace(gitRepo.rootDirectory.pathAsString + "/", "") + final def toGitPath(fsPath: File): String = fsPath.getPath.replace(gitRepo.rootDirectory.pathAsString + "/", "") /** * Files in gitPath are added. * commitMessage is used for the message of the commit. */ - def commitAddFile(commiter: PersonIdent, gitPath: String, commitMessage: String): IOResult[GitCommitId] = { + final def commitAddFile(commiter: PersonIdent, gitPath: String, commitMessage: String): IOResult[GitCommitId] = { gitRepo.semaphore.withPermit( for { _ <- GitArchiveLoggerPure.debug(s"Add file '${gitPath}' from configuration repository") @@ -140,7 +140,7 @@ trait GitItemRepository { * Files in gitPath are removed. * commitMessage is used for the message of the commit. */ - def commitRmFile(commiter: PersonIdent, gitPath: String, commitMessage: String): IOResult[GitCommitId] = { + final def commitRmFile(commiter: PersonIdent, gitPath: String, commitMessage: String): IOResult[GitCommitId] = { gitRepo.semaphore.withPermit( for { _ <- GitArchiveLoggerPure.debug(s"remove file '${gitPath}' from configuration repository") @@ -167,7 +167,7 @@ trait GitItemRepository { * 'gitRepo.git added' (with and without the 'update' mode). * commitMessage is used for the message of the commit. */ - def commitMvDirectory( + final def commitMvDirectory( commiter: PersonIdent, oldGitPath: String, newGitPath: String, @@ -198,26 +198,35 @@ trait GitItemRepository { } +object GitItemRepository {} + /* * An extension of simple GitItemRepositoty that in addition knows how to link commitId and modId together. * Used for all configuration objects, but not for facts. */ -trait GitConfigItemRepository extends GitItemRepository { +class GitConfigItemRepository( + gitRepo: GitRepositoryProvider, + relativePath: String, + gitModificationRepository: GitModificationRepository +) { - def gitModificationRepository: GitModificationRepository + private val gitItemRepository: GitItemRepository = new GitItemRepository(gitRepo, relativePath) + + final def toGitPath(fsPath: File): String = gitItemRepository.toGitPath(fsPath) + final def getItemDirectory: File = gitItemRepository.getItemDirectory /** * Files in gitPath are added. * commitMessage is used for the message of the commit. */ - def commitAddFileWithModId( + final def commitAddFileWithModId( modId: ModificationId, commiter: PersonIdent, gitPath: String, commitMessage: String ): IOResult[GitCommitId] = { for { - commit <- commitAddFile(commiter, gitPath, commitMessage) + commit <- gitItemRepository.commitAddFile(commiter, gitPath, commitMessage) mod <- gitModificationRepository.addCommit(commit, modId) } yield { commit @@ -228,14 +237,14 @@ trait GitConfigItemRepository extends GitItemRepository { * Files in gitPath are removed. * commitMessage is used for the message of the commit. */ - def commitRmFileWithModId( + final def commitRmFileWithModId( modId: ModificationId, commiter: PersonIdent, gitPath: String, commitMessage: String ): IOResult[GitCommitId] = { for { - commit <- commitRmFile(commiter, gitPath, commitMessage) + commit <- gitItemRepository.commitRmFile(commiter, gitPath, commitMessage) mod <- gitModificationRepository.addCommit(commit, modId) } yield { commit @@ -249,7 +258,7 @@ trait GitConfigItemRepository extends GitItemRepository { * 'gitRepo.git added' (with and without the 'update' mode). * commitMessage is used for the message of the commit. */ - def commitMvDirectoryWithModId( + final def commitMvDirectoryWithModId( modId: ModificationId, commiter: PersonIdent, oldGitPath: String, @@ -257,7 +266,7 @@ trait GitConfigItemRepository extends GitItemRepository { commitMessage: String ): IOResult[GitCommitId] = { for { - commit <- commitMvDirectory(commiter, oldGitPath, newGitPath, commitMessage) + commit <- gitItemRepository.commitMvDirectory(commiter, oldGitPath, newGitPath, commitMessage) mod <- gitModificationRepository.addCommit(commit, modId) } yield { commit diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/EditorTechniqueReader.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/EditorTechniqueReader.scala index 3add113f958..142e8c86613 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/EditorTechniqueReader.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/ncf/EditorTechniqueReader.scala @@ -38,20 +38,25 @@ trait EditorTechniqueReader { } class EditorTechniqueReaderImpl( - uuidGen: StringUuidGenerator, - personIdentService: PersonIdentService, - override val gitRepo: GitRepositoryProvider, - override val xmlPrettyPrinter: RudderPrettyPrinter, - override val gitModificationRepository: GitModificationRepository, - override val encoding: String, - override val groupOwner: String, - yamlTechniqueSerializer: YamlTechniqueSerializer, - parameterTypeService: ParameterTypeService, - ruddercCmd: String, - methodsSystemLib: String, - methodsLocalLib: String -) extends EditorTechniqueReader with GitConfigItemRepository with XmlArchiverUtils { - override val relativePath: String = "ncf" + uuidGen: StringUuidGenerator, + personIdentService: PersonIdentService, + gitRepo: GitRepositoryProvider, + override val xmlPrettyPrinter: RudderPrettyPrinter, + gitModificationRepository: GitModificationRepository, + override val encoding: String, + override val groupOwner: String, + yamlTechniqueSerializer: YamlTechniqueSerializer, + parameterTypeService: ParameterTypeService, + ruddercCmd: String, + methodsSystemLib: String, + methodsLocalLib: String +) extends EditorTechniqueReader with XmlArchiverUtils { + + private val relativePath: String = "ncf" + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + val configuration_repository = gitRepo.rootDirectory val ncfRootDir: File = configuration_repository / relativePath val methodsFile: File = ncfRootDir / "generic_methods.json" @@ -165,7 +170,12 @@ class EditorTechniqueReaderImpl( // commit file modId = ModificationId(uuidGen.newUuid) ident <- personIdentService.getPersonIdentOrDefault(RudderEventActor.name) - _ <- commitAddFileWithModId(modId, ident, toGitPath(methodsFile.toJava), "Saving updated generic methods definition") + _ <- gitConfigItemRepository.commitAddFileWithModId( + modId = modId, + commiter = ident, + gitPath = gitConfigItemRepository.toGitPath(methodsFile.toJava), + commitMessage = "Saving updated generic methods definition" + ) } yield { res } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/xml/GitArchivers.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/xml/GitArchivers.scala index 901eee79ca9..eeabedaf9d1 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/xml/GitArchivers.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/repository/xml/GitArchivers.scala @@ -83,27 +83,32 @@ import zio.* import zio.syntax.* class GitRuleArchiverImpl( - override val gitRepo: GitRepositoryProvider, - ruleSerialisation: RuleSerialisation, - ruleRootDir: String, // relative path ! - + override val gitRepo: GitRepositoryProvider, + ruleSerialisation: RuleSerialisation, + ruleRootDir: String, // relative path ! override val xmlPrettyPrinter: RudderPrettyPrinter, override val gitModificationRepository: GitModificationRepository, override val encoding: String, override val groupOwner: String -) extends GitRuleArchiver with XmlArchiverUtils with NamedZioLogger with GitConfigItemRepository with GitArchiverFullCommitUtils - with BuildFilePaths[RuleId] { +) extends GitRuleArchiver with XmlArchiverUtils with NamedZioLogger with GitArchiverFullCommitUtils with BuildFilePaths[RuleId] { - override def loggerName: String = this.getClass.getName override val relativePath = ruleRootDir - override val tagPrefix = "archives/configurations-rules/" + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + + override def getItemDirectory: File = gitConfigItemRepository.getItemDirectory + + override def loggerName: String = this.getClass.getName + + override val tagPrefix = "archives/configurations-rules/" override def getFileName(ruleId: RuleId): String = ruleId.serialize + ".xml" def archiveRule(rule: Rule, doCommit: Option[(ModificationId, PersonIdent, Option[String])]): IOResult[GitPath] = { for { crFile <- newFile(rule.id).toIO - gitPath = toGitPath(crFile) + gitPath = gitConfigItemRepository.toGitPath(crFile) archive <- writeXml( crFile, @@ -112,7 +117,12 @@ class GitRuleArchiverImpl( ) commit <- doCommit match { case Some((modId, commiter, reason)) => - commitAddFileWithModId(modId, commiter, gitPath, s"Archive rule with ID '${rule.id.serialize}'${GET(reason)}") + gitConfigItemRepository.commitAddFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = s"Archive rule with ID '${rule.id.serialize}'${GET(reason)}" + ) case None => ZIO.unit } } yield { @@ -133,18 +143,18 @@ class GitRuleArchiverImpl( def deleteRule(ruleId: RuleId, doCommit: Option[(ModificationId, PersonIdent, Option[String])]): IOResult[GitPath] = { for { crFile <- newFile(ruleId).toIO - gitPath = toGitPath(crFile) + gitPath = gitConfigItemRepository.toGitPath(crFile) _ <- ZIO.whenZIO(IOResult.attempt(crFile.exists)) { for { deleted <- IOResult.attempt(FileUtils.forceDelete(crFile)) _ <- logPure.debug("Deleted archive of rule: " + crFile.getPath) _ <- doCommit match { case Some((modId, commiter, reason)) => - commitRmFileWithModId( - modId, - commiter, - gitPath, - s"Delete archive of rule with ID '${ruleId.serialize}'${GET(reason)}" + gitConfigItemRepository.commitRmFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = s"Delete archive of rule with ID '${ruleId.serialize}'${GET(reason)}" ) case None => ZIO.unit } @@ -309,19 +319,18 @@ object TechniqueFiles { * - to migrate from `technique.json` to `technique.yml` if needed */ class TechniqueArchiverImpl( - override val gitRepo: GitRepositoryProvider, - override val xmlPrettyPrinter: RudderPrettyPrinter, - override val gitModificationRepository: GitModificationRepository, - personIdentservice: PersonIdentService, - techniqueParser: TechniqueParser, - techniqueCompiler: TechniqueCompiler, - override val groupOwner: String -) extends GitConfigItemRepository with XmlArchiverUtils with TechniqueArchiver { - - override val encoding: String = "UTF-8" + val gitRepo: GitRepositoryProvider, + override val xmlPrettyPrinter: RudderPrettyPrinter, + personIdentservice: PersonIdentService, + techniqueParser: TechniqueParser, + techniqueCompiler: TechniqueCompiler, + override val groupOwner: String +) extends XmlArchiverUtils with TechniqueArchiver { // we can't use "techniques" for relative path because of ncf and dsc files. This is an architecture smell, we need to clean it. - override val relativePath = "techniques" + val relativePath = "techniques" + + override val encoding: String = "UTF-8" def deleteTechnique( techniqueId: TechniqueId, @@ -484,11 +493,17 @@ class GitActiveTechniqueCategoryArchiverImpl( override val encoding: String, serializedCategoryName: String, override val groupOwner: String -) extends GitActiveTechniqueCategoryArchiver with Loggable with GitConfigItemRepository with XmlArchiverUtils +) extends GitActiveTechniqueCategoryArchiver with Loggable with XmlArchiverUtils with BuildCategoryPathName[ActiveTechniqueCategoryId] with GitArchiverFullCommitUtils { - override def loggerName: String = this.getClass.getName override lazy val relativePath = techniqueLibraryRootDir + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + + override def getItemDirectory: File = gitConfigItemRepository.getItemDirectory + + override def loggerName: String = this.getClass.getName override def getCategoryName(categoryId: ActiveTechniqueCategoryId) = categoryId.value override lazy val tagPrefix = "archives/directives/" @@ -506,7 +521,7 @@ class GitActiveTechniqueCategoryArchiverImpl( for { uptcFile <- newActiveTechniquecFile(uptc.id, newParents) - gitPath = toGitPath(uptcFile) + gitPath = gitConfigItemRepository.toGitPath(uptcFile) archive <- writeXml( uptcFile, activeTechniqueCategorySerialisation.serialise(uptc), @@ -518,16 +533,16 @@ class GitActiveTechniqueCategoryArchiverImpl( oldParents match { case Some(olds) => newActiveTechniquecFile(uptc.id, olds).flatMap(oldUptcFile => { - commitMvDirectoryWithModId( + gitConfigItemRepository.commitMvDirectoryWithModId( modId, commiter, - toGitPath(oldUptcFile), + gitConfigItemRepository.toGitPath(oldUptcFile), uptcGitPath, "Move archive of technique library category with ID '%s'%s".format(uptc.id.value, GET(reason)) ) }) case None => - commitAddFileWithModId( + gitConfigItemRepository.commitAddFileWithModId( modId, commiter, uptcGitPath, @@ -556,7 +571,7 @@ class GitActiveTechniqueCategoryArchiverImpl( ): IOResult[GitPath] = { for { uptcFile <- newActiveTechniquecFile(uptcId, getParents) - gitPath = toGitPath(uptcFile) + gitPath = gitConfigItemRepository.toGitPath(uptcFile) // don't forget to delete the category *directory* _ <- ZIO.whenZIO(IOResult.attempt(uptcFile.exists)) { for { @@ -564,11 +579,11 @@ class GitActiveTechniqueCategoryArchiverImpl( _ <- logPure.debug("Deleted archived technique library category: " + uptcFile.getPath) _ <- gitCommit match { case Some((modId, commiter, reason)) => - commitRmFileWithModId( - modId, - commiter, - gitPath, - s"Delete archive of technique library category with ID '${uptcId.value}'${GET(reason)}" + gitConfigItemRepository.commitRmFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = s"Delete archive of technique library category with ID '${uptcId.value}'${GET(reason)}" ) case None => ZIO.unit } @@ -726,21 +741,27 @@ class UpdatePiOnActiveTechniqueEvent( * A specific trait to create archive of an active technique. */ class GitActiveTechniqueArchiverImpl( - override val gitRepo: GitRepositoryProvider, + gitRepo: GitRepositoryProvider, activeTechniqueSerialisation: ActiveTechniqueSerialisation, techniqueLibraryRootDir: String, // relative path ! - override val xmlPrettyPrinter: RudderPrettyPrinter, - override val gitModificationRepository: GitModificationRepository, - val uptModificationCallback: Buffer[ActiveTechniqueModificationCallback], - override val encoding: String, - val activeTechniqueFileName: String, - override val groupOwner: String -) extends GitActiveTechniqueArchiver with NamedZioLogger with GitConfigItemRepository with XmlArchiverUtils + override val xmlPrettyPrinter: RudderPrettyPrinter, + val gitModificationRepository: GitModificationRepository, + val uptModificationCallback: Buffer[ActiveTechniqueModificationCallback], + override val encoding: String, + val activeTechniqueFileName: String, + override val groupOwner: String +) extends GitActiveTechniqueArchiver with NamedZioLogger with XmlArchiverUtils with BuildCategoryPathName[ActiveTechniqueCategoryId] { + private lazy val relativePath = techniqueLibraryRootDir + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + + override def getItemDirectory: File = gitConfigItemRepository.getItemDirectory + override def loggerName: String = this.getClass.getName - override lazy val relativePath = techniqueLibraryRootDir override def getCategoryName(categoryId: ActiveTechniqueCategoryId) = categoryId.value private def newActiveTechniqueFile(ptName: TechniqueName, parents: List[ActiveTechniqueCategoryId]) = { @@ -763,7 +784,7 @@ class GitActiveTechniqueArchiverImpl( ): IOResult[(GitPath, Seq[DirectiveNotArchived])] = { for { uptFile <- newActiveTechniqueFile(activeTechnique.techniqueName, parents) - gitPath = toGitPath(uptFile) + gitPath = gitConfigItemRepository.toGitPath(uptFile) _ <- writeXml( uptFile, activeTechniqueSerialisation.serialise(activeTechnique), @@ -775,11 +796,12 @@ class GitActiveTechniqueArchiverImpl( callbacks <- ZIO.foreach(uptModificationCallback.toList)(_.onArchive(activeTechnique, parents, gitCommit)) _ <- gitCommit match { case Some((modId, commiter, reason)) => - commitAddFileWithModId( - modId, - commiter, - gitPath, - s"Archive of technique library template for technique name '${activeTechnique.techniqueName.value}'${GET(reason)}" + gitConfigItemRepository.commitAddFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = + s"Archive of technique library template for technique name '${activeTechnique.techniqueName.value}'${GET(reason)}" ) case None => ZIO.unit } @@ -801,15 +823,16 @@ class GitActiveTechniqueArchiverImpl( // don't forget to delete the category *directory* _ <- IOResult.attempt(FileUtils.forceDelete(atFile)) _ = logPure.debug(s"Deleted archived technique library template: ${atFile.getPath}") - gitPath = toGitPath(atFile) + gitPath = gitConfigItemRepository.toGitPath(atFile) _ <- ZIO.foreach(uptModificationCallback.toList)(_.onDelete(ptName, parents, None)) _ <- gitCommit match { case Some((modId, commiter, reason)) => - commitRmFileWithModId( - modId, - commiter, - gitPath, - s"Delete archive of technique library template for technique name '${ptName.value}'${GET(reason)}" + gitConfigItemRepository.commitRmFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = + s"Delete archive of technique library template for technique name '${ptName.value}'${GET(reason)}" ) case None => ZIO.unit } @@ -817,7 +840,7 @@ class GitActiveTechniqueArchiverImpl( GitPath(gitPath) } } else { - GitPath(toGitPath(atFile)).succeed + GitPath(gitConfigItemRepository.toGitPath(atFile)).succeed } } yield { res @@ -854,11 +877,11 @@ class GitActiveTechniqueArchiverImpl( archived <- archiveActiveTechnique(activeTechnique, newParents, gitCommit) commited <- gitCommit match { case Some((modId, commiter, reason)) => - commitMvDirectoryWithModId( + gitConfigItemRepository.commitMvDirectoryWithModId( modId, commiter, - toGitPath(oldActiveTechniqueDirectory), - toGitPath(newActiveTechniqueDirectory), + gitConfigItemRepository.toGitPath(oldActiveTechniqueDirectory), + gitConfigItemRepository.toGitPath(newActiveTechniqueDirectory), "Move active technique for technique name '%s'%s".format( activeTechnique.techniqueName.value, GET(reason) @@ -877,19 +900,24 @@ class GitActiveTechniqueArchiverImpl( * A specific trait to create archive of an active technique. */ class GitDirectiveArchiverImpl( - override val gitRepo: GitRepositoryProvider, + gitRepo: GitRepositoryProvider, directiveSerialisation: DirectiveSerialisation, techniqueLibraryRootDir: String, // relative path ! - override val xmlPrettyPrinter: RudderPrettyPrinter, - override val gitModificationRepository: GitModificationRepository, - override val encoding: String, - override val groupOwner: String -) extends GitDirectiveArchiver with NamedZioLogger with GitConfigItemRepository with XmlArchiverUtils - with BuildCategoryPathName[ActiveTechniqueCategoryId] { + override val xmlPrettyPrinter: RudderPrettyPrinter, + gitModificationRepository: GitModificationRepository, + override val encoding: String, + override val groupOwner: String +) extends GitDirectiveArchiver with NamedZioLogger with XmlArchiverUtils with BuildCategoryPathName[ActiveTechniqueCategoryId] { + + private lazy val relativePath = techniqueLibraryRootDir + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + + override def getItemDirectory: File = gitConfigItemRepository.getItemDirectory override def loggerName: String = this.getClass.getName - override lazy val relativePath = techniqueLibraryRootDir override def getCategoryName(categoryId: ActiveTechniqueCategoryId) = categoryId.value private def newPiFile( @@ -918,7 +946,7 @@ class GitDirectiveArchiverImpl( for { piFile <- newPiFile(directive.id.uid, ptName, catIds) - gitPath = toGitPath(piFile) + gitPath = gitConfigItemRepository.toGitPath(piFile) archive <- writeXml( piFile, directiveSerialisation.serialise(ptName, Some(variableRootSection), directive), @@ -926,11 +954,11 @@ class GitDirectiveArchiverImpl( ) commit <- gitCommit match { case Some((modId, commiter, reason)) => - commitAddFileWithModId( - modId, - commiter, - gitPath, - "Archive directive with ID '%s'%s".format(directive.id.uid.value, GET(reason)) + gitConfigItemRepository.commitAddFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = "Archive directive with ID '%s'%s".format(directive.id.uid.value, GET(reason)) ) case None => ZIO.unit } @@ -957,14 +985,14 @@ class GitDirectiveArchiverImpl( for { _ <- IOResult.attempt(FileUtils.forceDelete(piFile)) _ <- logPure.debug(s"Deleted archive of directive: '${piFile.getPath}'") - gitPath = toGitPath(piFile) + gitPath = gitConfigItemRepository.toGitPath(piFile) _ <- gitCommit match { case Some((modId, commiter, reason)) => - commitRmFileWithModId( - modId, - commiter, - gitPath, - s"Delete archive of directive with ID '${directiveId.value}'${GET(reason)}" + gitConfigItemRepository.commitRmFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = s"Delete archive of directive with ID '${directiveId.value}'${GET(reason)}" ) case None => ZIO.unit } @@ -972,7 +1000,7 @@ class GitDirectiveArchiverImpl( GitPath(gitPath) } } else { - GitPath(toGitPath(piFile)).succeed + GitPath(gitConfigItemRepository.toGitPath(piFile)).succeed } } yield { res @@ -1001,11 +1029,17 @@ class GitNodeGroupArchiverImpl( override val encoding: String, serializedCategoryName: String, override val groupOwner: String -) extends GitNodeGroupArchiver with NamedZioLogger with GitConfigItemRepository with XmlArchiverUtils - with BuildCategoryPathName[NodeGroupCategoryId] with GitArchiverFullCommitUtils { +) extends GitNodeGroupArchiver with NamedZioLogger with XmlArchiverUtils with BuildCategoryPathName[NodeGroupCategoryId] + with GitArchiverFullCommitUtils { - override def loggerName: String = this.getClass.getName override lazy val relativePath = groupLibraryRootDir + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + + override def getItemDirectory: File = gitConfigItemRepository.getItemDirectory + + override def loggerName: String = this.getClass.getName override def getCategoryName(categoryId: NodeGroupCategoryId) = categoryId.value override lazy val tagPrefix = "archives/groups/" @@ -1027,14 +1061,14 @@ class GitNodeGroupArchiverImpl( nodeGroupCategorySerialisation.serialise(ngc), "Archived node group category: " + ngcFile.getPath ) - gitPath = toGitPath(ngcFile) + gitPath = gitConfigItemRepository.toGitPath(ngcFile) commit <- gitCommit match { case Some((modId, commiter, reason)) => - commitAddFileWithModId( - modId, - commiter, - gitPath, - "Archive of node group category with ID '%s'%s".format(ngc.id.value, GET(reason)) + gitConfigItemRepository.commitAddFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = "Archive of node group category with ID '%s'%s".format(ngc.id.value, GET(reason)) ) case None => ZIO.unit } @@ -1050,7 +1084,7 @@ class GitNodeGroupArchiverImpl( ): IOResult[GitPath] = { for { ngcFile <- newNgFile(ngcId, getParents) - gitPath = toGitPath(ngcFile) + gitPath = gitConfigItemRepository.toGitPath(ngcFile) // don't forget to delete the category *directory* _ <- ZIO.whenZIO(IOResult.attempt(ngcFile.exists)) { for { @@ -1058,11 +1092,11 @@ class GitNodeGroupArchiverImpl( _ <- logPure.debug(s"Deleted archived node group category: ${ngcFile.getPath}") _ <- gitCommit match { case Some((modId, commiter, reason)) => - commitRmFileWithModId( - modId, - commiter, - gitPath, - s"Delete archive of node group category with ID '${ngcId.value}'${GET(reason)}" + gitConfigItemRepository.commitRmFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = s"Delete archive of node group category with ID '${ngcId.value}'${GET(reason)}" ) case None => ZIO.unit } @@ -1116,17 +1150,17 @@ class GitNodeGroupArchiverImpl( } commit <- gitCommit match { case Some((modId, commiter, reason)) => - commitMvDirectoryWithModId( - modId, - commiter, - toGitPath(oldNgcDir), - toGitPath(newNgcDir), - "Move archive of node group category with ID '%s'%s".format(ngc.id.value, GET(reason)) + gitConfigItemRepository.commitMvDirectoryWithModId( + modId = modId, + commiter = commiter, + oldGitPath = gitConfigItemRepository.toGitPath(oldNgcDir), + newGitPath = gitConfigItemRepository.toGitPath(newNgcDir), + commitMessage = "Move archive of node group category with ID '%s'%s".format(ngc.id.value, GET(reason)) ) case None => ZIO.unit } } yield { - GitPath(toGitPath(archive)) + GitPath(gitConfigItemRepository.toGitPath(archive)) } } } @@ -1174,16 +1208,16 @@ class GitNodeGroupArchiverImpl( ) commit <- gitCommit match { case Some((modId, commiter, reason)) => - commitAddFileWithModId( - modId, - commiter, - toGitPath(ngFile), - "Archive of node group with ID '%s'%s".format(ng.id.withDefaultRev.serialize, GET(reason)) + gitConfigItemRepository.commitAddFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitConfigItemRepository.toGitPath(ngFile), + commitMessage = "Archive of node group with ID '%s'%s".format(ng.id.withDefaultRev.serialize, GET(reason)) ) case None => ZIO.unit } } yield { - GitPath(toGitPath(archive)) + GitPath(gitConfigItemRepository.toGitPath(archive)) } } @@ -1194,7 +1228,7 @@ class GitNodeGroupArchiverImpl( ): IOResult[GitPath] = { for { ngFile <- newNgFile(ngId, getParents) - gitPath = toGitPath(ngFile) + gitPath = gitConfigItemRepository.toGitPath(ngFile) exists <- IOResult.attempt(ngFile.exists) res <- if (exists) { for { @@ -1203,11 +1237,12 @@ class GitNodeGroupArchiverImpl( _ <- logPure.debug(s"Deleted archived node group: ${ngFile.getPath}") _ <- gitCommit match { case Some((modId, commiter, reason)) => - commitRmFileWithModId( - modId, - commiter, - gitPath, - s"Delete archive of node group with ID '${ngId.withDefaultRev.serialize}'${GET(reason)}" + gitConfigItemRepository.commitRmFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = + s"Delete archive of node group with ID '${ngId.withDefaultRev.serialize}'${GET(reason)}" ) case None => ZIO.unit } @@ -1244,17 +1279,18 @@ class GitNodeGroupArchiverImpl( } commit <- gitCommit match { case Some((modId, commiter, reason)) => - commitMvDirectoryWithModId( - modId, - commiter, - toGitPath(oldNgXmlFile), - toGitPath(newNgXmlFile), - "Move archive of node group with ID '%s'%s".format(ng.id.withDefaultRev.serialize, GET(reason)) + gitConfigItemRepository.commitMvDirectoryWithModId( + modId = modId, + commiter = commiter, + oldGitPath = gitConfigItemRepository.toGitPath(oldNgXmlFile), + newGitPath = gitConfigItemRepository.toGitPath(newNgXmlFile), + commitMessage = + "Move archive of node group with ID '%s'%s".format(ng.id.withDefaultRev.serialize, GET(reason)) ) case None => ZIO.unit } } yield { - GitPath(toGitPath(archive)) + GitPath(gitConfigItemRepository.toGitPath(archive)) } } } @@ -1277,12 +1313,18 @@ class GitParameterArchiverImpl( override val gitModificationRepository: GitModificationRepository, override val encoding: String, override val groupOwner: String -) extends GitParameterArchiver with NamedZioLogger with GitConfigItemRepository with XmlArchiverUtils - with GitArchiverFullCommitUtils with BuildFilePaths[String] { +) extends GitParameterArchiver with NamedZioLogger with XmlArchiverUtils with GitArchiverFullCommitUtils + with BuildFilePaths[String] { - override def loggerName: String = this.getClass.getName override val relativePath = parameterRootDir - override val tagPrefix = "archives/parameters/" + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + + override def getItemDirectory: File = gitConfigItemRepository.getItemDirectory + + override def loggerName: String = this.getClass.getName + override val tagPrefix = "archives/parameters/" override def getFileName(parameterName: String) = parameterName + ".xml" @@ -1292,7 +1334,7 @@ class GitParameterArchiverImpl( ): IOResult[GitPath] = { for { paramFile <- newFile(parameter.name).toIO - gitPath = toGitPath(paramFile) + gitPath = gitConfigItemRepository.toGitPath(paramFile) archive <- writeXml( paramFile, parameterSerialisation.serialise(parameter), @@ -1301,7 +1343,7 @@ class GitParameterArchiverImpl( commit <- doCommit match { case Some((modId, commiter, reason)) => val msg = "Archive parameter with name '%s'%s".format(parameter.name, GET(reason)) - commitAddFileWithModId(modId, commiter, gitPath, msg) + gitConfigItemRepository.commitAddFileWithModId(modId, commiter, gitPath, msg) case None => ZIO.unit } } yield { @@ -1325,18 +1367,18 @@ class GitParameterArchiverImpl( ): IOResult[GitPath] = { for { paramFile <- newFile(parameterName).toIO - gitPath = toGitPath(paramFile) + gitPath = gitConfigItemRepository.toGitPath(paramFile) _ <- ZIO.whenZIO(IOResult.attempt(paramFile.exists)) { for { deleted <- IOResult.attempt(FileUtils.forceDelete(paramFile)) _ <- logPure.debug(s"Deleted archive of parameter: ${paramFile.getPath}") _ <- doCommit match { case Some((modId, commiter, reason)) => - commitRmFileWithModId( - modId, - commiter, - gitPath, - s"Delete archive of parameter with name '${parameterName}'${GET(reason)}" + gitConfigItemRepository.commitRmFileWithModId( + modId = modId, + commiter = commiter, + gitPath = gitPath, + commitMessage = s"Delete archive of parameter with name '${parameterName}'${GET(reason)}" ) case None => ZIO.unit } diff --git a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/rule/category/GitRuleCategoryArchiver.scala b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/rule/category/GitRuleCategoryArchiver.scala index fb02d36253b..4095034b5c0 100644 --- a/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/rule/category/GitRuleCategoryArchiver.scala +++ b/webapp/sources/rudder/rudder-core/src/main/scala/com/normation/rudder/rule/category/GitRuleCategoryArchiver.scala @@ -126,12 +126,18 @@ class GitRuleCategoryArchiverImpl( override val encoding: String, categoryFileName: String, override val groupOwner: String -) extends GitRuleCategoryArchiver with NamedZioLogger with GitConfigItemRepository with XmlArchiverUtils - with GitArchiverFullCommitUtils with BuildCategoryPathName[RuleCategoryId] { +) extends GitRuleCategoryArchiver with NamedZioLogger with XmlArchiverUtils with GitArchiverFullCommitUtils + with BuildCategoryPathName[RuleCategoryId] { - override def loggerName: String = this.getClass.getName override val relativePath = ruleCategoryRootDir - override val tagPrefix = "archives/configurations-rules/" + + private val gitConfigItemRepository: GitConfigItemRepository = + new GitConfigItemRepository(gitRepo, relativePath, gitModificationRepository) + + override def getItemDirectory: File = gitConfigItemRepository.getItemDirectory + + override def loggerName: String = this.getClass.getName + override val tagPrefix = "archives/configurations-rules/" def getCategoryName(categoryId: RuleCategoryId): String = categoryId.value @@ -146,7 +152,7 @@ class GitRuleCategoryArchiverImpl( // Build Rule category file, needs to reverse parents , start from end) for { ruleCategoryFile <- categoryFile(category.id, parents.reverse) - gitPath = toGitPath(ruleCategoryFile) + gitPath = gitConfigItemRepository.toGitPath(ruleCategoryFile) archive <- writeXml( ruleCategoryFile, ruleCategorySerialisation.serialise(category), @@ -155,7 +161,7 @@ class GitRuleCategoryArchiverImpl( commit <- gitCommit match { case Some((modId, commiter, reason)) => val commitMsg = s"Archive rule Category with ID '${category.id.value}' ${GET(reason)}" - commitAddFileWithModId(modId, commiter, gitPath, commitMsg) + gitConfigItemRepository.commitAddFileWithModId(modId, commiter, gitPath, commitMsg) case None => ZIO.unit } @@ -179,7 +185,7 @@ class GitRuleCategoryArchiverImpl( // Build Rule category file, needs to reverse parents , start from end) for { ruleCategoryFile <- categoryFile(categoryId, parents.reverse) - gitPath = toGitPath(ruleCategoryFile) + gitPath = gitConfigItemRepository.toGitPath(ruleCategoryFile) _ <- ZIO.whenZIO(IOResult.attempt(ruleCategoryFile.exists)) { for { _ <- IOResult.attempt(FileUtils.forceDelete(ruleCategoryFile)) @@ -187,7 +193,7 @@ class GitRuleCategoryArchiverImpl( _ <- doCommit match { case Some((modId, commiter, reason)) => val commitMsg = s"Delete archive of rule with ID '${categoryId.value} ${GET(reason)}" - commitRmFileWithModId(modId, commiter, gitPath, commitMsg) + gitConfigItemRepository.commitRmFileWithModId(modId, commiter, gitPath, commitMsg) case None => ZIO.unit } @@ -232,14 +238,14 @@ class GitRuleCategoryArchiverImpl( commit <- gitCommit match { case Some((modId, commiter, reason)) => val commitMsg = s"Move archive of rule category with ID '${category.id.value}'${GET(reason)}" - val oldPath = toGitPath(oldCategoryDir) - val newPath = toGitPath(newCategoryDir) - commitMvDirectoryWithModId(modId, commiter, oldPath, newPath, commitMsg) + val oldPath = gitConfigItemRepository.toGitPath(oldCategoryDir) + val newPath = gitConfigItemRepository.toGitPath(newCategoryDir) + gitConfigItemRepository.commitMvDirectoryWithModId(modId, commiter, oldPath, newPath, commitMsg) case None => ZIO.unit } } yield { - GitPath(toGitPath(archive)) + GitPath(gitConfigItemRepository.toGitPath(archive)) } } } diff --git a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/services/JGitRepositoryTest.scala b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/services/JGitRepositoryTest.scala index 5dd366fc6d4..490c29e0b80 100644 --- a/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/services/JGitRepositoryTest.scala +++ b/webapp/sources/rudder/rudder-core/src/test/scala/com/normation/cfclerk/services/JGitRepositoryTest.scala @@ -44,111 +44,74 @@ import com.normation.cfclerk.xmlparsers.SectionSpecParser import com.normation.cfclerk.xmlparsers.TechniqueParser import com.normation.cfclerk.xmlparsers.VariableSpecParser import com.normation.errors -import com.normation.errors.Inconsistency import com.normation.errors.IOResult +import com.normation.errors.RudderError import com.normation.errors.effectUioUnit import com.normation.eventlog.EventActor import com.normation.eventlog.ModificationId -import com.normation.rudder.db.DB -import com.normation.rudder.git.GitCommitId -import com.normation.rudder.git.GitConfigItemRepository +import com.normation.rudder.git.GitItemRepository import com.normation.rudder.git.GitRepositoryProvider import com.normation.rudder.git.GitRepositoryProviderImpl import com.normation.rudder.ncf.EditorTechnique import com.normation.rudder.ncf.TechniqueCompilationOutput import com.normation.rudder.ncf.TechniqueCompiler import com.normation.rudder.ncf.TechniqueCompilerApp -import com.normation.rudder.repository.GitModificationRepository import com.normation.rudder.repository.xml.RudderPrettyPrinter +import com.normation.rudder.repository.xml.TechniqueArchiver import com.normation.rudder.repository.xml.TechniqueArchiverImpl -import com.normation.rudder.repository.xml.XmlArchiverUtils import com.normation.rudder.services.user.TrivialPersonIdentService -import com.normation.zio.* -import net.liftweb.common.Loggable -import org.apache.commons.io.FileUtils import org.eclipse.jgit.lib.PersonIdent import org.eclipse.jgit.lib.Repository import org.eclipse.jgit.revwalk.RevWalk -import org.joda.time.DateTime -import org.junit.runner.RunWith -import org.specs2.mutable.Specification -import org.specs2.runner.JUnitRunner -import org.specs2.specification.AfterAll -import scala.annotation.nowarn -import scala.util.Random -import zio.{System as _, *} -import zio.syntax.* - -/** - * Details of tests executed in each instances of - * the test. - * To see values for gitRoot, ptLib, etc, see at the end - * of that file. - */ -@nowarn("msg=a type was inferred to be `\\w+`; this may indicate a programming error.") -@RunWith(classOf[JUnitRunner]) -class JGitRepositoryTest extends Specification with Loggable with AfterAll { - - val gitRoot: File = File("/tmp/test-jgit-" + DateTime.now().toString()) - - // Set sequential execution - sequential +import org.eclipse.jgit.treewalk.TreeWalk +import zio.Chunk +import zio.System +import zio.ZIO +import zio.ZLayer +import zio.syntax.ToZio +import zio.test.* +import zio.test.Assertion.* + +object JGitRepositoryTest extends ZIOSpecDefault { + def spec: Spec[Any, RudderError] = suite("JGitRepositoryTest")(GitItemRepositoryTest.spec, TechniqueArchiverTest.spec) +} - /** - * Add a switch to be able to see tmp files (not clean temps) with - * -Dtests.clean.tmp=false - */ - override def afterAll(): Unit = { - if (java.lang.System.getProperty("tests.clean.tmp") != "false") { - logger.info("Deleting directory " + gitRoot.pathAsString) - FileUtils.deleteDirectory(gitRoot.toJava) +object GitItemRepositoryTest { + def spec: Spec[Any, RudderError] = suite("GitItemRepository")( + // to assess the usefulness of semaphore, you can remove `gitRepo.semaphore.withPermit` + // in `commitAddFile` to check that you get the JGitInternalException. + // More advanced tests may be needed to handle more complex cases of concurrent access, + // see: https://issues.rudder.io/issues/19910 + test("should not throw JGitInternalError on concurrent write") { + + def addFileToRepositoryAndCommit(i: Int)(gitRoot: TempDir, archive: GitItemRepository) = for { + name <- zio.Random.nextString(8).map(s => i.toString + "_" + s) + file = gitRoot.path / name + _ <- IOResult.attempt(file.write("something in " + name)) + actor = new PersonIdent("test", "test@test.com") + _ <- archive.commitAddFile(actor, name, "add " + name) + } yield name + + for { + gitRoot <- ZIO.service[TempDir] + gitRepositoryProvider <- ZIO.service[GitRepositoryProvider] + archive <- ZIO.service[GitItemRepository] + files <- ZIO.foreachPar((1 to 50))(i => addFileToRepositoryAndCommit(i)(gitRoot, archive)).withParallelism(16) + created <- readElementsAt(gitRepositoryProvider.db, "refs/heads/master") + } yield assert(created)(hasSameElements(files)) + + }.provide(TempDir.layer, Layers.gitRepositoryProvider, gitItemRepositoryLayer) + ) + + private val gitItemRepositoryLayer = { + ZLayer { + for { + repository <- ZIO.service[GitRepositoryProvider] + } yield new GitItemRepository(gitRepo = repository, relativePath = "") } } - gitRoot.createDirectories() - - val repo: GitRepositoryProviderImpl = GitRepositoryProviderImpl.make(gitRoot.pathAsString).runNow - val prettyPrinter: RudderPrettyPrinter = new RudderPrettyPrinter(Int.MaxValue, 2) - val modRepo: GitModificationRepository = new GitModificationRepository { - override def getCommits(modificationId: ModificationId): IOResult[Option[GitCommitId]] = None.succeed - override def addCommit(commit: GitCommitId, modId: ModificationId): IOResult[DB.GitCommitJoin] = - DB.GitCommitJoin(commit, modId).succeed - } - val personIdent: TrivialPersonIdentService = new TrivialPersonIdentService() - val techniqueParser: TechniqueParser = { - val varParser = new VariableSpecParser - new TechniqueParser(varParser, new SectionSpecParser(varParser), new SystemVariableSpecServiceImpl()) - } - val techniqueCompiler = new TechniqueCompiler { - override def compileTechnique(technique: EditorTechnique): IOResult[TechniqueCompilationOutput] = { - TechniqueCompilationOutput(TechniqueCompilerApp.Rudderc, 0, Chunk.empty, "", "", "").succeed - } - - override def getCompilationOutputFile(technique: EditorTechnique): File = File("compilation-config.yml") - - override def getCompilationConfigFile(technique: EditorTechnique): File = File("compilation-output.yml") - } - - // for test, we use as a group owner whatever git root directory has - val currentUserName: String = repo.rootDirectory.groupName - - val archive: GitConfigItemRepository with XmlArchiverUtils = new GitConfigItemRepository with XmlArchiverUtils { - override val gitRepo: GitRepositoryProvider = repo - override def relativePath: String = "" - override def xmlPrettyPrinter = prettyPrinter - override def encoding: String = "UTF-8" - override def gitModificationRepository: GitModificationRepository = modRepo - - override def groupOwner: String = currentUserName - } - - val techniqueArchive: TechniqueArchiverImpl = - new TechniqueArchiverImpl(repo, prettyPrinter, modRepo, personIdent, techniqueParser, techniqueCompiler, currentUserName) - // listing files at a commit is complicated - - import org.eclipse.jgit.treewalk.TreeWalk - def readElementsAt(repository: Repository, commit: String): ZIO[Any, errors.SystemError, List[String]] = { val ref = repository.findRef(commit) @@ -161,7 +124,7 @@ class JGitRepositoryTest extends Specification with Loggable with AfterAll { for { commit <- IOResult.attempt(walk.parseCommit(ref.getObjectId)) tree <- IOResult.attempt(commit.getTree) - res <- ZIO.scoped(treeWalkM.flatMap { treeWalk => + res <- ZIO.scoped[Any](treeWalkM.flatMap { treeWalk => treeWalk.setRecursive(true) // ok, can't throw exception IOResult.attempt(treeWalk.addTree(tree)) *> @@ -172,83 +135,149 @@ class JGitRepositoryTest extends Specification with Loggable with AfterAll { ) } - "The test lib" should { - "not throw JGitInternalError on concurrent write" in { - - // to assess the usefulness of semaphore, you can remove `gitRepo.semaphore.withPermit` - // in `commitAddFile` to check that you get the JGitInternalException. - // More advanced tests may be needed to handle more complex cases of concurrent access, - // see: https://issues.rudder.io/issues/19910 +} - val actor = new PersonIdent("test", "test@test.com") +object TechniqueArchiverTest { - def getName(length: Int) = { - if (length < 1) Inconsistency("Length must be positive").fail - else { - IOResult.attempt("")(Random.alphanumeric.take(length).toList.mkString("")) - } - } - def add(i: Int) = (for { - name <- getName(8).map(s => i.toString + "_" + s) - file = gitRoot / name - f <- IOResult.attempt(file.write("something in " + name)) - _ <- archive.commitAddFileWithModId(ModificationId(name), actor, name, "add " + name) - } yield (name)) + def prettyPrinter: RudderPrettyPrinter = new RudderPrettyPrinter(Int.MaxValue, 2) - logger.debug(s"Commiting files in: " + gitRoot.pathAsString) - val files = ZIO.foreachPar(1 to 50)(i => add(i)).withParallelism(16).runNow + val category = TechniqueCategoryMetadata("My new category", "A new category", isSystem = false) + val catPath = List("systemSettings", "myNewCategory") + val modId = ModificationId("add-technique-cat") - val created = readElementsAt(repo.db, "refs/heads/master").runNow - created must containTheSameElementsAs(files) + case class GroupOwner(value: String) + // for test, we use as a group owner whatever git root directory has + def currentUserName(repo: GitRepositoryProvider): GroupOwner = GroupOwner(repo.rootDirectory.groupName) + + def spec: Spec[Any, RudderError] = suite("TechniqueArchiver.saveTechniqueCategory")( + test("should create a new file and commit if the category does not exist") { + for { + gitRepositoryProvider <- ZIO.service[GitRepositoryProvider] + techniqueArchive <- ZIO.service[TechniqueArchiver] + _ <- techniqueArchive + .saveTechniqueCategory( + catPath, + category, + modId, + EventActor("test"), + s"test: commit add category ${catPath.mkString("/")}" + ) + catFile = gitRepositoryProvider.rootDirectory / "techniques" / "systemSettings" / "myNewCategory" / "category.xml" + xml = catFile.contentAsString + lastCommitMsg = gitRepositoryProvider.git.log().setMaxCount(1).call().iterator().next().getFullMessage + } yield { + // note: no false ; it's only written when true + assert(xml)( + equalTo( + """ + | My new category + | A new category + |""".stripMargin + ) + ) && + assert(lastCommitMsg)(equalTo("test: commit add category systemSettings/myNewCategory")) + } + }, + test("should do nothing when the category already exists") { + for { + gitRepositoryProvider <- ZIO.service[GitRepositoryProvider] + techniqueArchive <- ZIO.service[TechniqueArchiver] + _ <- techniqueArchive + .saveTechniqueCategory( + catPath, + category, + modId, + EventActor("test"), + s"test: commit add category ${catPath.mkString("/")}" + ) + _ <- techniqueArchive.saveTechniqueCategory(catPath, category, modId, EventActor("test"), s"test: commit again") + lastCommitMsg = gitRepositoryProvider.git.log().setMaxCount(1).call().iterator().next().getFullMessage + } yield assert(lastCommitMsg)(equalTo("test: commit add category systemSettings/myNewCategory")) } + ).provide( + TempDir.layer, + techniqueParserLayer, + stubbedTechniqueCompilerLayer, + Layers.gitRepositoryProvider, + ZLayer.succeed(new TrivialPersonIdentService()), + currentUserNameLayer, + techniqueArchiverLayer + ) + + private val techniqueParserLayer = ZLayer.succeed { + val varParser = new VariableSpecParser + new TechniqueParser(varParser, new SectionSpecParser(varParser), new SystemVariableSpecServiceImpl()) + } - "save a category" should { - - val category = TechniqueCategoryMetadata("My new category", "A new category", isSystem = false) - val catPath = List("systemSettings", "myNewCategory") - - val modId = new ModificationId("add-technique-cat") - - "create a new file and commit if the category does not exist" in { + private val stubbedTechniqueCompilerLayer = ZLayer.succeed { + new TechniqueCompiler { + override def compileTechnique(technique: EditorTechnique): IOResult[TechniqueCompilationOutput] = { + TechniqueCompilationOutput(TechniqueCompilerApp.Rudderc, 0, Chunk.empty, "", "", "").succeed + } - techniqueArchive - .saveTechniqueCategory( - catPath, - category, - modId, - EventActor("test"), - s"test: commit add category ${catPath.mkString("/")}" - ) - .runNow + override def getCompilationOutputFile(technique: EditorTechnique): File = File("compilation-config.yml") - val catFile = repo.rootDirectory / "techniques" / "systemSettings" / "myNewCategory" / "category.xml" + override def getCompilationConfigFile(technique: EditorTechnique): File = File("compilation-output.yml") + } + } - val xml = catFile.contentAsString + private val currentUserNameLayer = ZLayer { + for { + repo <- ZIO.service[GitRepositoryProvider] + } yield currentUserName(repo) + } - val lastCommitMsg = repo.git.log().setMaxCount(1).call().iterator().next().getFullMessage + private val techniqueArchiverLayer = ZLayer { + for { + techniqueParse <- ZIO.service[TechniqueParser] + techniqueCompiler <- ZIO.service[TechniqueCompiler] + gitRepositoryProvider <- ZIO.service[GitRepositoryProvider] + personIdentservice <- ZIO.service[TrivialPersonIdentService] + groupOwner <- ZIO.service[GroupOwner] + } yield new TechniqueArchiverImpl( + gitRepo = gitRepositoryProvider, + xmlPrettyPrinter = prettyPrinter, + personIdentservice = personIdentservice, + techniqueParser = techniqueParse, + techniqueCompiler = techniqueCompiler, + groupOwner = groupOwner.value + ) - // note: no false ; it's only written when true - (xml === - """ - | My new category - | A new category - |""".stripMargin) and ( - lastCommitMsg === "test: commit add category systemSettings/myNewCategory" - ) + } - } +} - "does nothing when the category already exsits" in { - techniqueArchive.saveTechniqueCategory(catPath, category, modId, EventActor("test"), s"test: commit again").runNow - val lastCommitMsg = repo.git.log().setMaxCount(1).call().iterator().next().getFullMessage +case class TempDir(path: File) - // last commit must be the old one - lastCommitMsg === "test: commit add category systemSettings/myNewCategory" +object TempDir { - } - } + /** + * Add a switch to be able to see tmp files (not clean temps) with + * -Dtests.clean.tmp=false + */ + val layer: ZLayer[Any, Nothing, TempDir] = ZLayer.scoped { + for { + keepFiles <- System.property("tests.clean.tmp").map(_.contains("false")).orDie + tempDir <- ZIO + .attemptBlockingIO(File.newTemporaryDirectory(prefix = "test-jgit-")) + .withFinalizer { dir => + (ZIO.logInfo(s"Deleting directory ${dir.path}") *> + ZIO.attemptBlocking(dir.delete(swallowIOExceptions = true))) + .unless(keepFiles) + .orDie + } + .orDie + } yield TempDir(tempDir) + } +} +object Layers { + private[services] val gitRepositoryProvider = ZLayer { + for { + gitRoot <- ZIO.service[TempDir] + result <- GitRepositoryProviderImpl.make(gitRoot.path.pathAsString) + } yield result } } diff --git a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala index 69ffef5e1b2..353f484ec12 100644 --- a/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala +++ b/webapp/sources/rudder/rudder-web/src/main/scala/bootstrap/liftweb/RudderConfig.scala @@ -1783,7 +1783,6 @@ object RudderConfigInit { lazy val techniqueArchiver = new TechniqueArchiverImpl( gitConfigRepo, prettyPrinter, - gitModificationRepository, personIdentService, techniqueParser, techniqueCompiler, diff --git a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateJsonTechniquesToYaml.scala b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateJsonTechniquesToYaml.scala index 835748db398..d3d69f47e94 100644 --- a/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateJsonTechniquesToYaml.scala +++ b/webapp/sources/rudder/rudder-web/src/test/scala/bootstrap/liftweb/checks/migration/TestMigrateJsonTechniquesToYaml.scala @@ -43,30 +43,10 @@ import com.normation.eventlog.ModificationId import com.normation.inventory.domain.Version import com.normation.rudder.MockGitConfigRepo import com.normation.rudder.MockTechniques -import com.normation.rudder.db.DB import com.normation.rudder.facts.nodes.QueryContext -import com.normation.rudder.git.GitCommitId import com.normation.rudder.hooks.CmdResult -import com.normation.rudder.ncf.BundleName -import com.normation.rudder.ncf.CompilationResult -import com.normation.rudder.ncf.DeleteEditorTechnique -import com.normation.rudder.ncf.EditorTechniqueCompilationResult -import com.normation.rudder.ncf.EditorTechniqueReader -import com.normation.rudder.ncf.EditorTechniqueReaderImpl -import com.normation.rudder.ncf.GenericMethod -import com.normation.rudder.ncf.GitResourceFileService -import com.normation.rudder.ncf.ReadEditorTechniqueCompilationResult -import com.normation.rudder.ncf.ResourceFile -import com.normation.rudder.ncf.ResourceFileState -import com.normation.rudder.ncf.RuddercOptions -import com.normation.rudder.ncf.RuddercResult -import com.normation.rudder.ncf.RuddercService -import com.normation.rudder.ncf.RuddercTechniqueCompiler -import com.normation.rudder.ncf.TechniqueCompilationErrorsActorSync -import com.normation.rudder.ncf.TechniqueCompilationStatusService -import com.normation.rudder.ncf.TechniqueWriterImpl +import com.normation.rudder.ncf.* import com.normation.rudder.ncf.yaml.YamlTechniqueSerializer -import com.normation.rudder.repository.GitModificationRepository import com.normation.rudder.repository.xml.RudderPrettyPrinter import com.normation.rudder.repository.xml.TechniqueArchiverImpl import com.normation.rudder.repository.xml.TechniqueFiles @@ -151,11 +131,6 @@ class TestMigrateJsonTechniquesToYaml extends Specification with ContentMatchers val techniqueArchiver = new TechniqueArchiverImpl( gitMock.gitRepo, xmlPrettyPrinter, - new GitModificationRepository { - override def getCommits(modificationId: ModificationId): IOResult[Option[GitCommitId]] = None.succeed - override def addCommit(commit: GitCommitId, modId: ModificationId): IOResult[DB.GitCommitJoin] = - DB.GitCommitJoin(commit, modId).succeed - }, new TrivialPersonIdentService(), techMock.techniqueParser, techniqueCompiler,