Skip to content
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

Sessions table view model refactoring #703

Merged
merged 16 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
8 changes: 6 additions & 2 deletions Packages/ConfCore/ConfCore/AppleAPIClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import Siesta

// MARK: - Initialization and configuration

public final class AppleAPIClient {
public final class AppleAPIClient: Logging, Signposting {
public static let log = makeLogger()
public static let signposter = makeSignposter()

fileprivate var environment: Environment
fileprivate var service: Service
Expand Down Expand Up @@ -67,7 +69,9 @@ public final class AppleAPIClient {
}

service.configureTransformer(environment.sessionsPath) { (entity: Entity<Data>) throws -> ContentsResponse? in
return try decoder.decode(ContentsResponse.self, from: entity.content)
try Self.signposter.withIntervalSignpost("decode contents", id: Self.signposter.makeSignpostID()) {
try decoder.decode(ContentsResponse.self, from: entity.content)
}
}

service.configureTransformer(environment.liveVideosPath) { (entity: Entity<Data>) throws -> [SessionAsset]? in
Expand Down
37 changes: 36 additions & 1 deletion Packages/ConfCore/ConfCore/Logging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import OSLog

public struct LoggingConfig {
public let subsystem: String
public let category: String
public var category: String
}

public protocol Logging {
Expand Down Expand Up @@ -48,3 +48,38 @@ public extension Logging {
public func makeLogger(subsystem: String, category: String) -> Logger {
Logger(subsystem: subsystem, category: category)
}

public protocol Signposting: Logging {
static var signposter: OSSignposter { get }
var signposter: OSSignposter { get }
}

public extension Signposting {
static func makeSignposter() -> OSSignposter { OSSignposter(logger: log) }
var signposter: OSSignposter { Self.signposter }
}

public extension OSSignposter {
/// Convenient but several caveats because OSLogMessage is stupid
func withEscapingOneShotIntervalSignpost<T>(
_ name: StaticString,
_ message: String? = nil,
around task: (@escaping () -> Void) throws -> T
) rethrows -> T {
var state: OSSignpostIntervalState?
if let message {
state = beginInterval(name, id: makeSignpostID(), "\(message)")
} else {
state = beginInterval(name, id: makeSignpostID())
}

let end = {
if let innerState = state {
state = nil
endInterval(name, innerState)
}
}

return try task(end)
}
}
78 changes: 37 additions & 41 deletions Packages/ConfCore/ConfCore/Session.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ public class Session: Object, Decodable {

/// The event identifier for the event this session belongs to
@objc public dynamic var eventIdentifier = ""
@objc public dynamic var eventStartDate = Date.distantPast

/// Track name
@objc public dynamic var trackName = ""
@objc public dynamic var trackOrder = 0

/// Track identifier
@objc public dynamic var trackIdentifier = ""
Expand Down Expand Up @@ -95,25 +97,11 @@ public class Session: Object, Decodable {

public static let videoPredicate: NSPredicate = NSPredicate(format: "ANY assets.rawAssetType == %@", SessionAssetType.streamingVideo.rawValue)

public static func standardSort(sessionA: Session, sessionB: Session) -> Bool {
guard let eventA = sessionA.event.first, let eventB = sessionB.event.first else { return false }
guard let trackA = sessionA.track.first, let trackB = sessionB.track.first else { return false }

if trackA.order == trackB.order {
if eventA.startDate == eventB.startDate {
return sessionA.title < sessionB.title
} else {
return eventA.startDate > eventB.startDate
}
} else {
return trackA.order < trackB.order
}
}

public static func standardSortForSchedule(sessionA: Session, sessionB: Session) -> Bool {
guard let instanceA = sessionA.instances.first, let instanceB = sessionB.instances.first else { return false }

return SessionInstance.standardSort(instanceA: instanceA, instanceB: instanceB)
public static func sameTrackSortDescriptors() -> [RealmSwift.SortDescriptor] {
return [
RealmSwift.SortDescriptor(keyPath: "eventStartDate"),
RealmSwift.SortDescriptor(keyPath: "title")
]
}

func merge(with other: Session, in realm: Realm) {
Expand All @@ -129,36 +117,29 @@ public class Session: Object, Decodable {
mediaDuration = other.mediaDuration

// merge assets
let assets = other.assets.filter { otherAsset in
return !self.assets.contains(where: { $0.identifier == otherAsset.identifier })
// Pulling the identifiers into Swift Set is an optimization for realm,
// You can't see it but `map` on `List` is lazy and each call to `.contains(element)`
// is O(n) but with a higher constant time because it accesses the realm property
// every time. Pulling strings into a Set gets the `.contains` call down to O(1)
// and ensures the Realm object accesses are only done once
let currentAssetIds = Set(self.assets.map { $0.identifier })
other.assets.forEach { otherAsset in
guard !currentAssetIds.contains(otherAsset.identifier) else { return }
self.assets.append(otherAsset)
}
self.assets.append(objectsIn: assets)

let currentRelatedIds = Set(related.map { $0.identifier })
other.related.forEach { newRelated in
let effectiveRelated: RelatedResource
guard !currentRelatedIds.contains(newRelated.identifier) else { return }

if let existingResource = realm.object(ofType: RelatedResource.self, forPrimaryKey: newRelated.identifier) {
effectiveRelated = existingResource
} else {
effectiveRelated = newRelated
}

guard !related.contains(where: { $0.identifier == effectiveRelated.identifier }) else { return }
related.append(effectiveRelated)
related.append(realm.object(ofType: RelatedResource.self, forPrimaryKey: newRelated.identifier) ?? newRelated)
}

let currentFocusIds = Set(focuses.map { $0.name })
other.focuses.forEach { newFocus in
let effectiveFocus: Focus
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The existing logic doesn't handle removals, should we fix that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(that's true for all 3 lists that get updated here)


if let existingFocus = realm.object(ofType: Focus.self, forPrimaryKey: newFocus.name) {
effectiveFocus = existingFocus
} else {
effectiveFocus = newFocus
}

guard !focuses.contains(where: { $0.name == effectiveFocus.name }) else { return }
guard !currentFocusIds.contains(newFocus.name) else { return }

focuses.append(effectiveFocus)
focuses.append(realm.object(ofType: Focus.self, forPrimaryKey: newFocus.name) ?? newFocus)
}
}

Expand Down Expand Up @@ -323,6 +304,21 @@ extension Session {
return asset
}

public func thumbImageAsset() -> SessionAsset? {
allenhumphreys marked this conversation as resolved.
Show resolved Hide resolved
guard let baseURL = event.first.flatMap({ URL(string: $0.imagesPath) }) else { return nil }

let filename = "\(staticContentId)_wide_162x91_2x.jpg"

let url = baseURL.appendingPathComponent("\(staticContentId)/\(filename)")

let asset = SessionAsset()

asset.assetType = .image
asset.remoteURL = url.absoluteString

return asset
}

public func assets(matching types: [SessionAssetType]) -> Results<SessionAsset> {
assert(!types.contains(.image), "This method does not support finding image assets")

Expand Down
8 changes: 0 additions & 8 deletions Packages/ConfCore/ConfCore/SessionAsset.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,6 @@ public class SessionAsset: Object, Decodable {
return "identifier"
}

func merge(with other: SessionAsset, in realm: Realm) {
assert(other.remoteURL == remoteURL, "Can't merge two objects with different identifiers!")

year = other.year
sessionId = other.sessionId
relativeLocalURL = other.relativeLocalURL
}

public func generateIdentifier() -> String {
return String(year) + "@" + sessionId + "~" + rawAssetType.replacingOccurrences(of: "WWDCSessionAssetType", with: "")
}
Expand Down
30 changes: 12 additions & 18 deletions Packages/ConfCore/ConfCore/SessionInstance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,13 @@ public class SessionInstance: Object, ConditionallyDecodable {
return ["code"]
}

public static func standardSort(instanceA: SessionInstance, instanceB: SessionInstance) -> Bool {
guard let sessionA = instanceA.session, let sessionB = instanceB.session else { return false }

if instanceA.sessionType == instanceB.sessionType {
return Session.standardSort(sessionA: sessionA, sessionB: sessionB)
} else {
return instanceA.sessionType < instanceB.sessionType
}
public static func standardSortDescriptors() -> [RealmSwift.SortDescriptor] {
return [
RealmSwift.SortDescriptor(keyPath: "rawSessionType"),
RealmSwift.SortDescriptor(keyPath: "session.trackOrder"),
RealmSwift.SortDescriptor(keyPath: "session.eventStartDate"),
RealmSwift.SortDescriptor(keyPath: "session.title")
]
}

func merge(with other: SessionInstance, in realm: Realm) {
Expand All @@ -113,17 +112,12 @@ public class SessionInstance: Object, ConditionallyDecodable {
session.merge(with: otherSession, in: realm)
}

let otherKeywords = other.keywords.map { newKeyword -> (Keyword) in
if newKeyword.realm == nil,
let existingKeyword = realm.object(ofType: Keyword.self, forPrimaryKey: newKeyword.name) {
return existingKeyword
} else {
return newKeyword
}
}
let currentKeywordIds = Set(keywords.map(\.name))
other.keywords.forEach { newKeyword in
guard !currentKeywordIds.contains(newKeyword.name) else { return }

keywords.removeAll()
keywords.append(objectsIn: otherKeywords)
keywords.append(realm.object(ofType: Keyword.self, forPrimaryKey: newKeyword.name) ?? newKeyword)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is no longer equivalent since it doesn't handle removals

}
}

// MARK: - Decodable
Expand Down
Loading