Skip to content

Commit e1ddade

Browse files
tomerdtomerd
committed
fixup
1 parent 91833d3 commit e1ddade

8 files changed

+64
-99
lines changed

‎Sources/PackageCollections/Model/Collection.swift

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,11 @@ extension PackageCollectionsModel {
7777

7878
/// URL of the source file
7979
public let url: URL
80-
81-
public init(type: PackageCollectionsModel.CollectionSourceType = .feed, url: URL) {
82-
self.type = type
83-
self.url = url
84-
}
80+
81+
public init(type: CollectionSourceType, url: URL) {
82+
self.type = type
83+
self.url = url
84+
}
8585
}
8686

8787
/// Represents the source type of a `PackageCollection`
@@ -91,27 +91,34 @@ extension PackageCollectionsModel {
9191
}
9292

9393
extension PackageCollectionsModel.CollectionSourceType: Codable {
94-
public enum DiscriminatorKeys: String, Codable {
95-
case feed
96-
}
97-
9894
public enum CodingKeys: CodingKey {
9995
case _case
10096
}
10197

10298
public init(from decoder: Decoder) throws {
10399
let container = try decoder.container(keyedBy: CodingKeys.self)
104-
switch try container.decode(DiscriminatorKeys.self, forKey: ._case) {
105-
case .feed:
100+
let value = try container.decode(String.self, forKey: ._case)
101+
switch value {
102+
case "feed":
106103
self = .feed
104+
default:
105+
throw UnknownType(value)
107106
}
108107
}
109108

110109
public func encode(to encoder: Encoder) throws {
111110
var container = encoder.container(keyedBy: CodingKeys.self)
112111
switch self {
113112
case .feed:
114-
try container.encode(DiscriminatorKeys.feed, forKey: ._case)
113+
try container.encode("feed", forKey: ._case)
114+
}
115+
}
116+
117+
struct UnknownType: Error {
118+
let type: String
119+
120+
init(_ type: String) {
121+
self.type = type
115122
}
116123
}
117124
}

‎Sources/PackageCollections/PackageCollectionsProvider.swift renamed to ‎Sources/PackageCollections/PackageCollectionProvider.swift

File renamed without changes.

‎Sources/PackageCollections/PackageCollections+Configuration.swift

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@
1111
// TODO: how do we read default config values? ENV variables? user settings?
1212
extension PackageCollections {
1313
struct Configuration {
14-
let collectionMaximumByteCount: Int = 100_000
15-
let maximumPackagesPerCollection: Int = 50
16-
let maximumVersionsPerPackage: Int = 6 // 2 majors, 3 minors per major
17-
let defaultSources: [PackageCollectionsModel.CollectionSource] = []
14+
// TODO: add configuration like mx size of feed, retries, etc
1815
}
1916
}

‎Sources/PackageCollections/PackageCollections+Storage.swift

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,18 @@ extension PackageCollections.Storage {
2121
func close() throws {
2222
var errors = [Error]()
2323

24-
if let closabe = self.collectionsProfiles as? Closable {
25-
do {
26-
try closabe.close()
27-
} catch {
28-
errors.append(error)
24+
let tryClose = { (item: Any) in
25+
if let closable = item as? Closable {
26+
do {
27+
try closable.close()
28+
} catch {
29+
errors.append(error)
30+
}
2931
}
3032
}
3133

32-
if let closable = self.collections as? Closable {
33-
do {
34-
try closabe.close()
35-
} catch {
36-
errors.append(error)
37-
}
38-
}
34+
tryClose(self.collectionsProfiles)
35+
tryClose(self.collections)
3936

4037
if !errors.isEmpty {
4138
throw MultipleErrors(errors)

‎Sources/PackageCollections/PackageCollections+Vallidation.swift

Lines changed: 4 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,48 +8,7 @@
88
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
99
*/
1010

11-
import struct Foundation.NSRange
12-
import class Foundation.NSRegularExpression
13-
14-
extension PackageCollections {
15-
struct Validator {
16-
// TODO: configuration?
17-
let maximumPackages: Int = 50
18-
19-
func validate(_ collection: PackageCollectionsModel.Collection) -> [ValidationError]? {
20-
var errors: [ValidationError]?
21-
let appendError = { (error: ValidationError) in
22-
if errors == nil {
23-
errors = .init()
24-
}
25-
errors?.append(error)
26-
}
27-
28-
if collection.packages.count > self.maximumPackages {
29-
appendError(.property(
30-
name: "PackageGroup.packages",
31-
description: "Only \(self.maximumPackages) allowed per group. \(collection.name) has \(collection.packages.count)."
32-
))
33-
}
34-
35-
// Each package
36-
// TODO: check repository URL
37-
// TODO: requires remote repo?
38-
39-
// Each package version
40-
// TODO: check max num of major versions (2)
41-
// TODO: check max num of minor versions (3)
42-
// TODO: check versions are in reversed order (requires semver)
43-
// copy code from package-feed-tool
44-
45-
return errors
46-
}
47-
}
48-
}
49-
5011
extension PackageCollectionsModel.CollectionSource {
51-
static let feedURLPattern = "^https://"
52-
5312
func validate() -> [ValidationError]? {
5413
var errors: [ValidationError]?
5514
let appendError = { (error: ValidationError) in
@@ -59,16 +18,12 @@ extension PackageCollectionsModel.CollectionSource {
5918
errors?.append(error)
6019
}
6120

21+
let allowedSchemes = Set(["https"])
22+
6223
switch self.type {
6324
case .feed:
64-
do {
65-
let regex = try NSRegularExpression(pattern: Self.feedURLPattern, options: .caseInsensitive)
66-
let urlString = url.absoluteString
67-
if regex.firstMatch(in: urlString, options: [], range: NSRange(location: 0, length: urlString.count)) == nil {
68-
appendError(.other(description: "Feed URL must be HTTPS: \(urlString)"))
69-
}
70-
} catch {
71-
appendError(.other(description: "Invalid feed URL: \(url)"))
25+
if !allowedSchemes.contains(url.scheme?.lowercased() ?? "") {
26+
appendError(.other(description: "Schema not allowed: \(url.absoluteString)"))
7227
}
7328
}
7429

‎Sources/PackageCollections/PackageCollections.swift

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,17 @@ internal struct PackageCollections: PackageCollectionsProtocol {
4343
}
4444
let collectionOrder = identifiers.enumerated().reduce([PackageCollectionsModel.CollectionIdentifier: Int]()) { partial, element in
4545
var dictionary = partial
46-
map[element.element] = element.offset
47-
return map
46+
dictionary[element.element] = element.offset
47+
return dictionary
4848
}
4949
self.storage.collections.list(identifiers: identifiers) { result in
5050
switch result {
5151
case .failure(let error):
5252
callback(.failure(error))
53-
case .success(var groups):
53+
case .success(var collections):
5454
// re-order by profile order which reflects the user's election
55-
groups.sort(by: { lhs, rhs in groupOrder[lhs.identifier] ?? 0 < groupOrder[rhs.identifier] ?? 0 })
56-
callback(.success(groups))
55+
collections.sort(by: { lhs, rhs in collectionOrder[lhs.identifier] ?? 0 < collectionOrder[rhs.identifier] ?? 0 })
56+
callback(.success(collections))
5757
}
5858
}
5959
}
@@ -97,7 +97,7 @@ internal struct PackageCollections: PackageCollectionsProtocol {
9797
let lock = Lock()
9898
var refreshResults = [Result<PackageCollectionsModel.Collection, Error>]()
9999
sources.forEach { source in
100-
self.refreshPackageGroupFromSource(source: source, profile: profile) { refreshResult in
100+
self.refreshCollectionFromSource(source: source, profile: profile) { refreshResult in
101101
lock.withLock { refreshResults.append(refreshResult) }
102102
if refreshResults.count == (lock.withLock { sources.count }) {
103103
let errors = refreshResults.compactMap { $0.failure }
@@ -119,12 +119,14 @@ internal struct PackageCollections: PackageCollectionsProtocol {
119119
return callback(.failure(MultipleErrors(errors)))
120120
}
121121

122+
// first record the registration
122123
self.storage.collectionsProfiles.add(source: source, order: order, to: profile) { result in
123124
switch result {
124125
case .failure(let error):
125126
callback(.failure(error))
126127
case .success:
127-
self.refreshPackageGroupFromSource(source: source, order: order, profile: profile, callback: callback)
128+
// next try to fetch the collection from the network and store it locally so future operations dont need to access the network
129+
self.refreshCollectionFromSource(source: source, order: order, profile: profile, callback: callback)
128130
}
129131
}
130132
}
@@ -139,6 +141,8 @@ internal struct PackageCollections: PackageCollectionsProtocol {
139141
case .failure(let error):
140142
callback(.failure(error))
141143
case .success:
144+
// check to see if the collection is used in some other profile,
145+
// if not delete it from storage to reduce disk space
142146
self.storage.collectionsProfiles.exists(source: source, in: nil) { result in
143147
switch result {
144148
case .failure(let error):
@@ -163,6 +167,9 @@ internal struct PackageCollections: PackageCollectionsProtocol {
163167
self.storage.collectionsProfiles.move(source: source, to: order, in: profile, callback: callback)
164168
}
165169

170+
// Returns information about a package collection.
171+
// The collection is not required to be in the configured list.
172+
// If not found locally (storage), the collection will be fetched from the source.
166173
func getCollection(_ source: PackageCollectionsModel.CollectionSource,
167174
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
168175
if let errors = source.validate() {
@@ -177,7 +184,7 @@ internal struct PackageCollections: PackageCollectionsProtocol {
177184
}
178185
provider.get(source, callback: callback)
179186
case .success(let collection):
180-
callback(.success(group))
187+
callback(.success(collection))
181188
}
182189
}
183190
}
@@ -229,10 +236,12 @@ internal struct PackageCollections: PackageCollectionsProtocol {
229236
fatalError("not implemented")
230237
}
231238

232-
private func refreshPackageGroupFromSource(source: PackageCollectionsModel.CollectionSource,
233-
order _: Int? = nil,
234-
profile _: PackageCollectionsModel.Profile? = nil,
235-
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
239+
// Fetch the collection from the network and store it in local storage
240+
// This helps avoid network access in normal operations
241+
private func refreshCollectionFromSource(source: PackageCollectionsModel.CollectionSource,
242+
order _: Int? = nil,
243+
profile _: PackageCollectionsModel.Profile? = nil,
244+
callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
236245
if let errors = source.validate() {
237246
return callback(.failure(MultipleErrors(errors)))
238247
}
@@ -250,15 +259,15 @@ internal struct PackageCollections: PackageCollectionsProtocol {
250259
}
251260

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

256265
collections.forEach { collection in
257266
collection.packages.forEach { package in
258267
// Avoid copy-on-write: remove entry from dictionary before mutating
259-
var entry = collectionGroups.removeValue(forKey: package.reference) ?? (package, .init())
268+
var entry = packageCollections.removeValue(forKey: package.reference) ?? (package, .init())
260269
entry.collections.insert(collection.identifier)
261-
collectionGroups[package.reference] = entry
270+
packageCollections[package.reference] = entry
262271

263272
package.versions.forEach { version in
264273
version.targets.forEach { target in
@@ -273,7 +282,7 @@ internal struct PackageCollections: PackageCollectionsProtocol {
273282

274283
return targetsPackages.map { _, pair in
275284
let targetPackages = pair.packages
276-
.compactMap { collectionGroups[$0] }
285+
.compactMap { packageCollections[$0] }
277286
.map { pair -> PackageCollectionsModel.TargetListResult.Package in
278287
let versions = pair.package.versions.map { PackageCollectionsModel.TargetListResult.PackageVersion(version: $0.version, packageName: $0.packageName) }
279288
return PackageCollectionsModel.TargetListResult.Package(repository: pair.package.repository,

‎Sources/PackageCollections/Storage/PackageCollectionsProfileStorage.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,13 +296,13 @@ private extension PackageCollectionsModel.Profile {
296296

297297
private extension PackageCollectionsModel.CollectionSource {
298298
init(_ from: Model.Source) throws {
299+
guard let url = URL(string: from.value) else {
300+
throw SerializationError.invalidURL(from.value)
301+
}
302+
self.url = url
299303
switch from.type {
300304
case Model.SourceType.feed.rawValue:
301-
guard let url = URL(string: from.value) else {
302-
throw SerializationError.invalidURL(from.value)
303-
}
304305
self.type = .feed
305-
self.url = url
306306
default:
307307
throw SerializationError.unknownType(from.type)
308308
}

‎Sources/PackageCollections/Storage/PackageCollectionsStorage.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ protocol PackageCollectionsStorage {
5252
func list(identifiers: [PackageCollectionsModel.CollectionIdentifier]?,
5353
callback: @escaping (Result<[PackageCollectionsModel.Collection], Error>) -> Void)
5454

55-
/// Returns `PackageSearchResult` for the given query.
55+
/// Returns `PackageSearchResult` for the given search criteria.
5656
///
5757
/// - Parameters:
5858
/// - identifiers: Optional. The identifiers of the `PackageCollection`s
@@ -62,7 +62,7 @@ protocol PackageCollectionsStorage {
6262
query: String,
6363
callback: @escaping (Result<PackageCollectionsModel.PackageSearchResult, Error>) -> Void)
6464

65-
/// Returns `TargetSearchResult` for the given search criteria, or all if non specified.
65+
/// Returns `TargetSearchResult` for the given search criteria.
6666
///
6767
/// - Parameters:
6868
/// - identifiers: Optional. The identifiers of the `PackageCollection`

0 commit comments

Comments
 (0)