From 2770670759d8d30235779e689c7087364fb406c1 Mon Sep 17 00:00:00 2001 From: Calvin Cestari <calvin.cestari@apollographql.com> Date: Mon, 31 Oct 2022 15:55:00 -0700 Subject: [PATCH 1/5] Improves Glob efficiency --- Sources/ApolloCodegenLib/Glob.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/ApolloCodegenLib/Glob.swift b/Sources/ApolloCodegenLib/Glob.swift index d31d1c6584..0391e36d73 100644 --- a/Sources/ApolloCodegenLib/Glob.swift +++ b/Sources/ApolloCodegenLib/Glob.swift @@ -136,7 +136,7 @@ public struct Glob { } }() - var directories: [String] = [searchURL.path] // include searching the globstar root directory + var directories: [URL] = [searchURL] // include searching the globstar root directory do { let resourceKeys: [URLResourceKey] = [.isDirectoryKey] @@ -164,7 +164,7 @@ public struct Glob { isDirectory == true else { continue } - directories.append(url.path) + directories.append(url) } } catch(let error) { @@ -172,7 +172,7 @@ public struct Glob { } return OrderedSet<String>(directories.compactMap({ directory in - var path = URL(fileURLWithPath: directory).appendingPathComponent(lastPart).standardizedFileURL.path + var path = directory.appendingPathComponent(lastPart).standardizedFileURL.path if isExclude { path.insert("!", at: path.startIndex) } From 5207c4866e74b91cffea38287a670231fb6eaabf Mon Sep 17 00:00:00 2001 From: Calvin Cestari <calvin.cestari@apollographql.com> Date: Mon, 31 Oct 2022 16:25:50 -0700 Subject: [PATCH 2/5] Adds excluded folders to Glob match logic --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 24 +++++-- Sources/ApolloCodegenLib/Glob.swift | 27 +++++-- .../ApolloCodegenTests.swift | 72 +++++++++++++++++++ Tests/ApolloCodegenTests/GlobTests.swift | 48 ++++++------- 4 files changed, 136 insertions(+), 35 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index c838780ea7..bf0041d558 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -183,11 +183,14 @@ public class ApolloCodegen { ) } - private static func createSchema( + static func createSchema( _ config: ConfigurationContext, _ frontend: GraphQLJSFrontend ) throws -> GraphQLSchema { - let matches = try Glob(config.input.schemaSearchPaths, relativeTo: config.rootURL).match() + + let matches = try match( + searchPaths: config.input.schemaSearchPaths, + relativeTo: config.rootURL) guard !matches.isEmpty else { throw Error.cannotLoadSchema @@ -197,12 +200,15 @@ public class ApolloCodegen { return try frontend.loadSchema(from: sources) } - private static func createOperationsDocument( + static func createOperationsDocument( _ config: ConfigurationContext, _ frontend: GraphQLJSFrontend, _ experimentalFeatures: ApolloCodegenConfiguration.ExperimentalFeatures ) throws -> GraphQLDocument { - let matches = try Glob(config.input.operationSearchPaths, relativeTo: config.rootURL).match() + + let matches = try match( + searchPaths: config.input.operationSearchPaths, + relativeTo: config.rootURL) guard !matches.isEmpty else { throw Error.cannotLoadOperations @@ -217,6 +223,16 @@ public class ApolloCodegen { return try frontend.mergeDocuments(documents) } + static func match(searchPaths: [String], relativeTo relativeURL: URL?) throws -> OrderedSet<String> { + let excludedDirectories = [ + ".build", + ".swiftpm", + ".Pods"] + + return try Glob(searchPaths, relativeTo: relativeURL) + .match(excludingDirectories: excludedDirectories) + } + /// Generates Swift files for the compiled schema, ir and configured output structure. static func generateFiles( compilationResult: CompilationResult, diff --git a/Sources/ApolloCodegenLib/Glob.swift b/Sources/ApolloCodegenLib/Glob.swift index 0391e36d73..277f7abc46 100644 --- a/Sources/ApolloCodegenLib/Glob.swift +++ b/Sources/ApolloCodegenLib/Glob.swift @@ -70,8 +70,8 @@ public struct Glob { /// Executes the pattern match on the underlying file system. /// /// - Returns: A set of matched file paths. - func match() throws -> OrderedSet<String> { - let expandedPatterns = try expand(self.patterns) + func match(excludingDirectories excluded: [String]? = nil) throws -> OrderedSet<String> { + let expandedPatterns = try expand(self.patterns, excludingDirectories: excluded) var includeMatches: [String] = [] var excludeMatches: [String] = [] @@ -91,7 +91,10 @@ public struct Glob { } /// Separates a comma-delimited string into paths, expanding any globstars and removes duplicates. - private func expand(_ patterns: [String]) throws -> OrderedSet<String> { + private func expand( + _ patterns: [String], + excludingDirectories excluded: [String]? + ) throws -> OrderedSet<String> { var paths: OrderedSet<String> = [] for pattern in patterns { if pattern.containsExclude && !pattern.isExclude { @@ -101,14 +104,18 @@ public struct Glob { throw MatchError.invalidExclude(path: pattern) } - paths.formUnion(try expandGlobstar(pattern)) + paths.formUnion(try expand(pattern, excludingDirectories: excluded)) } return paths } - /// Expands the globstar (`**`) to find all directory paths to search for the match pattern and removes duplicates. - private func expandGlobstar(_ pattern: String) throws -> OrderedSet<String> { + /// Expands `pattern` including any globstar character (`**`) to find all directory paths to + /// search for the match pattern and removes duplicates. + private func expand( + _ pattern: String, + excludingDirectories excluded: [String]? + ) throws -> OrderedSet<String> { guard pattern.includesGlobstar else { return [URL(fileURLWithPath: pattern, relativeTo: rootURL).path] } @@ -157,11 +164,17 @@ public struct Glob { if let enumeratorError = enumeratorError { throw enumeratorError } + var excludedSet: Set<String> = [] + if let excluded = excluded { + excludedSet = Set<String>(excluded) + } + for case (let url as URL) in enumerator { guard let resourceValues = try? url.resourceValues(forKeys: Set(resourceKeys)), let isDirectory = resourceValues.isDirectory, - isDirectory == true + isDirectory == true, + excludedSet.intersection(url.pathComponents).isEmpty else { continue } directories.append(url) diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index c4bc70770d..161fc3398d 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -12,7 +12,9 @@ class ApolloCodegenTests: XCTestCase { .appendingPathComponent("Codegen") .appendingPathComponent(UUID().uuidString) testFileManager = ApolloFileManager(base: FileManager.default) + try testFileManager.createDirectoryIfNeeded(atPath: directoryURL.path) + testFileManager.base.changeCurrentDirectoryPath(directoryURL.path) } override func tearDownWithError() throws { @@ -2215,4 +2217,74 @@ class ApolloCodegenTests: XCTestCase { .notTo(throwError()) } + // Glob Exclusion Tests + + func test__match__givenFilesInExcludedPaths_shouldNotReturnExcludedPaths() throws { + // given + createFile(filename: "included.file") + + createFile(filename: "excludedBuildFolder.file", inDirectory: ".build") + createFile(filename: "excludedBuildSubfolderOne.file", inDirectory: ".build/subfolder") + createFile(filename: "excludedBuildSubfolderTwo.file", inDirectory: ".build/subfolder/two") + createFile(filename: "excludedNestedOneBuildFolder.file", inDirectory: "nested/.build") + createFile(filename: "excludedNestedTwoBuildFolder.file", inDirectory: "nested/two/.build") + + createFile(filename: "excludedSwiftpmFolder.file", inDirectory: ".swiftpm") + createFile(filename: "excludedSwiftpmSubfolderOne.file", inDirectory: ".swiftpm/subfolder") + createFile(filename: "excludedSwiftpmSubfolderTwo.file", inDirectory: ".swiftpm/subfolder/two") + createFile(filename: "excludedNestedOneSwiftpmFolder.file", inDirectory: "nested/.swiftpm") + createFile(filename: "excludedNestedTwoSwiftpmFolder.file", inDirectory: "nested/two/.swiftpm") + + createFile(filename: "excludedPodsFolder.file", inDirectory: ".Pods") + createFile(filename: "excludedPodsSubfolderOne.file", inDirectory: ".Pods/subfolder") + createFile(filename: "excludedPodsSubfolderTwo.file", inDirectory: ".Pods/subfolder/two") + createFile(filename: "excludedNestedOnePodsFolder.file", inDirectory: "nested/.Pods") + createFile(filename: "excludedNestedTwoPodsFolder.file", inDirectory: "nested/two/.Pods") + + // when + let matches = try ApolloCodegen.match( + searchPaths: ["\(directoryURL.path)/**/*.file"], + relativeTo: nil) + + // then + expect(matches.count).to(equal(1)) + expect(matches.contains(where: { $0.contains(".build") })).to(beFalse()) + expect(matches.contains(where: { $0.contains(".swiftpm") })).to(beFalse()) + expect(matches.contains(where: { $0.contains(".Pods") })).to(beFalse()) + } + + func test__match__givenFilesInExcludedPaths_usingRelativeDirectory_shouldNotReturnExcludedPaths() throws { + // given + createFile(filename: "included.file") + + createFile(filename: "excludedBuildFolder.file", inDirectory: ".build") + createFile(filename: "excludedBuildSubfolderOne.file", inDirectory: ".build/subfolder") + createFile(filename: "excludedBuildSubfolderTwo.file", inDirectory: ".build/subfolder/two") + createFile(filename: "excludedNestedOneBuildFolder.file", inDirectory: "nested/.build") + createFile(filename: "excludedNestedTwoBuildFolder.file", inDirectory: "nested/two/.build") + + createFile(filename: "excludedSwiftpmFolder.file", inDirectory: ".swiftpm") + createFile(filename: "excludedSwiftpmSubfolderOne.file", inDirectory: ".swiftpm/subfolder") + createFile(filename: "excludedSwiftpmSubfolderTwo.file", inDirectory: ".swiftpm/subfolder/two") + createFile(filename: "excludedNestedOneSwiftpmFolder.file", inDirectory: "nested/.swiftpm") + createFile(filename: "excludedNestedTwoSwiftpmFolder.file", inDirectory: "nested/two/.swiftpm") + + createFile(filename: "excludedPodsFolder.file", inDirectory: ".Pods") + createFile(filename: "excludedPodsSubfolderOne.file", inDirectory: ".Pods/subfolder") + createFile(filename: "excludedPodsSubfolderTwo.file", inDirectory: ".Pods/subfolder/two") + createFile(filename: "excludedNestedOnePodsFolder.file", inDirectory: "nested/.Pods") + createFile(filename: "excludedNestedTwoPodsFolder.file", inDirectory: "nested/two/.Pods") + + // when + let matches = try ApolloCodegen.match( + searchPaths: ["**/*.file"], + relativeTo: directoryURL) + + // then + expect(matches.count).to(equal(1)) + expect(matches.contains(where: { $0.contains(".build") })).to(beFalse()) + expect(matches.contains(where: { $0.contains(".swiftpm") })).to(beFalse()) + expect(matches.contains(where: { $0.contains(".Pods") })).to(beFalse()) + } + } diff --git a/Tests/ApolloCodegenTests/GlobTests.swift b/Tests/ApolloCodegenTests/GlobTests.swift index a43e779f11..47de184601 100644 --- a/Tests/ApolloCodegenTests/GlobTests.swift +++ b/Tests/ApolloCodegenTests/GlobTests.swift @@ -65,7 +65,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("file.one").path ])) } @@ -83,7 +83,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("file.two").path, baseURL.appendingPathComponent("file.one").path, ])) @@ -102,7 +102,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("file.one").path ])) } @@ -120,7 +120,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("other/file.oye").path, baseURL.appendingPathComponent("other/file.one").path, ])) @@ -143,7 +143,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("a/file.one").path ])) } @@ -168,7 +168,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("a/file.one").path, baseURL.appendingPathComponent("other/file.oye").path, baseURL.appendingPathComponent("other/file.one").path, @@ -192,7 +192,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("a/file.one").path ])) } @@ -214,7 +214,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("a/file.one").path, baseURL.appendingPathComponent("other/file.oye").path, baseURL.appendingPathComponent("other/file.one").path, @@ -233,7 +233,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("file.one").path ])) } @@ -256,7 +256,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("file.two").path, baseURL.appendingPathComponent("file.one").path, baseURL.appendingPathComponent("other/file.oye").path, @@ -277,7 +277,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("a/b/c/d/e/f/file.one").path ])) } @@ -296,7 +296,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("a/b/c/d/file.one").path, baseURL.appendingPathComponent("a/b/c/d/e/f/file.two").path, baseURL.appendingPathComponent("a/b/c/d/e/f/file.one").path, @@ -316,7 +316,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("a/b/c/d/e/f/file.one").path ])) } @@ -335,7 +335,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("a/b/c/d/file.two").path, baseURL.appendingPathComponent("a/b/c/d/e/f/file.two").path, baseURL.appendingPathComponent("a/b/c/d/e/f/file.one").path, @@ -347,7 +347,7 @@ class GlobTests: XCTestCase { let pattern = baseURL.appendingPathComponent("a/b/c/d/**/!file.swift").path // then - expect(Glob([pattern]).match).to(throwError(Glob.MatchError.invalidExclude(path: pattern))) + expect(try Glob([pattern]).match()).to(throwError(Glob.MatchError.invalidExclude(path: pattern))) } func test_match_givenGlobstarPattern_usingPathExclude_whenMultipleMatch_shouldExclude() throws { @@ -367,7 +367,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("a/b/c/d/e/f/file.ext").path, baseURL.appendingPathComponent("a/b/c/d/e/f/file.one").path, ])) @@ -393,7 +393,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("file.one").path, baseURL.appendingPathComponent("other/file.one").path, baseURL.appendingPathComponent("a/file.one").path, @@ -423,7 +423,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("a/file.one").path, baseURL.appendingPathComponent("a/b/file.one").path, baseURL.appendingPathComponent("a/b/c/file.one").path, @@ -451,7 +451,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern).match).to(equal([ + expect(try Glob(pattern).match()).to(equal([ baseURL.appendingPathComponent("file.one").path, baseURL.appendingPathComponent("other/file.one").path, baseURL.appendingPathComponent("a/file.one").path, @@ -481,7 +481,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern, relativeTo: rootURL).match).to(equal([ + expect(try Glob(pattern, relativeTo: rootURL).match()).to(equal([ baseURL.appendingPathComponent("a/file.one").path, baseURL.appendingPathComponent("a/b/file.one").path, baseURL.appendingPathComponent("a/b/c/file.one").path, @@ -509,7 +509,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern, relativeTo: rootURL).match).to(equal([ + expect(try Glob(pattern, relativeTo: rootURL).match()).to(equal([ baseURL.appendingPathComponent("a/file.one").path, baseURL.appendingPathComponent("a/b/file.one").path, baseURL.appendingPathComponent("a/b/c/file.one").path, @@ -530,7 +530,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern, relativeTo: rootURL).match).to(equal([ + expect(try Glob(pattern, relativeTo: rootURL).match()).to(equal([ baseURL.appendingPathComponent("file.one").path, ])) } @@ -556,7 +556,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob(pattern, relativeTo: rootURL).match).to(equal([ + expect(try Glob(pattern, relativeTo: rootURL).match()).to(equal([ baseURL.appendingPathComponent("file.one").path, baseURL.appendingPathComponent("other/file.one").path, baseURL.appendingPathComponent("a/file.one").path, @@ -579,7 +579,7 @@ class GlobTests: XCTestCase { ]) // then - expect(Glob([pattern]).match).to(equal([ + expect(try Glob([pattern]).match()).to(equal([ baseURL.appendingPathComponent("other/file.xyz").path ])) } From 5e4baaf487e24d65b155bf8beb5e5fde888504be Mon Sep 17 00:00:00 2001 From: Calvin Cestari <calvin.cestari@apollographql.com> Date: Mon, 31 Oct 2022 16:59:25 -0700 Subject: [PATCH 3/5] Adds Glob match directory exclusion tests --- Tests/ApolloCodegenTests/GlobTests.swift | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Tests/ApolloCodegenTests/GlobTests.swift b/Tests/ApolloCodegenTests/GlobTests.swift index 47de184601..bf57dc46a3 100644 --- a/Tests/ApolloCodegenTests/GlobTests.swift +++ b/Tests/ApolloCodegenTests/GlobTests.swift @@ -584,4 +584,25 @@ class GlobTests: XCTestCase { ])) } + func test_match_givenExcludedDirectories_shouldNotMatchExcludedFiles() throws { + // given + let pattern = baseURL.appendingPathComponent("**/file.xyz").path + + // when + try create(files: [ + baseURL.appendingPathComponent("file.xyz").path, + baseURL.appendingPathComponent("nested/file.xyz").path, + baseURL.appendingPathComponent("nested/two/file.xyz").path, + baseURL.appendingPathComponent("DoNotInclude/file.xyz").path, + baseURL.appendingPathComponent("nested/DoNotInclude/file.xyz").path + ]) + + // then + expect(try Glob([pattern]).match(excludingDirectories: ["DoNotInclude"])).to(equal([ + baseURL.appendingPathComponent("file.xyz").path, + baseURL.appendingPathComponent("nested/file.xyz").path, + baseURL.appendingPathComponent("nested/two/file.xyz").path + ])) + } + } From 73641dc5aa1282e928bab50763aef858a800bb3c Mon Sep 17 00:00:00 2001 From: Calvin Cestari <calvin.cestari@apollographql.com> Date: Mon, 31 Oct 2022 16:59:44 -0700 Subject: [PATCH 4/5] Fixes parallelization of Glob tests --- Tests/ApolloCodegenTests/GlobTests.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/ApolloCodegenTests/GlobTests.swift b/Tests/ApolloCodegenTests/GlobTests.swift index bf57dc46a3..f47c54dac8 100644 --- a/Tests/ApolloCodegenTests/GlobTests.swift +++ b/Tests/ApolloCodegenTests/GlobTests.swift @@ -4,7 +4,7 @@ import Nimble import ApolloCodegenInternalTestHelpers class GlobTests: XCTestCase { - let baseURL = CodegenTestHelper.outputFolderURL().appendingPathComponent("Glob") + var baseURL: URL! let fileManager = ApolloFileManager.default // MARK: Setup @@ -12,6 +12,7 @@ class GlobTests: XCTestCase { override func setUpWithError() throws { try super.setUpWithError() + baseURL = CodegenTestHelper.outputFolderURL().appendingPathComponent("Glob/\(UUID().uuidString)") try fileManager.createDirectoryIfNeeded(atPath: baseURL.path) } From 52f13a06b906b06ebb79a3484960672db884f920 Mon Sep 17 00:00:00 2001 From: Calvin Cestari <calvin.cestari@apollographql.com> Date: Mon, 31 Oct 2022 17:21:17 -0700 Subject: [PATCH 5/5] Renames tests --- Tests/ApolloCodegenTests/ApolloCodegenTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index 161fc3398d..301581786e 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -2217,9 +2217,9 @@ class ApolloCodegenTests: XCTestCase { .notTo(throwError()) } - // Glob Exclusion Tests + // Special-folder Exclusion Tests - func test__match__givenFilesInExcludedPaths_shouldNotReturnExcludedPaths() throws { + func test__match__givenFilesInSpecialExcludedPaths_shouldNotReturnExcludedPaths() throws { // given createFile(filename: "included.file") @@ -2253,7 +2253,7 @@ class ApolloCodegenTests: XCTestCase { expect(matches.contains(where: { $0.contains(".Pods") })).to(beFalse()) } - func test__match__givenFilesInExcludedPaths_usingRelativeDirectory_shouldNotReturnExcludedPaths() throws { + func test__match__givenFilesInSpecialExcludedPaths_usingRelativeDirectory_shouldNotReturnExcludedPaths() throws { // given createFile(filename: "included.file")