Skip to content

Commit

Permalink
Fix problem report logs being duplicated
Browse files Browse the repository at this point in the history
  • Loading branch information
Jon Petersson authored and buggmagnet committed May 13, 2024
1 parent f91ba00 commit 168bd35
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 27 deletions.
4 changes: 3 additions & 1 deletion ios/MullvadVPN/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,9 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD

private func configureLogging() {
var loggerBuilder = LoggerBuilder(header: "MullvadVPN version \(Bundle.main.productVersion)")
loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.newLogFileURL(for: .mainApp))
loggerBuilder.addFileOutput(
fileURL: ApplicationConfiguration.newLogFileURL(for: .mainApp, in: ApplicationConfiguration.containerURL)
)
#if DEBUG
loggerBuilder.addOSLogOutput(subsystem: ApplicationTarget.mainApp.bundleIdentifier)
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ final class ProblemReportInteractor {
redactContainerPathsForSecurityGroupIdentifiers: [securityGroupIdentifier]
)

let logFileURLs = ApplicationTarget.allCases.flatMap { ApplicationConfiguration.logFileURLs(for: $0) }
let logFileURLs = ApplicationTarget.allCases.flatMap {
ApplicationConfiguration.logFileURLs(for: $0, in: ApplicationConfiguration.containerURL)
}
report.addLogFiles(fileURLs: logFileURLs)

return report
Expand Down
7 changes: 4 additions & 3 deletions ios/MullvadVPNTests/MullvadLogging/LogRotationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@ import XCTest

final class LogRotationTests: XCTestCase {
let fileManager = FileManager.default
let directoryPath = FileManager.default.temporaryDirectory.appendingPathComponent("LogRotationTests")
let directoryPath = FileManager.default.temporaryDirectory
.appendingPathComponent("LogRotationTests", isDirectory: true)

override func setUpWithError() throws {
try? fileManager.createDirectory(
at: directoryPath,
withIntermediateDirectories: false
withIntermediateDirectories: true
)
}

override func tearDownWithError() throws {
try fileManager.removeItem(atPath: directoryPath.relativePath)
try fileManager.removeItem(at: directoryPath)
}

func testRotatingActiveLogWhenSizeLimitIsExceeded() throws {
Expand Down
53 changes: 39 additions & 14 deletions ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,24 @@ import Foundation
import XCTest

class MullvadLoggingTests: XCTestCase {
func temporaryFileURL() -> URL {
// Create a URL for an unique file in the system's temporary directory.
let directory = NSTemporaryDirectory()
let filename = UUID().uuidString
let fileURL = URL(fileURLWithPath: directory).appendingPathComponent(filename)

// Add a teardown block to delete any file at `fileURL`.
addTeardownBlock {
try? FileManager.default.removeItem(at: fileURL)
}
let fileManager = FileManager.default
let directoryPath = FileManager.default.temporaryDirectory.appendingPathComponent("LoggingTests", isDirectory: true)

override func setUpWithError() throws {
try fileManager.createDirectory(
at: directoryPath,
withIntermediateDirectories: true
)
}

// Return the temporary file URL for use in a test method.
return fileURL
override func tearDownWithError() throws {
try fileManager.removeItem(at: directoryPath)
}

func testLogFileOutputStreamWritesHeader() throws {
let headerText = "This is a header"
let logMessage = "And this is a log message\n"
let fileURL = temporaryFileURL()
let fileURL = directoryPath.appendingPathComponent(UUID().uuidString)
let stream = LogFileOutputStream(fileURL: fileURL, header: headerText)
stream.write(logMessage)
sync()
Expand All @@ -42,7 +41,7 @@ class MullvadLoggingTests: XCTestCase {
let expectedHeader = "Header of a log file"

var builder = LoggerBuilder(header: expectedHeader)
let fileURL = temporaryFileURL()
let fileURL = directoryPath.appendingPathComponent(UUID().uuidString)
builder.addFileOutput(fileURL: fileURL)

builder.install()
Expand All @@ -55,4 +54,30 @@ class MullvadLoggingTests: XCTestCase {

XCTAssert(contents.hasPrefix(expectedHeader))
}

func testGettingLogFilesByApplicationTarget() async throws {
let mainTargetLog = ApplicationConfiguration.newLogFileURL(for: .mainApp, in: directoryPath)
let packetTunnelTargetLog = ApplicationConfiguration.newLogFileURL(for: .packetTunnel, in: directoryPath)

let logPaths = [
directoryPath.appendingPathComponent("test1.log"),
directoryPath.appendingPathComponent("test2.log"),
mainTargetLog,
packetTunnelTargetLog,
]

logPaths.forEach { url in
let stream = LogFileOutputStream(fileURL: url, header: "")
stream.write("test")
sync()
}

var urls = ApplicationConfiguration.logFileURLs(for: .mainApp, in: directoryPath)
XCTAssertEqual(urls.count, 1)
XCTAssertEqual(urls.first, mainTargetLog)

urls = ApplicationConfiguration.logFileURLs(for: .packetTunnel, in: directoryPath)
XCTAssertEqual(urls.count, 1)
XCTAssertEqual(urls.first, packetTunnelTargetLog)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,12 @@ extension PacketTunnelProvider {
var loggerBuilder = LoggerBuilder(header: "PacketTunnel version \(Bundle.main.productVersion)")
let pid = ProcessInfo.processInfo.processIdentifier
loggerBuilder.metadata["pid"] = .string("\(pid)")
loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.newLogFileURL(for: .packetTunnel))
loggerBuilder.addFileOutput(
fileURL: ApplicationConfiguration.newLogFileURL(
for: .packetTunnel,
in: ApplicationConfiguration.containerURL
)
)
#if DEBUG
loggerBuilder.addOSLogOutput(subsystem: ApplicationTarget.packetTunnel.bundleIdentifier)
#endif
Expand Down
14 changes: 7 additions & 7 deletions ios/Shared/ApplicationConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,25 @@ enum ApplicationConfiguration {
FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: securityGroupIdentifier)!
}

/// Returns URL for new log file associated with application target and located within shared container.
static func newLogFileURL(for target: ApplicationTarget) -> URL {
/// Returns URL for new log file associated with application target and located within the specified container.
static func newLogFileURL(for target: ApplicationTarget, in containerURL: URL) -> URL {
containerURL.appendingPathComponent(
"\(target.bundleIdentifier)_\(Date().logFileFormatted).log",
isDirectory: false
)
}

/// Returns URLs for log files associated with application target and located within shared container.
static func logFileURLs(for target: ApplicationTarget) -> [URL] {
/// Returns URLs for log files associated with application target and located within the specified container.
static func logFileURLs(for target: ApplicationTarget, in containerURL: URL) -> [URL] {
let fileManager = FileManager.default
let containerUrl = containerURL
let filePathsInDirectory = try? fileManager.contentsOfDirectory(atPath: containerURL.relativePath)

let filteredFilePaths: [URL] = filePathsInDirectory?.compactMap { path in
let pathIsLog = path.split(separator: ".").last == "log"
let pathBelongsToTarget = path.contains(target.bundleIdentifier)
// Pattern should be "<Target Bundle ID>_", eg. "net.mullvad.MullvadVPN_".
let pathBelongsToTarget = path.contains("\(target.bundleIdentifier)_")

return pathIsLog && pathBelongsToTarget ? containerUrl.appendingPathComponent(path) : nil
return pathIsLog && pathBelongsToTarget ? containerURL.appendingPathComponent(path) : nil
} ?? []

let sortedFilePaths = try? filteredFilePaths.sorted { path1, path2 in
Expand Down

0 comments on commit 168bd35

Please sign in to comment.