Skip to content

simplify package collections model #3069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 19, 2020
Merged

simplify package collections model #3069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
347880c
simplify package collections model
tomerd Nov 19, 2020
a1c911e
fixup
tomerd Nov 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Sources/PackageCollections/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ add_library(PackageCollections
Providers/JSONPackageCollectionProvider.swift
Providers/PackageCollectionProvider.swift
Providers/PackageMetadataProvider.swift
Storage/FilePackageCollectionsSourcesStorage.swift
Storage/PackageCollectionsSourcesStorage.swift
Storage/PackageCollectionsStorage.swift
Storage/SQLitePackageCollectionsStorage.swift
API.swift
PackageCollections.swift
PackageCollections+Configuration.swift
Expand Down
71 changes: 0 additions & 71 deletions Sources/PackageCollections/Model/Collection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,74 +141,3 @@ extension PackageCollectionsModel.CollectionIdentifier: Codable {
}
}
}

// FIXME: add minimumPlatformVersions
extension PackageCollectionsModel.Collection {
/// A representation of package metadata
public struct Package: Equatable, Codable {
public typealias Version = PackageVersion

/// Package reference
public let reference: PackageReference

/// Package's repository address
public let repository: RepositorySpecifier

/// A summary about the package
public let summary: String?

/// Published versions of the package
public let versions: [Version]

/// URL of the package's README
public let readmeURL: URL?

/// Initializes a `Package`
init(
repository: RepositorySpecifier,
summary: String?,
versions: [Version],
readmeURL: URL?
) {
self.reference = .init(repository: repository)
self.repository = repository
self.summary = summary
self.versions = versions
self.readmeURL = readmeURL
}
}
}

extension PackageCollectionsModel.Collection {
/// A representation of package version
public struct PackageVersion: Equatable, Codable {
public typealias Target = PackageCollectionsModel.PackageTarget
public typealias Product = PackageCollectionsModel.PackageProduct

/// The version
public let version: TSCUtility.Version

/// The package name
public let packageName: String

// Custom instead of `PackageModel.Target` because we don't need the additional details
/// The package version's targets
public let targets: [Target]

// Custom instead of `PackageModel.Product` because of the simplified `Target`
/// The package version's products
public let products: [Product]

/// The package version's Swift tools version
public let toolsVersion: ToolsVersion

/// The package version's supported platforms verified to work
public let verifiedPlatforms: [PackageModel.Platform]?

/// The package version's Swift versions verified to work
public let verifiedSwiftVersions: [SwiftLanguageVersion]?

/// The package version's license
public let license: PackageCollectionsModel.License?
}
}
19 changes: 12 additions & 7 deletions Sources/PackageCollections/Model/Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import SourceControl

extension PackageCollectionsModel {
/// Package metadata
public struct Package: Equatable {
public struct Package: Codable, Equatable {
/// Package reference
public let reference: PackageReference

Expand All @@ -25,6 +25,9 @@ extension PackageCollectionsModel {
/// Package description
public let description: String?

/// Keywords for the package
public let keywords: [String]?

/// Published versions of the package
public let versions: [Version]

Expand Down Expand Up @@ -67,6 +70,7 @@ extension PackageCollectionsModel {
init(
repository: RepositorySpecifier,
description: String?,
keywords: [String]?,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a semantic distinction to an empty vs a nil collection of keywords, or is it mainly to avoid a coded entry for an empty collection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the latter

versions: [Version],
latestVersion: Version?,
watchersCount: Int?,
Expand All @@ -76,6 +80,7 @@ extension PackageCollectionsModel {
self.reference = .init(repository: repository)
self.repository = repository
self.description = description
self.keywords = keywords
self.versions = versions
self.latestVersion = latestVersion
self.watchersCount = watchersCount
Expand All @@ -88,9 +93,9 @@ extension PackageCollectionsModel {
// FIXME: add minimumPlatformVersions
extension PackageCollectionsModel.Package {
/// A representation of package version
public struct Version: Equatable {
public typealias Target = PackageCollectionsModel.PackageTarget
public typealias Product = PackageCollectionsModel.PackageProduct
public struct Version: Codable, Equatable {
public typealias Target = PackageCollectionsModel.Target
public typealias Product = PackageCollectionsModel.Product

/// The version
public let version: TSCUtility.Version
Expand Down Expand Up @@ -122,7 +127,7 @@ extension PackageCollectionsModel.Package {

extension PackageCollectionsModel {
/// A representation of package target
public struct PackageTarget: Equatable, Hashable, Codable {
public struct Target: Equatable, Hashable, Codable {
/// The target name
public let name: String

Expand All @@ -133,15 +138,15 @@ extension PackageCollectionsModel {

extension PackageCollectionsModel {
/// A representation of package product
public struct PackageProduct: Equatable, Codable {
public struct Product: Equatable, Codable {
/// The product name
public let name: String

/// The product type
public let type: ProductType

/// The product's targets
public let targets: [PackageTarget]
public let targets: [Target]
}
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageCollections/Model/Search.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ extension PackageCollectionsModel {
public struct Item {
// Merged package metadata from across collections
/// The matching package
public let package: PackageCollectionsModel.Collection.Package
public let package: PackageCollectionsModel.Package

/// Package collections that contain the package
public let collections: [PackageCollectionsModel.CollectionIdentifier]
Expand Down
1 change: 0 additions & 1 deletion Sources/PackageCollections/Model/TargetListResult.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ extension PackageCollectionsModel {
public typealias TargetListResult = [TargetListItem]

public struct TargetListItem {
public typealias Target = PackageCollectionsModel.PackageTarget
public typealias Package = PackageCollectionsModel.TargetListResult.Package

/// Target
Expand Down
41 changes: 33 additions & 8 deletions Sources/PackageCollections/PackageCollections.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,44 @@ import TSCBasic
// TODO: is there a better name? this conflicts with the module name which is okay in this case but not ideal in Swift
public struct PackageCollections: PackageCollectionsProtocol {
private let configuration: Configuration
private let storage: Storage
private let storageContainer: (storage: Storage, owned: Bool)
private let collectionProviders: [PackageCollectionsModel.CollectionSourceType: PackageCollectionProvider]
private let metadataProvider: PackageMetadataProvider

init(configuration: Configuration,
private var storage: Storage {
self.storageContainer.storage
}

// initialize with defaults
public init(configuration: Configuration = .init()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 @neonichu

let storage = Storage(sources: FilePackageCollectionsSourcesStorage(),
collections: SQLitePackageCollectionsStorage())
let collectionProviders = [PackageCollectionsModel.CollectionSourceType.json: JSONPackageCollectionProvider()]
let metadataProvider = GitHubPackageMetadataProvider()

self.configuration = configuration
self.storageContainer = (storage, true)
self.collectionProviders = collectionProviders
self.metadataProvider = metadataProvider
}

// internal initializer for testing
init(configuration: Configuration = .init(),
storage: Storage,
collectionProviders: [PackageCollectionsModel.CollectionSourceType: PackageCollectionProvider],
metadataProvider: PackageMetadataProvider) {
self.configuration = configuration
self.storage = storage
self.storageContainer = (storage, false)
self.collectionProviders = collectionProviders
self.metadataProvider = metadataProvider
}

public func shutdown() throws {
if self.storageContainer.owned {
try self.storageContainer.storage.close()
}
}

// MARK: - Collections

public func listCollections(identifiers: Set<PackageCollectionsModel.CollectionIdentifier>? = nil,
Expand Down Expand Up @@ -279,8 +303,8 @@ public struct PackageCollections: PackageCollectionsProtocol {
}

private func targetListResultFromCollections(_ collections: [PackageCollectionsModel.Collection]) -> PackageCollectionsModel.TargetListResult {
var packageCollections = [PackageReference: (package: PackageCollectionsModel.Collection.Package, collections: Set<PackageCollectionsModel.CollectionIdentifier>)]()
var targetsPackages = [String: (target: PackageCollectionsModel.PackageTarget, packages: Set<PackageReference>)]()
var packageCollections = [PackageReference: (package: PackageCollectionsModel.Package, collections: Set<PackageCollectionsModel.CollectionIdentifier>)]()
var targetsPackages = [String: (target: PackageCollectionsModel.Target, packages: Set<PackageReference>)]()

collections.forEach { collection in
collection.packages.forEach { package in
Expand All @@ -306,7 +330,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
.map { pair -> PackageCollectionsModel.TargetListResult.Package in
let versions = pair.package.versions.map { PackageCollectionsModel.TargetListResult.PackageVersion(version: $0.version, packageName: $0.packageName) }
return .init(repository: pair.package.repository,
description: pair.package.summary,
description: pair.package.description,
versions: versions,
collections: Array(pair.collections))
}
Expand All @@ -315,7 +339,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
}
}

internal static func mergedPackageMetadata(package: PackageCollectionsModel.Collection.Package,
internal static func mergedPackageMetadata(package: PackageCollectionsModel.Package,
basicMetadata: PackageCollectionsModel.PackageBasicMetadata?) -> PackageCollectionsModel.Package {
var versions = package.versions.map { packageVersion -> PackageCollectionsModel.Package.Version in
.init(version: packageVersion.version,
Expand All @@ -334,7 +358,8 @@ public struct PackageCollections: PackageCollectionsProtocol {

return .init(
repository: package.repository,
description: basicMetadata?.description ?? package.summary,
description: basicMetadata?.description ?? package.description,
keywords: basicMetadata?.keywords ?? package.keywords,
versions: versions,
latestVersion: latestVersion,
watchersCount: basicMetadata?.watchersCount,
Expand Down
16 changes: 11 additions & 5 deletions Sources/PackageCollections/Providers/GitHubPackageMetadataProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {

func get(_ reference: PackageReference, callback: @escaping (Result<PackageCollectionsModel.PackageBasicMetadata, Error>) -> Void) {
guard reference.kind == .remote else {
return callback(.failure(Errors.unprocessable(reference)))
return callback(.failure(Errors.invalidReferenceType(reference)))
}
guard let baseURL = self.apiURL(reference.path) else {
return callback(.failure(Errors.unprocessable(reference)))
return callback(.failure(Errors.invalidGitUrl(reference.path)))
}

let metadataURL = baseURL
Expand All @@ -61,7 +61,9 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
// get the main data
sync.enter()
let options = self.makeRequestOptions(validResponseCodes: [200])
httpClient.get(metadataURL, options: options) { result in
var headers = HTTPClientHeaders()
headers.add(name: "Accept", value: "application/vnd.github.mercy-preview+json")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a literal string or something that should be parameterized?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you asking about constants for the header name?

httpClient.get(metadataURL, headers: headers, options: options) { result in
defer { sync.leave() }
resultsLock.withLock {
results[metadataURL] = result
Expand Down Expand Up @@ -103,6 +105,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {

callback(.success(.init(
description: metadata.description,
keywords: metadata.topics,
versions: tags.compactMap { TSCUtility.Version(string: $0.name) },
watchersCount: metadata.watchersCount,
readmeURL: readme?.downloadURL,
Expand All @@ -127,7 +130,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
let owner = String(url[ownerRange])
let repo = String(url[repoRange])

return URL(string: "https://api.\(host)/\(owner)/\(repo)")
return URL(string: "https://api.\(host)/repos/\(owner)/\(repo)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}
}
return nil
Expand All @@ -154,7 +157,8 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
}

enum Errors: Error, Equatable {
case unprocessable(PackageReference)
case invalidReferenceType(PackageReference)
case invalidGitUrl(String)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: URL

case invalidResponse(URL)
}
}
Expand All @@ -164,6 +168,7 @@ extension GitHubPackageMetadataProvider {
let name: String
let fullName: String
let description: String?
let topics: [String]?
let isPrivate: Bool
let isFork: Bool
let defaultBranch: String
Expand All @@ -181,6 +186,7 @@ extension GitHubPackageMetadataProvider {
case name
case fullName = "full_name"
case description
case topics
case isPrivate = "private"
case isFork = "fork"
case defaultBranch = "default_branch"
Expand Down
20 changes: 12 additions & 8 deletions Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,17 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
return .failure(Errors.invalidJSON(error))
}

let packages = collection.packages.map { package -> PackageCollectionsModel.Collection.Package in
let versions = package.versions.compactMap { version -> PackageCollectionsModel.Collection.Package.Version? in
let packages = collection.packages.map { package -> PackageCollectionsModel.Package in
let versions = package.versions.compactMap { version -> PackageCollectionsModel.Package.Version? in
// note this filters out / ignores missing / bad data in attempt to make the most out of the provided set
guard let parsedVersion = TSCUtility.Version(string: version.version) else {
return nil
}
guard let toolsVersion = ToolsVersion(string: version.toolsVersion) else {
return nil
}
let targets = version.targets.map { PackageCollectionsModel.PackageTarget(name: $0.name, moduleName: $0.moduleName) }
let products = version.products.compactMap { PackageCollectionsModel.PackageProduct(from: $0, packageTargets: targets) }
let targets = version.targets.map { PackageCollectionsModel.Target(name: $0.name, moduleName: $0.moduleName) }
let products = version.products.compactMap { PackageCollectionsModel.Product(from: $0, packageTargets: targets) }
let verifiedPlatforms: [PackageModel.Platform]? = version.verifiedPlatforms?.compactMap { PackageModel.Platform(from: $0) }
let verifiedSwiftVersions = version.verifiedSwiftVersions?.compactMap { SwiftLanguageVersion(string: $0) }
let license = version.license.flatMap { PackageCollectionsModel.License(from: $0) }
Expand All @@ -117,9 +117,13 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
license: license)
}
return .init(repository: RepositorySpecifier(url: package.url),
summary: package.description,
description: package.description,
keywords: package.keywords,
versions: versions,
readmeURL: package.readmeURL.flatMap { Foundation.URL(string: $0) })
latestVersion: versions.first,
watchersCount: nil,
readmeURL: package.readmeURL.flatMap { Foundation.URL(string: $0) },
authors: nil)
}
return .success(.init(source: source,
name: collection.name,
Expand Down Expand Up @@ -227,8 +231,8 @@ extension JSONPackageCollectionProvider.CollectionV1 {

// MARK: - Extensions for mapping from JSON to PackageCollectionsModel

extension PackageCollectionsModel.PackageProduct {
fileprivate init?(from: JSONPackageCollectionProvider.CollectionV1.Product, packageTargets: [PackageCollectionsModel.PackageTarget]) {
extension PackageCollectionsModel.Product {
fileprivate init?(from: JSONPackageCollectionProvider.CollectionV1.Product, packageTargets: [PackageCollectionsModel.Target]) {
let targets = packageTargets.filter { from.targets.map { $0.lowercased() }.contains($0.name.lowercased()) }
self = .init(name: from.name, type: from.type, targets: targets)
}
Expand Down
1 change: 1 addition & 0 deletions Sources/PackageCollections/Providers/PackageMetadataProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ protocol PackageMetadataProvider {
extension PackageCollectionsModel {
struct PackageBasicMetadata: Equatable {
let description: String?
let keywords: [String]?
let versions: [TSCUtility.Version]
let watchersCount: Int?
let readmeURL: Foundation.URL?
Expand Down
Loading