Skip to content

update collections APIs to use package identity #3743

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 3 commits into from
Sep 15, 2021
Merged

update collections APIs to use package identity #3743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b7ed39
update collections APIs to use package identity
tomerd Sep 14, 2021
dcb79b3
fixup
tomerd Sep 14, 2021
aae361d
more backward compat
tomerd Sep 14, 2021
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
9 changes: 4 additions & 5 deletions Sources/Commands/SwiftPackageCollectionsTool.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
try JSONEncoder.makeWithDefaults().print(results.items)
} else {
results.items.forEach {
print("\($0.package.repository.url): \($0.package.summary ?? "")")
print("\($0.package.identity): \($0.package.summary ?? "")")
}
}

Expand All @@ -208,7 +208,7 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
try JSONEncoder.makeWithDefaults().print(packages)
} else {
packages.forEach {
print("\($0.repository.url): \($0.summary ?? "")")
print("\($0.identity): \($0.summary ?? "")")
}
}
}
Expand Down Expand Up @@ -265,10 +265,9 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
mutating func run() throws {
try with { collections in
let identity = PackageIdentity(url: packageURL)
let reference = PackageReference.remote(identity: identity, location: packageURL)

do { // assume URL is for a package in an imported collection
let result = try tsc_await { collections.getPackageMetadata(reference, callback: $0) }
let result = try tsc_await { collections.getPackageMetadata(identity: identity, location: packageURL, callback: $0) }

if let versionString = version {
guard let version = TSCUtility.Version(versionString), let result = result.package.versions.first(where: { $0.version == version }), let printedResult = printVersion(result) else {
Expand Down Expand Up @@ -314,7 +313,7 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
let description = optionalRow("Description", collection.overview)
let keywords = optionalRow("Keywords", collection.keywords?.joined(separator: ", "))
let createdAt = optionalRow("Created At", DateFormatter().string(from: collection.createdAt))
let packages = collection.packages.map { "\($0.repository.url)" }.joined(separator: "\n\(indent(levels: 2))")
let packages = collection.packages.map { "\($0.identity)" }.joined(separator: "\n\(indent(levels: 2))")

if jsonOptions.json {
try JSONEncoder.makeWithDefaults().print(collection)
Expand Down
35 changes: 35 additions & 0 deletions Sources/PackageCollections/API.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ public protocol PackageCollectionsProtocol {
/// - Parameters:
/// - reference: The package reference
/// - callback: The closure to invoke when result becomes available
@available(*, deprecated, message: "user getPackageMetadata(identity:) instead")
func getPackageMetadata(
_ reference: PackageReference,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void
Expand All @@ -126,12 +127,46 @@ public protocol PackageCollectionsProtocol {
/// - collections: Optional. If specified, only look for package in these collections. Data from the most recently
/// processed collection will be used.
/// - callback: The closure to invoke when result becomes available
@available(*, deprecated, message: "user getPackageMetadata(identity:) instead")
func getPackageMetadata(
_ reference: PackageReference,
collections: Set<PackageCollectionsModel.CollectionIdentifier>?,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void
)

/// Returns metadata for the package identified by the given `PackageIdentity`, along with the
/// identifiers of `PackageCollection`s where the package is found.
///
/// A failure is returned if the package is not found.
///
/// - Parameters:
/// - identity: The package identity
/// - location: The package location (optional for deduplication)
/// - callback: The closure to invoke when result becomes available
func getPackageMetadata(
identity: PackageIdentity,
location: String?,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void
)

/// Returns metadata for the package identified by the given `PackageIdentity`, along with the
/// identifiers of `PackageCollection`s where the package is found.
///
/// A failure is returned if the package is not found.
///
/// - Parameters:
/// - identity: The package identity
/// - location: The package location (optional for deduplication)
/// - collections: Optional. If specified, only look for package in these collections. Data from the most recently
/// processed collection will be used.
/// - callback: The closure to invoke when result becomes available
func getPackageMetadata(
identity: PackageIdentity,
location: String?,
collections: Set<PackageCollectionsModel.CollectionIdentifier>?,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void
)

/// Lists packages from the specified collections.
///
/// - Parameters:
Expand Down
27 changes: 19 additions & 8 deletions Sources/PackageCollections/Model/Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ import SourceControl
extension PackageCollectionsModel {
/// Package metadata
public struct Package: Codable, Equatable {
/// Package reference
public let reference: PackageReference
/// Package identity
public let identity: PackageIdentity

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

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to keep reference and repository until folks switch over to identity and location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yim-lee on the Package model?

Copy link
Contributor

Choose a reason for hiding this comment

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

All of the models. rdar://83113368, rdar://83114538 need to be done before we can remove reference and repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yim-lee does this help: dcb79b3

Copy link
Contributor

Choose a reason for hiding this comment

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

@tomerd yes

/// Package description
public let summary: String?
Expand Down Expand Up @@ -83,9 +83,20 @@ extension PackageCollectionsModel {
/// The package's programming languages
public let languages: Set<String>?

@available(*, deprecated, message: "use identity and location instead")
public var reference: PackageReference {
return .init(identity: self.identity, kind: .remote, location: self.location, name: nil)
}

@available(*, deprecated, message: "use identity and location instead")
public var repository: RepositorySpecifier {
return .init(url: self.location)
}

/// Initializes a `Package`
init(
repository: RepositorySpecifier,
identity: PackageIdentity,
location: String,
summary: String?,
keywords: [String]?,
versions: [Version],
Expand All @@ -95,8 +106,8 @@ extension PackageCollectionsModel {
authors: [Author]?,
languages: Set<String>?
) {
self.reference = .init(repository: repository)
self.repository = repository
self.identity = identity
self.location = location
self.summary = summary
self.keywords = keywords
self.versions = versions
Expand Down Expand Up @@ -271,6 +282,6 @@ extension PackageCollectionsModel.Package.Version {

extension Model.Package {
var displayName: String {
self.latestVersion?.packageName ?? self.reference.identity.description
self.latestVersion?.packageName ?? self.identity.description
}
}
12 changes: 10 additions & 2 deletions Sources/PackageCollections/Model/TargetListResult.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ extension PackageCollectionsModel.TargetListResult {
public struct Package: Hashable, Encodable {
public typealias Version = PackageCollectionsModel.TargetListResult.PackageVersion

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

/// Package's location
public let location: String

Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto ^. Cannot remove repository until rdar://83113368 and rdar://83114538 are done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yim-lee does this help: dcb79b3

/// Package description
public let summary: String?
Expand All @@ -44,6 +47,11 @@ extension PackageCollectionsModel.TargetListResult {

/// Package collections that contain this package and at least one of the `versions`
public let collections: [PackageCollectionsModel.CollectionIdentifier]

@available(*, deprecated, message: "use identity and location instead")
public var repository: RepositorySpecifier {
return .init(url: self.location)
}
}
}

Expand Down
73 changes: 46 additions & 27 deletions Sources/PackageCollections/PackageCollections.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,26 +314,26 @@ public struct PackageCollections: PackageCollectionsProtocol {
case .failure(let error):
callback(.failure(error))
case .success(let collections):
var packageCollections = [PackageReference: (package: Model.Package, collections: Set<Model.CollectionIdentifier>)]()
var packageCollections = [PackageIdentity: (package: Model.Package, collections: Set<Model.CollectionIdentifier>)]()
// Use package data from the most recently processed collection
collections.sorted(by: { $0.lastProcessedAt > $1.lastProcessedAt }).forEach { collection in
collection.packages.forEach { package in
var entry = packageCollections.removeValue(forKey: package.reference)
var entry = packageCollections.removeValue(forKey: package.identity)
if entry == nil {
entry = (package, .init())
}

if var entry = entry {
entry.collections.insert(collection.identifier)
packageCollections[package.reference] = entry
packageCollections[package.identity] = entry
}
}
}

let result = PackageCollectionsModel.PackageSearchResult(
items: packageCollections.sorted { $0.value.package.displayName < $1.value.package.displayName }
.map { entry in
.init(package: entry.value.package, collections: Array(entry.value.collections))
.init(package: entry.value.package, collections: Array(entry.value.collections))
}
)
callback(.success(result))
Expand All @@ -343,32 +343,48 @@ public struct PackageCollections: PackageCollectionsProtocol {

// MARK: - Package Metadata

@available(*, deprecated, message: "user identity based API instead")
public func getPackageMetadata(_ reference: PackageReference,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void) {
self.getPackageMetadata(reference, collections: nil, callback: callback)
}

@available(*, deprecated, message: "user identity based API instead")
public func getPackageMetadata(_ reference: PackageReference,
collections: Set<PackageCollectionsModel.CollectionIdentifier>?,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void) {
self.getPackageMetadata(identity: reference.identity, location: reference.location, collections: .none, callback: callback)
}

public func getPackageMetadata(identity: PackageIdentity,
location: String? = .none,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void) {
self.getPackageMetadata(identity: identity, location: location, collections: .none, callback: callback)
}

public func getPackageMetadata(identity: PackageIdentity,
location: String? = .none,
collections: Set<PackageCollectionsModel.CollectionIdentifier>?,
callback: @escaping (Result<PackageCollectionsModel.PackageMetadata, Error>) -> Void) {
guard Self.isSupportedPlatform else {
return callback(.failure(PackageCollectionError.unsupportedPlatform))
}

// first find in storage
self.findPackage(reference: reference, collections: collections) { result in
self.findPackage(identity: identity, location: location, collections: collections) { result in
switch result {
case .failure(let error):
callback(.failure(error))
case .success(let packageSearchResult):

// then try to get more metadata from provider (optional)
let authTokenType = self.metadataProvider.getAuthTokenType(for: reference)
let authTokenType = self.metadataProvider.getAuthTokenType(for: packageSearchResult.package.location)
let isAuthTokenConfigured = authTokenType.flatMap { self.configuration.authTokens()?[$0] } != nil

self.metadataProvider.get(reference) { result in
self.metadataProvider.get(identity: packageSearchResult.package.identity, location: packageSearchResult.package.location) { result in
switch result {
case .failure(let error):
self.diagnosticsEngine?.emit(warning: "Failed fetching information about \(reference) from \(self.metadataProvider.name): \(error)")
self.diagnosticsEngine?.emit(warning: "Failed fetching information about \(identity) from \(self.metadataProvider.name): \(error)")

let provider: PackageMetadataProviderContext?
switch error {
Expand Down Expand Up @@ -517,7 +533,8 @@ public struct PackageCollections: PackageCollectionsProtocol {
}
}

func findPackage(reference: PackageReference,
func findPackage(identity: PackageIdentity,
location: String?,
collections: Set<PackageCollectionsModel.CollectionIdentifier>?,
callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult.Item, Error>) -> Void) {
self.storage.sources.list { result in
Expand All @@ -530,17 +547,23 @@ public struct PackageCollections: PackageCollectionsProtocol {
collectionIdentifiers = collectionIdentifiers.filter { collections.contains($0) }
}
if collectionIdentifiers.isEmpty {
return callback(.failure(NotFoundError("\(reference)")))
return callback(.failure(NotFoundError("\(identity)")))
}
self.storage.collections.findPackage(identifier: reference.identity, collectionIdentifiers: collectionIdentifiers) { findPackageResult in
self.storage.collections.findPackage(identifier: identity, collectionIdentifiers: collectionIdentifiers) { findPackageResult in
switch findPackageResult {
case .failure(let error):
callback(.failure(error))
case .success(let packagesCollections):
// A package identity can be associated with multiple repository URLs
let matches = packagesCollections.packages.filter { $0.reference.canonicalLocation == reference.canonicalLocation }
let matches: [PackageCollectionsModel.Package]
if let location = location {
// A package identity can be associated with multiple repository URLs
matches = packagesCollections.packages.filter { CanonicalPackageIdentity($0.location) == CanonicalPackageIdentity(location) }
}
else {
matches = packagesCollections.packages
}
guard let package = matches.first else {
return callback(.failure(NotFoundError("\(reference)")))
return callback(.failure(NotFoundError("\(identity), \(location ?? "none")")))
}
callback(.success(.init(package: package, collections: packagesCollections.collections)))
}
Expand All @@ -550,22 +573,22 @@ public struct PackageCollections: PackageCollectionsProtocol {
}

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>)]()
var packageCollections = [PackageIdentity: (package: Model.Package, collections: Set<Model.CollectionIdentifier>)]()
var targetsPackages = [String: (target: Model.Target, packages: Set<PackageIdentity>)]()

collections.forEach { collection in
collection.packages.forEach { package in
// Avoid copy-on-write: remove entry from dictionary before mutating
var entry = packageCollections.removeValue(forKey: package.reference) ?? (package, .init())
var entry = packageCollections.removeValue(forKey: package.identity) ?? (package, .init())
entry.collections.insert(collection.identifier)
packageCollections[package.reference] = entry
packageCollections[package.identity] = entry

package.versions.forEach { version in
version.manifests.values.forEach { manifest in
manifest.targets.forEach { target in
// Avoid copy-on-write: remove entry from dictionary before mutating
var entry = targetsPackages.removeValue(forKey: target.name) ?? (target: target, packages: .init())
entry.packages.insert(package.reference)
entry.packages.insert(package.identity)
targetsPackages[target.name] = entry
}
}
Expand All @@ -586,7 +609,8 @@ public struct PackageCollections: PackageCollectionsProtocol {
)
}
}
return .init(repository: pair.package.repository,
return .init(identity: pair.package.identity,
location: pair.package.location,
summary: pair.package.summary,
versions: versions,
collections: Array(pair.collections))
Expand Down Expand Up @@ -614,7 +638,8 @@ public struct PackageCollections: PackageCollectionsProtocol {
versions.sort(by: >)

return Model.Package(
repository: package.repository,
identity: package.identity,
location: package.location,
summary: basicMetadata?.summary ?? package.summary,
keywords: basicMetadata?.keywords ?? package.keywords,
versions: versions,
Expand All @@ -634,9 +659,3 @@ private struct UnknownProvider: Error {
self.sourceType = sourceType
}
}

private extension PackageReference {
var canonicalLocation: String {
(self.location.hasSuffix(".git") ? String(self.location.dropLast(4)) : self.location).lowercased()
}
}
Loading