Skip to content

Commit

Permalink
Improve internal Logger (#2361)
Browse files Browse the repository at this point in the history
  • Loading branch information
OdNairy authored Nov 15, 2024
1 parent 605e8fd commit 2f63984
Show file tree
Hide file tree
Showing 35 changed files with 109 additions and 73 deletions.
2 changes: 1 addition & 1 deletion Sources/MapboxMaps/Annotations/Annotation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ extension Array where Element: Annotation {
let duplicates = self.removeDuplicates(by: \.id)
if !duplicates.isEmpty {
let ids = duplicates.lazy.map(\.id).joined(separator: ", ")
Log.error(forMessage: "Duplicated annotations: \(ids)", category: "Annotations")
Log.error("Duplicated annotations: \(ids)", category: "Annotations")
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/MapboxMaps/Annotations/AnnotationImagesManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ internal final class AnnotationImagesManager: AnnotationImagesManagerProtocol {
addedAnnotationImages.insert(id)
} catch {
Log.warning(
forMessage: "Could not add image to style due to error: \(error)",
"Could not add image to style due to error: \(error)",
category: "Annnotations")
}
}
Expand All @@ -75,7 +75,7 @@ internal final class AnnotationImagesManager: AnnotationImagesManagerProtocol {
addedAnnotationImages.remove(imageName)
} catch {
Log.warning(
forMessage: "Could not remove image from style due to error: \(error)",
"Could not remove image from style due to error: \(error)",
category: "Annnotations")
}
}
Expand Down
14 changes: 7 additions & 7 deletions Sources/MapboxMaps/Annotations/AnnotationManagerImpl.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ final class AnnotationManagerImpl<AnnotationType: Annotation & AnnotationInterna
do {
try style.moveLayer(withId: id, to: layerPosition ?? .default)
} catch {
Log.error(forMessage: "Failed to mover layer to a new position. Error: \(error)", category: "Annotations")
Log.error("Failed to mover layer to a new position. Error: \(error)", category: "Annotations")
}
}
}
Expand Down Expand Up @@ -135,7 +135,7 @@ final class AnnotationManagerImpl<AnnotationType: Annotation & AnnotationInterna
try style.addPersistentLayer(layer, layerPosition: layerPosition)
} catch {
Log.error(
forMessage: "Failed to create source / layer in \(implementationName). Error: \(error)",
"Failed to create source / layer in \(implementationName). Error: \(error)",
category: "Annotations")
}

Expand All @@ -160,7 +160,7 @@ final class AnnotationManagerImpl<AnnotationType: Annotation & AnnotationInterna
)
} catch {
Log.error(
forMessage: "Failed to add cluster layer in \(implementationName). Error: \(error)",
"Failed to add cluster layer in \(implementationName). Error: \(error)",
category: "Annotations")
}
}
Expand Down Expand Up @@ -197,7 +197,7 @@ final class AnnotationManagerImpl<AnnotationType: Annotation & AnnotationInterna
try style.removeLayer(withId: "mapbox-iOS-cluster-text-layer-manager-" + id)
} catch {
Log.error(
forMessage: "Failed to remove cluster layer in \(implementationName). Error: \(error)",
"Failed to remove cluster layer in \(implementationName). Error: \(error)",
category: "Annotations")
}
}
Expand Down Expand Up @@ -232,7 +232,7 @@ final class AnnotationManagerImpl<AnnotationType: Annotation & AnnotationInterna
try body()
} catch {
Log.warning(
forMessage: "Failed to remove \(what) for \(implementationName) with id \(id) due to error: \(error)",
"Failed to remove \(what) for \(implementationName) with id \(id) due to error: \(error)",
category: "Annotations")
}
}
Expand Down Expand Up @@ -326,7 +326,7 @@ final class AnnotationManagerImpl<AnnotationType: Annotation & AnnotationInterna
}
} catch {
Log.error(
forMessage: "Could not set layer properties in PointAnnotationManager due to error \(error)",
"Could not set layer properties in PointAnnotationManager due to error \(error)",
category: "Annotations")
}
}
Expand Down Expand Up @@ -500,7 +500,7 @@ final class AnnotationManagerImpl<AnnotationType: Annotation & AnnotationInterna
try style.addSource(GeoJSONSource(id: dragId))
try style.addPersistentLayer(AnnotationType.makeLayer(id: dragId), layerPosition: .above(id))
} catch {
Log.error(forMessage: "Add drag source/layer \(error)", category: "Annotations")
Log.error("Add drag source/layer \(error)", category: "Annotations")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public final class AnnotationOrchestrator {

if let manager, warnIfRemoved {
Log.warning(
forMessage: "\(type(of: manager)) with id \(id) was removed implicitly when invoking \(function) with the same id.",
"\(type(of: manager)) with id \(id) was removed implicitly when invoking \(function) with the same id.",
category: "Annotations")
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/MapboxMaps/Annotations/ViewAnnotation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public final class ViewAnnotation {
do {
try body()
} catch {
Log.error(forMessage: "Failed to \(action) annotation \(id): \(error)", category: "ViewAnnotation")
Log.error("Failed to \(action) annotation \(id): \(error)", category: "ViewAnnotation")
}
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/MapboxMaps/Annotations/ViewAnnotationManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ extension ViewAnnotationManager {
func validateAnnotationSuperview(_ view: UIView, expected: UIView?) {
// Re-add the view if the superview of the annotation view is different than the container
if view.superview != expected {
Log.warning(forMessage: "Superview changed for annotation view. Use `ViewAnnotationManager.remove(_ view: UIView)` instead to remove it from the layout calculation.", category: "Annotations")
Log.warning("Superview changed for annotation view. Use `ViewAnnotationManager.remove(_ view: UIView)` instead to remove it from the layout calculation.", category: "Annotations")
view.removeFromSuperview()
expected?.addSubview(view)
}
Expand All @@ -589,7 +589,7 @@ func validateAnnotationSuperview(_ view: UIView, expected: UIView?) {
func validateAnnotationHidden(_ view: UIView, expected: Bool) {
// View is still considered for layout calculation, users should not modify the visibility of view directly
if view.isHidden != expected {
Log.warning(forMessage: "Visibility changed for annotation view. Use `ViewAnnotationManager.update(view: UIView, options: ViewAnnotationOptions)` instead to update visibility and remove the view from layout calculation.", category: "Annotations")
Log.warning("Visibility changed for annotation view. Use `ViewAnnotationManager.update(view: UIView, options: ViewAnnotationOptions)` instead to update visibility and remove the view from layout calculation.", category: "Annotations")
view.isHidden = expected
}
}
2 changes: 1 addition & 1 deletion Sources/MapboxMaps/Camera/SimpleCameraAnimator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal final class SimpleCameraAnimator: SimpleCameraAnimatorProtocol {
hasNilMismatch(for: \.anchor) ||
hasNilMismatch(for: \.bearing) ||
hasNilMismatch(for: \.pitch) {
Log.warning(forMessage: "Animator updated with differing non-nil to-value properties.", category: "maps-ios")
Log.warning("Animator updated with differing non-nil to-value properties.", category: "maps-ios")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ extension MapContentUniqueProperties {
case .flat:
lights.flat = try? lightContainer.decode(FlatLight.self)
default:
Log.warning(forMessage: "Incorrect light configuration. Specify both directional and ambient lights OR flat light.", category: "StyleDSL")
Log.warning("Incorrect light configuration. Specify both directional and ambient lights OR flat light.", category: "StyleDSL")
}
}
}
Expand All @@ -96,7 +96,7 @@ private extension MapContentUniqueProperties.Lights {
os_log(.debug, log: .contentDSL, "set 3d lights")
try style.setLights(ambient: ambient, directional: directional)
} else if directional != nil || ambient != nil {
Log.warning(forMessage: "Incorrect 3D light configuration. Specify both directional and ambient lights.", category: "StyleDSL")
Log.warning("Incorrect 3D light configuration. Specify both directional and ambient lights.", category: "StyleDSL")
} else if let flat = flat {
os_log(.debug, log: .contentDSL, "set flat light")
try style.setLights(flat)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ extension MapStyleReconciler {
}
}
} catch {
Log.error(forMessage: "Failed updating import config properties, \(error)")
Log.error("Failed updating import config properties, \(error)")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ final class MountedViewAnnotation: MapContentMountedComponent {

func unmount(with context: MapContentNodeContext) throws {
guard let remove else {
return Log.error(forMessage: "Could not remove the view annotation", category: "Annotations")
return Log.error("Could not remove the view annotation", category: "Annotations")
}
remove()
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/MapboxMaps/ContentBuilders/Tree/MapContentNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ final class MapContentNode: Identifiable {
return
}
} catch {
Log.error(forMessage: "\(error)", category: "StyleDSL")
Log.error("\(error)", category: "StyleDSL")
}

}
Expand Down Expand Up @@ -160,7 +160,7 @@ func wrapStyleDSLError(_ closure: () throws -> Void) {
do {
try closure()
} catch {
Log.error(forMessage: "\(error)", category: "StyleDSL")
Log.error("\(error)", category: "StyleDSL")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ private extension MapContentNodeContext {
initialMapUniqueProperties.transition = TransitionOptions(style.styleManager.getStyleTransition())
return initialMapUniqueProperties
} catch {
Log.warning(forMessage: "Unable to decode initial MapContentUniqueProperties \(error) from StyleJSON", category: "StyleDSL")
Log.warning("Unable to decode initial MapContentUniqueProperties \(error) from StyleJSON", category: "StyleDSL")
return nil
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ extension RenderedQueryOptions {
do {
filterJson = try filter.toJSON()
} catch {
Log.error(forMessage: "Filter expression could not be encoded", category: "RenderedQueryOptions")
Log.error("Filter expression could not be encoded", category: "RenderedQueryOptions")
}
}

Expand All @@ -30,7 +30,7 @@ extension RenderedQueryOptions {
let filterData = try JSONSerialization.data(withJSONObject: filter, options: [])
filterExp = try JSONDecoder().decode(Exp.self, from: filterData)
} catch {
Log.error(forMessage: "Filter expression could not be decoded", category: "RenderedQueryOptions")
Log.error("Filter expression could not be decoded", category: "RenderedQueryOptions")
}

return filterExp
Expand Down
2 changes: 1 addition & 1 deletion Sources/MapboxMaps/Foundation/Extensions/Image.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ extension CoreMapsImage {
/// - uiImage: The source image.
internal convenience init?(uiImage: UIImage) {
guard let data = Data(uiImage: uiImage) else {
Log.warning(forMessage: "Failed to convert UIImage")
Log.warning("Failed to convert UIImage")
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ extension UIScene {
}
}
#endif
Log.info(forMessage: "Found no window attached to the current scene: \(self)")
Log.info("Found no window attached to the current scene: \(self)")
return []
}
}
54 changes: 44 additions & 10 deletions Sources/MapboxMaps/Foundation/Logger.swift
Original file line number Diff line number Diff line change
@@ -1,30 +1,64 @@
import Foundation
@_implementationOnly import MapboxCommon_Private.MBXLog_Internal

internal struct Log {
/// A logging utility with MapboxCommon backend by default.
@_spi(Logging) public struct Log {
private typealias Logger = MapboxCommon_Private.Log

private static func logCategory(_ additionalCategory: String?) -> String {
let logPrefix = "maps-ios"
guard let additionalCategory = additionalCategory else {
guard let additionalCategory = additionalCategory, !additionalCategory.isEmpty else {
return logPrefix
}
return "\(logPrefix)/\(additionalCategory)"
}

internal static func debug(forMessage message: String, category: String? = nil) {
Logger.debug(forMessage: message, category: logCategory(category))
/// Log a debug message.
@_spi(Logging) public static func debug(_ message: String, category: Category? = nil) {
Logger.debug(forMessage: message, category: logCategory(category?.rawValue))
}

internal static func info(forMessage message: String, category: String? = nil) {
Logger.info(forMessage: message, category: logCategory(category))
/// Log an info message.
@_spi(Logging) public static func info(_ message: String, category: Category? = nil) {
Logger.info(forMessage: message, category: logCategory(category?.rawValue))
}

internal static func warning(forMessage message: String, category: String? = nil) {
Logger.warning(forMessage: message, category: logCategory(category))
/// Log a warning message.
@_spi(Logging) public static func warning(_ message: String, category: Category? = nil) {
Logger.warning(forMessage: message, category: logCategory(category?.rawValue))
}

internal static func error(forMessage message: String, category: String? = nil) {
Logger.error(forMessage: message, category: logCategory(category))
/// Log an error message.
@_spi(Logging) public static func error(_ message: String, category: Category? = nil) {
Logger.error(forMessage: message, category: logCategory(category?.rawValue))
}
}

extension Log {
@_spi(Logging) public struct Category: RawRepresentable, ExpressibleByStringLiteral {
public let rawValue: String

public init(rawValue: String) {
self.rawValue = rawValue
}

init(_ value: String) {
self.rawValue = value
}

public init(stringLiteral value: StringLiteralType) {
self.rawValue = value
}

public init(unicodeScalarLiteral value: String) {
self.rawValue = value
}

public init(extendedGraphemeClusterLiteral value: String) {
self.rawValue = value
}

static let `default` = Category("")
}

}
4 changes: 2 additions & 2 deletions Sources/MapboxMaps/Foundation/MapView+Attribution.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ extension MapView: AttributionDialogManagerDelegate {
func attributionDialogManager(_ attributionDialogManager: AttributionDialogManager, didTriggerActionFor attribution: Attribution) {
switch attribution.kind {
case .actionable(let url):
Log.debug(forMessage: "Open url: \(url))", category: "Attribution")
Log.debug("Open url: \(url))", category: "Attribution")
attributionUrlOpener.openAttributionURL(url)
case .feedback:
let url = mapboxFeedbackURL()
Log.debug(forMessage: "Open url: \(url))", category: "Attribution")
Log.debug("Open url: \(url))", category: "Attribution")
attributionUrlOpener.openAttributionURL(url)
case .nonActionable:
break
Expand Down
2 changes: 1 addition & 1 deletion Sources/MapboxMaps/Foundation/MapView+Snapshot.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extension MapView {
}

guard let metalView = metalView else {
Log.error(forMessage: "No metal view present.", category: "MapView.snapshot")
Log.error("No metal view present.", category: "MapView.snapshot")
throw SnapshotError.noMetalView
}

Expand Down
6 changes: 3 additions & 3 deletions Sources/MapboxMaps/Foundation/MapView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -520,12 +520,12 @@ open class MapView: UIView, SizeTrackingLayerDelegate {

// Metal is unavailable on older simulators
guard ProcessInfo().isOperatingSystemAtLeast(OperatingSystemVersion(majorVersion: 13, minorVersion: 0, patchVersion: 0)) else {
Log.warning(forMessage: "Metal rendering is not supported on iOS versions < iOS 13. Please test on device or on iOS simulators version >= 13.", category: "MapView")
Log.warning("Metal rendering is not supported on iOS versions < iOS 13. Please test on device or on iOS simulators version >= 13.", category: "MapView")
return
}

// Metal is unavailable for a different reason
Log.error(forMessage: "No suitable Metal simulator can be found.", category: "MapView")
Log.error("No suitable Metal simulator can be found.", category: "MapView")
#endif
}

Expand Down Expand Up @@ -576,7 +576,7 @@ open class MapView: UIView, SizeTrackingLayerDelegate {
if metalView.contentScaleFactor != pixelRatio {
// DrawableSize setter will recalculate `contentScaleFactor` if the new drawableSize doesn't fit into
// the current bounds.size and scale.
Log.error(forMessage: "MetalView content scale factor \(metalView.contentScaleFactor) is not equal to pixel ratio \(pixelRatio)")
Log.error("MetalView content scale factor \(metalView.contentScaleFactor) is not equal to pixel ratio \(pixelRatio)")
}

// GL-Native will trigger update on `mapboxMap.size` update but it will come in the next frame.
Expand Down
Loading

0 comments on commit 2f63984

Please sign in to comment.