Move out to a seperate method

Author: Ucanbarlic

Sources/App/Commands/Ingest.swift

@@ -124,13 +124,22 @@ func ingest(client: Client,
                         Current.logger().warning("storeS3Readme failed")
                         s3Readme = .error("\(error)")
                     }
+                    
+                    let fork: Fork?
+                    do {
+                        fork = try await getFork(on: database, metadata: metadata)
+                    } catch {
+                        fork = nil
+                        Current.logger().warning("updating forked from failed")
+                    }
 
                     try await updateRepository(on: database,
                                                for: repo,
                                                metadata: metadata,
                                                licenseInfo: license,
                                                readmeInfo: readme,
-                                               s3Readme: s3Readme)
+                                               s3Readme: s3Readme,
+                                               fork: fork)
                     return pkg
                 }
 
@@ -177,29 +186,15 @@ func updateRepository(on database: Database,
                       metadata: Github.Metadata,
                       licenseInfo: Github.License?,
                       readmeInfo: Github.Readme?,
-                      s3Readme: S3Readme?) async throws {
+                      s3Readme: S3Readme?,
+                      fork: Fork? = nil) async throws {
     guard let repoMetadata = metadata.repository else {
         if repository.$package.value == nil {
             try await repository.$package.load(on: database)
         }
         throw AppError.genericError(repository.package.id,
                                     "repository metadata is nil for package \(repository.name ?? "unknown")")
     }
-    
-    var fork: Fork?
-    do {
-        if let parentUrl = metadata.repository?.normalizedParentUrl {
-            if let packageId = try await Package.query(on: database)
-                .filter(\.$url, .custom("ilike"), parentUrl)
-                .first()?.id {
-                fork = .parentId(packageId)
-            } else {
-                fork = .parentURL(parentUrl)
-            }
-        }
-    } catch {
-        Current.logger().warning("updating forked from failed")
-    }
 
     repository.defaultBranch = repoMetadata.defaultBranch
     repository.forks = repoMetadata.forkCount
@@ -228,6 +223,20 @@ func updateRepository(on database: Database,
     try await repository.save(on: database)
 }
 
+func getFork(on database: Database, metadata: Github.Metadata) async throws -> Fork? {
+    guard let url = metadata.repository?.parent?.url,
+          let parentUrl = URL(string: url)?.normalizedParent?.absoluteString else {
+        return nil
+    }
+
+    if let packageId = try await Package.query(on: database)
+        .filter(\.$url, .custom("ilike"), parentUrl)
+        .first()?.id {
+        return .parentId(packageId)
+    } else {
+        return .parentURL(parentUrl)
+    }
+}
 
 // Helper to ensure the canonical source for these critical fields is the same in all the places where we need them
 private extension Github.Metadata {
@@ -241,12 +250,11 @@ private extension Github.Metadata.Repository {
     var repositoryName: String? { name }
 }
 
-private extension Github.Metadata.Repository {
-    // Returns a normalized version of the URL. Adding a `.git` if not present.
-    var normalizedParentUrl: String? {
-        guard let url = parent?.url else { return nil }
-        guard !url.hasSuffix(".git") else { return url }
-        let normalizedUrl = url + ".git"
-        return normalizedUrl
+private extension URL {
+    var normalizedParent: Self? {
+        guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { return nil }
+        if components.scheme == "http" { components.scheme = "https" }
+        if !components.path.hasSuffix(".git") { components.path = components.path + ".git" }
+        return components.url!
     }
 }

Tests/AppTests/IngestorTests.swift

@@ -156,7 +156,8 @@ class IngestorTests: AppTestCase {
                                                      html: "readme html",
                                                      htmlUrl: "readme html url",
                                                      imagesToCache: []),
-                                   s3Readme: .cached(s3ObjectUrl: "url", githubEtag: "etag"))
+                                   s3Readme: .cached(s3ObjectUrl: "url", githubEtag: "etag"),
+                                   fork: .parentURL("https://github.com/foo/bar.git"))
 
         // validate
         do {
@@ -165,6 +166,7 @@ class IngestorTests: AppTestCase {
             let repo = try await Repository.query(on: app.db).first().unwrap()
             XCTAssertEqual(repo.defaultBranch, "main")
             XCTAssertEqual(repo.forks, 1)
+            XCTAssertEqual(repo.forkedFrom, .parentURL("https://github.com/foo/bar.git"))
             XCTAssertEqual(repo.fundingLinks, [
                 .init(platform: .gitHub, url: "https://github.com/username"),
                 .init(platform: .customUrl, url: "https://example.com/username1"),
@@ -598,91 +600,40 @@ class IngestorTests: AppTestCase {
         XCTAssertEqual(postMigrationFetchedRepo.s3Readme, .cached(s3ObjectUrl: "object-url", githubEtag: ""))
     }
 
-    func test_ingest_storeForkedFromPackageInSPI() async throws {
-        let pkg = Package(url: "https://github.com/foo/bar.git".url,
-                          processingStage: .analysis)
-        let forkedPkg = Package(
-            url: "https://github.com/taz/bar.git",
-            processingStage: .reconciliation
-        )
-        try await pkg.save(on: app.db)
-        try await forkedPkg.save(on: app.db)
-        Current.fetchMetadata = { _, owner, repository in
-                .mock(owner: owner, repository: repository, parentUrl: "https://github.com/foo/bar.git")
-        }
+    func test_getFork() async throws {
+        try await Package(id: .id0, url: "https://github.com/foo/parent.git".url, processingStage: .analysis).save(on: app.db)
+        try await Package(url: "https://github.com/bar/forked.git", processingStage: .analysis).save(on: app.db)
 
-        // MUT
-        try await ingest(client: app.client, database: app.db, mode: .limit(1))
-        
-        guard let forkedId = forkedPkg.id else {
-            XCTFail("Failed to get forked package id")
-            return
+        var metadata = Github.Metadata.mock
+
+        do {  // test lookup when package is in the index
+            metadata.repository?.parent = .init(url: "https://github.com/foo/parent.git")
+            let fork = try await getFork(on: app.db, metadata: metadata)
+            XCTAssertEqual(fork, .parentId(.id0))
         }
-        
-        guard let id = pkg.id else {
-            XCTFail("Failed to get package id")
-            return
+
+        do {  // test lookup when package is in the index but with different case in URL
+            metadata.repository?.parent = .init(url: "https://github.com/Foo/Parent.git")
+            let fork = try await getFork(on: app.db, metadata: metadata)
+            XCTAssertEqual(fork, .parentId(.id0))
         }
 
-        let repo = try await Repository
-            .query(on: app.db)
-            .filter(\Repository.$package.$id == forkedId).first()
-        
-        XCTAssertNotNil(repo?.forkedFrom)
-        
-        XCTAssertEqual(repo?.forkedFrom, .parentId(id))
-        
-    }
-    
-    func test_ingest_storeForkedFromPackageNotInSPI() async throws {
-        let forkedPkg = Package(
-            url: "https://github.com/taz/bar.git",
-            processingStage: .reconciliation
-        )
-        try await forkedPkg.save(on: app.db)
-        Current.fetchMetadata = { _, owner, repository in 
-            .mock(owner: owner, repository: repository, parentUrl: "https://github.com/foo/bar.git")
+        do {  // test whem metadata repo url doesn't have `.git` at end
+            metadata.repository?.parent = .init(url: "https://github.com/Foo/Parent")
+            let fork = try await getFork(on: app.db, metadata: metadata)
+            XCTAssertEqual(fork, .parentId(.id0))
         }
 
-        // MUT
-        try await ingest(client: app.client, database: app.db, mode: .limit(1))
-        
-        guard let forkedId = forkedPkg.id else {
-            XCTFail("Failed to get forked package id")
-            return
-        }
-        
-        let repo = try await Repository
-            .query(on: app.db)
-            .filter(\Repository.$package.$id == forkedId).first()
-        
-        XCTAssertNotNil(repo?.forkedFrom)
-        
-        XCTAssertEqual(repo?.forkedFrom, .parentURL("https://github.com/foo/bar.git"))
-    }
-    
-    func test_ingest_storeForkedFromShouldNeNil() async throws {
-        let forkedPkg = Package(
-            url: "https://github.com/taz/bar.git",
-            processingStage: .reconciliation
-        )
-        try await forkedPkg.save(on: app.db)
-        Current.fetchMetadata = { _, owner, repository in
-                .mock(owner: owner, repository: repository, parentUrl: nil)
+        do {  // test lookup when package is not in the index
+            metadata.repository?.parent = .init(url: "https://github.com/some/other.git")
+            let fork = try await getFork(on: app.db, metadata: metadata)
+            XCTAssertEqual(fork, .parentURL("https://github.com/some/other.git"))
         }
 
-        // MUT
-        try await ingest(client: app.client, database: app.db, mode: .limit(1))
-        
-        guard let forkedId = forkedPkg.id else {
-            XCTFail("Failed to get forked package id")
-            return
+        do {  // test lookup when parent url is nil
+            metadata.repository?.parent = .init(url: nil)
+            let fork = try await getFork(on: app.db, metadata: metadata)
+            XCTAssertEqual(fork, nil)
         }
-        
-        let repo = try await Repository
-            .query(on: app.db)
-            .filter(\Repository.$package.$id == forkedId).first()
-        
-        XCTAssertNil(repo?.forkedFrom)
     }
 }