Skip to content

Commit

Permalink
refactor(git): git handler class hierarchy (#5967)
Browse files Browse the repository at this point in the history
* chore: declare internally used methods as private

* chore: rename `GitHandler` to `GitSubTreeHandler`

To align naming with another components and functions,
and to make it more descriptive.

* refactor: extract class `AbstractGitHandler`

It has the only abstract method `getFiles(...)`
that would be implemented by each Git repo scanning mode.

* refactor: move `GitSubTreeHandler` impl to a separate file

* refactor:  replace inheritance with composition

Now `GitRepoHandler` extends `AbstractGitHandler` instead of `GitSubTreeHandler`,
and contains the latter as a class member
to be able to delegate to its `getFiles()` method if necessary.

* chore: fix imports

* refactor: declare `VcsHandler.name` as readonly

* chore: linting

* chore: fix imports

* refactor: remove unnecessary hack to avoid infinite recursion
  • Loading branch information
vvagaytsev committed Apr 25, 2024
1 parent 5f76a73 commit 746f038
Show file tree
Hide file tree
Showing 9 changed files with 490 additions and 446 deletions.
5 changes: 3 additions & 2 deletions core/src/commands/get/get-debug-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { CommandParams } from "../base.js"
import { Command } from "../base.js"
import { findProjectConfig } from "../../config/base.js"
import fsExtra from "fs-extra"

const { ensureDir, copy, remove, pathExists, writeFile } = fsExtra
import { getPackageVersion } from "../../util/util.js"
import { platform, release } from "os"
Expand All @@ -20,7 +21,7 @@ import { ERROR_LOG_FILENAME } from "../../constants.js"
import dedent from "dedent"
import type { Garden } from "../../garden.js"
import { zipFolder } from "../../util/archive.js"
import { GitHandler } from "../../vcs/git.js"
import { GitSubTreeHandler } from "../../vcs/git-sub-tree.js"
import { ValidationError } from "../../exceptions.js"
import { ChoicesParameter, BooleanParameter } from "../../cli/params.js"
import { printHeader } from "../../logger/util.js"
Expand Down Expand Up @@ -73,7 +74,7 @@ export async function collectBasicDebugInfo(root: string, gardenDirPath: string,

// Find all services paths
const cache = new TreeCache()
const vcs = new GitHandler({
const vcs = new GitSubTreeHandler({
projectRoot: root,
gardenDirPath,
ignoreFile: projectConfig.dotIgnoreFile || defaultDotIgnoreFile,
Expand Down
6 changes: 3 additions & 3 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import {
} from "./exceptions.js"
import type { VcsHandler, ModuleVersion, VcsInfo } from "./vcs/vcs.js"
import { getModuleVersionString } from "./vcs/vcs.js"
import { GitHandler } from "./vcs/git.js"
import { GitSubTreeHandler } from "./vcs/git-sub-tree.js"
import { BuildStaging } from "./build-staging/build-staging.js"
import type { ConfigGraph } from "./graph/config-graph.js"
import { ResolvedConfigGraph } from "./graph/config-graph.js"
Expand Down Expand Up @@ -358,7 +358,7 @@ export class Garden {
this.asyncLock = new AsyncLock()

const gitMode = params.projectConfig.scan?.git?.mode || gardenEnv.GARDEN_GIT_SCAN_MODE
const handlerCls = gitMode === "repo" ? GitRepoHandler : GitHandler
const handlerCls = gitMode === "repo" ? GitRepoHandler : GitSubTreeHandler

this.vcs = new handlerCls({
garden: this,
Expand Down Expand Up @@ -1786,7 +1786,7 @@ export async function resolveGardenParamsPartial(currentDirectory: string, opts:
const treeCache = new TreeCache()

// Note: another VcsHandler is created later, this one is temporary
const gitHandler = new GitHandler({
const gitHandler = new GitSubTreeHandler({
projectRoot,
gardenDirPath,
ignoreFile: defaultConfigFilename,
Expand Down
29 changes: 21 additions & 8 deletions core/src/vcs/git-repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,15 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { augmentGlobs, GitHandler } from "./git.js"
import type { BaseIncludeExcludeFiles, GetFilesParams, IncludeExcludeFilesHandler, VcsFile } from "./vcs.js"
import { AbstractGitHandler, augmentGlobs } from "./git.js"
import { GitSubTreeHandler } from "./git-sub-tree.js"
import type {
BaseIncludeExcludeFiles,
GetFilesParams,
IncludeExcludeFilesHandler,
VcsFile,
VcsHandlerParams,
} from "./vcs.js"
import { isDirectory, matchPath } from "../util/fs.js"
import fsExtra from "fs-extra"
import { pathToCacheContext } from "../cache.js"
Expand Down Expand Up @@ -54,8 +61,14 @@ const getIncludeExcludeFiles: IncludeExcludeFilesHandler<GitRepoGetFilesParams,
}

// @Profile()
export class GitRepoHandler extends GitHandler {
override name = "git-repo"
export class GitRepoHandler extends AbstractGitHandler {
private readonly gitHandlerDelegate: GitSubTreeHandler
override readonly name = "git-repo"

constructor(params: VcsHandlerParams) {
super(params)
this.gitHandlerDelegate = new GitSubTreeHandler(params)
}

/**
* This has the same signature as the GitHandler super class method but instead of scanning the individual directory
Expand Down Expand Up @@ -129,7 +142,7 @@ export class GitRepoHandler extends GitHandler {
return filtered
}

filterPaths({
private filterPaths({
log,
files,
path,
Expand Down Expand Up @@ -162,9 +175,9 @@ export class GitRepoHandler extends GitHandler {
* Scans the given repo root and caches the list of files in the tree cache.
* Uses an async lock to ensure a repo root is only scanned once.
*
* Delegates to {@link GitHandler.getFiles}.
* Delegates to {@link GitSubTreeHandler.getFiles}.
*/
async scanRepo(params: ScanRepoParams): Promise<FileTree> {
private async scanRepo(params: ScanRepoParams): Promise<FileTree> {
const { log, path } = params

const key = ["git-repo-files", path]
Expand All @@ -186,7 +199,7 @@ export class GitRepoHandler extends GitHandler {
}

log.silly(() => `Scanning repository at ${path}`)
const files = await super.getFiles({ ...params, scanRoot: undefined })
const files = await this.gitHandlerDelegate.getFiles({ ...params, scanRoot: undefined })

const fileTree = FileTree.fromFiles(files)

Expand Down
Loading

0 comments on commit 746f038

Please sign in to comment.