Skip to content

package collections small refactoring #3072

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.

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 1 commit into from
Nov 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion Sources/PackageCollections/Model/Collection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import TSCUtility

public enum PackageCollectionsModel {}

// make things less verbose internally
internal typealias Model = PackageCollectionsModel

extension PackageCollectionsModel {
/// A `Collection` is a collection of packages.
public struct Collection: Equatable, Codable {
Expand Down Expand Up @@ -46,7 +49,7 @@ extension PackageCollectionsModel {

/// Who authored this collection
public let createdBy: Author?

/// When this collection was last processed locally
public let lastProcessedAt: Date

Expand Down
2 changes: 1 addition & 1 deletion Sources/PackageCollections/Model/Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ extension PackageCollectionsModel.Package {

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

/// The package version's supported platforms
public let minimumPlatformVersions: [SupportedPlatform]?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import TSCBasic

extension PackageCollectionsModel.CollectionSource {
extension Model.CollectionSource {
func validate() -> [ValidationError]? {
var errors: [ValidationError]?
let appendError = { (error: ValidationError) in
Expand Down
42 changes: 21 additions & 21 deletions Sources/PackageCollections/PackageCollections.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import TSCBasic
public struct PackageCollections: PackageCollectionsProtocol {
private let configuration: Configuration
private let storageContainer: (storage: Storage, owned: Bool)
private let collectionProviders: [PackageCollectionsModel.CollectionSourceType: PackageCollectionProvider]
private let collectionProviders: [Model.CollectionSourceType: PackageCollectionProvider]
private let metadataProvider: PackageMetadataProvider

private var storage: Storage {
Expand All @@ -26,7 +26,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
public init(configuration: Configuration = .init()) {
let storage = Storage(sources: FilePackageCollectionsSourcesStorage(),
collections: SQLitePackageCollectionsStorage())
let collectionProviders = [PackageCollectionsModel.CollectionSourceType.json: JSONPackageCollectionProvider()]
let collectionProviders = [Model.CollectionSourceType.json: JSONPackageCollectionProvider()]
let metadataProvider = GitHubPackageMetadataProvider()

self.configuration = configuration
Expand All @@ -38,7 +38,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
// internal initializer for testing
init(configuration: Configuration = .init(),
storage: Storage,
collectionProviders: [PackageCollectionsModel.CollectionSourceType: PackageCollectionProvider],
collectionProviders: [Model.CollectionSourceType: PackageCollectionProvider],
metadataProvider: PackageMetadataProvider) {
self.configuration = configuration
self.storageContainer = (storage, false)
Expand All @@ -65,7 +65,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
if identifiers.isEmpty {
return callback(.success([]))
}
let collectionOrder = identifiers.enumerated().reduce([PackageCollectionsModel.CollectionIdentifier: Int]()) { partial, element in
let collectionOrder = identifiers.enumerated().reduce([Model.CollectionIdentifier: Int]()) { partial, element in
var dictionary = partial
dictionary[element.element] = element.offset
return dictionary
Expand Down Expand Up @@ -94,7 +94,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
return callback(.success([]))
}
let lock = Lock()
var refreshResults = [Result<PackageCollectionsModel.Collection, Error>]()
var refreshResults = [Result<Model.Collection, Error>]()
sources.forEach { source in
self.refreshCollectionFromSource(source: source) { refreshResult in
lock.withLock { refreshResults.append(refreshResult) }
Expand Down Expand Up @@ -181,7 +181,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
case .success(let sources):
let identifiers = sources.map { .init(from: $0) }.filter { collections?.contains($0) ?? true }
if identifiers.isEmpty {
return callback(.success(PackageCollectionsModel.PackageSearchResult(items: [])))
return callback(.success(Model.PackageSearchResult(items: [])))
}
self.storage.collections.searchPackages(identifiers: identifiers, query: query, callback: callback)
}
Expand All @@ -202,7 +202,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
self.metadataProvider.get(reference) { result in
switch result {
case .failure(let error) where error is NotFoundError:
let metadata = PackageCollectionsModel.PackageMetadata(
let metadata = Model.PackageMetadata(
package: Self.mergedPackageMetadata(package: packageSearchResult.package, basicMetadata: nil),
collections: packageSearchResult.collections
)
Expand All @@ -211,7 +211,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
callback(.failure(error))
case .success(let basicMetadata):
// finally merge the results
let metadata = PackageCollectionsModel.PackageMetadata(
let metadata = Model.PackageMetadata(
package: Self.mergedPackageMetadata(package: packageSearchResult.package, basicMetadata: basicMetadata),
collections: packageSearchResult.collections
)
Expand Down Expand Up @@ -267,7 +267,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
// This helps avoid network access in normal operations
private func refreshCollectionFromSource(source: PackageCollectionsModel.CollectionSource,
order _: Int? = nil,
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
callback: @escaping (Result<Model.Collection, Error>) -> Void) {
if let errors = source.validate() {
return callback(.failure(MultipleErrors(errors)))
}
Expand All @@ -293,7 +293,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
case .failure(let error):
callback(.failure(error))
case .success(let sources):
let identifiers = sources.map { PackageCollectionsModel.CollectionIdentifier(from: $0) }
let identifiers = sources.map { Model.CollectionIdentifier(from: $0) }
if identifiers.isEmpty {
return callback(.failure(NotFoundError("\(identifier)")))
}
Expand All @@ -302,9 +302,9 @@ public struct PackageCollections: PackageCollectionsProtocol {
}
}

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

collections.forEach { collection in
collection.packages.forEach { package in
Expand All @@ -327,21 +327,21 @@ public struct PackageCollections: PackageCollectionsProtocol {
return targetsPackages.map { _, pair in
let targetPackages = pair.packages
.compactMap { packageCollections[$0] }
.map { pair -> PackageCollectionsModel.TargetListResult.Package in
let versions = pair.package.versions.map { PackageCollectionsModel.TargetListResult.PackageVersion(version: $0.version, packageName: $0.packageName) }
.map { pair -> Model.TargetListResult.Package in
let versions = pair.package.versions.map { Model.TargetListResult.PackageVersion(version: $0.version, packageName: $0.packageName) }
return .init(repository: pair.package.repository,
summary: pair.package.summary,
versions: versions,
collections: Array(pair.collections))
}

return PackageCollectionsModel.TargetListItem(target: pair.target, packages: targetPackages)
return Model.TargetListItem(target: pair.target, packages: targetPackages)
}
}

internal static func mergedPackageMetadata(package: PackageCollectionsModel.Package,
basicMetadata: PackageCollectionsModel.PackageBasicMetadata?) -> PackageCollectionsModel.Package {
var versions = package.versions.map { packageVersion -> PackageCollectionsModel.Package.Version in
internal static func mergedPackageMetadata(package: Model.Package,
basicMetadata: Model.PackageBasicMetadata?) -> Model.Package {
var versions = package.versions.map { packageVersion -> Model.Package.Version in
.init(version: packageVersion.version,
packageName: packageVersion.packageName,
targets: packageVersion.targets,
Expand Down Expand Up @@ -371,9 +371,9 @@ public struct PackageCollections: PackageCollectionsProtocol {
}

private struct UnknownProvider: Error {
let sourceType: PackageCollectionsModel.CollectionSourceType
let sourceType: Model.CollectionSourceType

init(_ sourceType: PackageCollectionsModel.CollectionSourceType) {
init(_ sourceType: Model.CollectionSourceType) {
self.sourceType = sourceType
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ import TSCBasic

struct GitHubPackageMetadataProvider: PackageMetadataProvider {
let httpClient: HTTPClient
let defaultHttpClient: Bool
let decoder: JSONDecoder
let queue: DispatchQueue

init(httpClient: HTTPClient? = nil) {
self.httpClient = httpClient ?? .init()
self.defaultHttpClient = httpClient == nil
self.httpClient = httpClient ?? Self.makeDefaultHTTPClient()
self.decoder = JSONDecoder()
#if os(Linux) || os(Windows)
self.decoder.dateDecodingStrategy = .iso8601
Expand All @@ -40,7 +38,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
self.queue = DispatchQueue(label: "org.swift.swiftpm.GitHubPackageMetadataProvider", attributes: .concurrent)
}

func get(_ reference: PackageReference, callback: @escaping (Result<PackageCollectionsModel.PackageBasicMetadata, Error>) -> Void) {
func get(_ reference: PackageReference, callback: @escaping (Result<Model.PackageBasicMetadata, Error>) -> Void) {
guard reference.kind == .remote else {
return callback(.failure(Errors.invalidReferenceType(reference)))
}
Expand Down Expand Up @@ -143,19 +141,18 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
var options = HTTPClientRequest.Options()
options.addUserAgent = true
options.validResponseCodes = validResponseCodes
if defaultHttpClient {
// TODO: make these defaults configurable?
options.timeout = httpClient.configuration.requestTimeout ?? .seconds(1)
options.retryStrategy = httpClient.configuration.retryStrategy ?? .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))
options.circuitBreakerStrategy = httpClient.configuration.circuitBreakerStrategy ?? .hostErrors(maxErrors: 5, age: .seconds(5))
} else {
options.timeout = httpClient.configuration.requestTimeout
options.retryStrategy = httpClient.configuration.retryStrategy
options.circuitBreakerStrategy = httpClient.configuration.circuitBreakerStrategy
}
return options
}

private static func makeDefaultHTTPClient() -> HTTPClient {
var client = HTTPClient()
// TODO: make these defaults configurable?
client.configuration.requestTimeout = .seconds(1)
client.configuration.retryStrategy = .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))
client.configuration.circuitBreakerStrategy = .hostErrors(maxErrors: 50, age: .seconds(30))
return client
}

enum Errors: Error, Equatable {
case invalidReferenceType(PackageReference)
case invalidGitUrl(String)
Expand Down
Loading