From 952daabe30dc2fd3e9279e1236a3f7f279e3b21d Mon Sep 17 00:00:00 2001 From: Valentin Corral Date: Tue, 11 Apr 2017 15:42:52 +0200 Subject: [PATCH 1/2] Only report when current stage is in notifyReleaseStages --- Sources/Bugsnag/Configuration.swift | 4 +- Sources/Bugsnag/PayloadTransformer.swift | 16 ++- Sources/Bugsnag/Reporter.swift | 16 ++- .../Mocks/ConfigurationMock.swift | 13 ++- .../Mocks/PayloadTransformerMock.swift | 12 +- .../PayloadTransformerTests.swift | 11 +- Tests/BugsnagTests/ReporterTests.swift | 104 +++++++++++++++++- 7 files changed, 140 insertions(+), 36 deletions(-) diff --git a/Sources/Bugsnag/Configuration.swift b/Sources/Bugsnag/Configuration.swift index 020e077..e893858 100644 --- a/Sources/Bugsnag/Configuration.swift +++ b/Sources/Bugsnag/Configuration.swift @@ -4,7 +4,7 @@ import HTTP public protocol ConfigurationType { var apiKey: String { get } - var notifyReleaseStages: [String] { get } + var notifyReleaseStages: [String]? { get } var endpoint: String { get } var filters: [String] { get } @@ -28,7 +28,7 @@ public struct Configuration: ConfigurationType { } public let apiKey: String - public let notifyReleaseStages: [String] + public let notifyReleaseStages: [String]? public let endpoint: String public let filters: [String] diff --git a/Sources/Bugsnag/PayloadTransformer.swift b/Sources/Bugsnag/PayloadTransformer.swift index fda472d..c390801 100644 --- a/Sources/Bugsnag/PayloadTransformer.swift +++ b/Sources/Bugsnag/PayloadTransformer.swift @@ -3,6 +3,9 @@ import Stacked import HTTP public protocol PayloadTransformerType { + var environment: Environment { get } + var apiKey: String { get } + func payloadFor( message: String, metadata: Node?, @@ -13,14 +16,9 @@ public protocol PayloadTransformerType { } internal struct PayloadTransformer: PayloadTransformerType { - let drop: Droplet - let config: ConfigurationType + let environment: Environment + let apiKey: String - init(drop: Droplet, config: ConfigurationType) { - self.drop = drop - self.config = config - } - internal func payloadFor( message: String, metadata: Node?, @@ -48,7 +46,7 @@ internal struct PayloadTransformer: PayloadTransformerType { ]) let app: Node = Node([ - "releaseStage": Node(drop.environment.description), + "releaseStage": Node(environment.description), "type": "Vapor" ]) @@ -91,7 +89,7 @@ internal struct PayloadTransformer: PayloadTransformerType { ]) return try JSON(node: [ - "apiKey": self.config.apiKey, + "apiKey": apiKey, "notifier": Node([ "name": "Bugsnag Vapor", "version": "1.0.11", diff --git a/Sources/Bugsnag/Reporter.swift b/Sources/Bugsnag/Reporter.swift index 3142cbf..4c664ed 100644 --- a/Sources/Bugsnag/Reporter.swift +++ b/Sources/Bugsnag/Reporter.swift @@ -38,8 +38,8 @@ public final class Reporter: ReporterType { config: config ) self.payloadTransformer = transformer ?? PayloadTransformer( - drop: drop, - config: config + environment: drop.environment, + apiKey: config.apiKey ) } @@ -54,7 +54,10 @@ public final class Reporter: ReporterType { completion complete: (() -> ())? ) throws { if let error = error as? AbortError { - guard error.metadata?["report"]?.bool ?? true else { + guard + error.metadata?["report"]?.bool ?? true, + shouldNotifyForReleaseStage() + else { return } try self.report( @@ -100,4 +103,11 @@ public final class Reporter: ReporterType { if let complete = complete { complete() } } } + + private func shouldNotifyForReleaseStage() -> Bool { + guard let notifyReleaseStages = config.notifyReleaseStages else { + return true + } + return notifyReleaseStages.contains(drop.environment.description) + } } diff --git a/Tests/BugsnagTests/Mocks/ConfigurationMock.swift b/Tests/BugsnagTests/Mocks/ConfigurationMock.swift index 8504083..b3f9465 100644 --- a/Tests/BugsnagTests/Mocks/ConfigurationMock.swift +++ b/Tests/BugsnagTests/Mocks/ConfigurationMock.swift @@ -3,10 +3,17 @@ import Bugsnag internal class ConfigurationMock: ConfigurationType { let apiKey = "1337" - let notifyReleaseStages: [String] = [] + let notifyReleaseStages: [String]? let endpoint = "some-endpoint" let filters: [String] = ["someFilter"] - required init(drop: Droplet) throws {} - init() {} + required init(drop: Droplet) throws { + notifyReleaseStages = [] + } + init() { + notifyReleaseStages = ["mock-environment"] + } + init(releaseStages: [String]?) { + notifyReleaseStages = releaseStages + } } diff --git a/Tests/BugsnagTests/Mocks/PayloadTransformerMock.swift b/Tests/BugsnagTests/Mocks/PayloadTransformerMock.swift index ae5e70e..dea49f0 100644 --- a/Tests/BugsnagTests/Mocks/PayloadTransformerMock.swift +++ b/Tests/BugsnagTests/Mocks/PayloadTransformerMock.swift @@ -3,14 +3,14 @@ import Bugsnag import HTTP internal class PayloadTransformerMock: PayloadTransformerType { - let drop: Droplet - let config: ConfigurationType - var lastPayloadData: (message: String, metadata: Node?, request: Request?, severity: Severity, filters: [String])? = nil + let environment: Environment + let apiKey: String + var lastPayloadData: (message: String, metadata: Node?, request: Request?, severity: Severity, filters: [String])? = nil - required init(drop: Droplet, config: ConfigurationType) { - self.drop = drop - self.config = config + init(environment: Environment, apiKey: String) { + self.environment = environment + self.apiKey = apiKey } internal func payloadFor( diff --git a/Tests/BugsnagTests/PayloadTransformerTests.swift b/Tests/BugsnagTests/PayloadTransformerTests.swift index 13af41c..64f2ddd 100644 --- a/Tests/BugsnagTests/PayloadTransformerTests.swift +++ b/Tests/BugsnagTests/PayloadTransformerTests.swift @@ -22,16 +22,7 @@ class PayloadTransformerTests: XCTestCase { ] override func setUp() { - let drop = Droplet( - arguments: nil, - workDir: nil, - environment: Environment.custom("mock-environment"), - config: nil, - localization: nil, - log: nil - ) - let config = ConfigurationMock() - self.payloadTransformer = PayloadTransformer(drop: drop, config: config) + self.payloadTransformer = PayloadTransformer(environment: .custom("mock-environment"), apiKey: "1337") let req = try! Request(method: .get, uri: "http://some-random-url.com/payload-test") req.parameters = ["url": "value"] req.query = ["query": "value"] diff --git a/Tests/BugsnagTests/ReporterTests.swift b/Tests/BugsnagTests/ReporterTests.swift index 63990b8..dddbe5b 100644 --- a/Tests/BugsnagTests/ReporterTests.swift +++ b/Tests/BugsnagTests/ReporterTests.swift @@ -17,15 +17,29 @@ class ReporterTests: XCTestCase { ("testThatThePayloadGetsSubmitted", testThatThePayloadGetsSubmitted), ("testThatFiltersComeFromConfig", testThatFiltersComeFromConfig), ("testSeverityGetsDefaultValue", testSeverityGetsDefaultValue), - ("testSeverityGetsGivenValue", testSeverityGetsGivenValue) + ("testSeverityGetsGivenValue", testSeverityGetsGivenValue), + ("testErrorNotReportedWhenEnvironmentNotInNotifyReleaseStages", testErrorNotReportedWhenEnvironmentNotInNotifyReleaseStages), + ("testErrorReportedWhenEnvironmentInNotifyReleaseStages", testErrorReportedWhenEnvironmentInNotifyReleaseStages), + ("testErrorNotBeingReportedWhenEmptyReleaseStages", testErrorNotBeingReportedWhenEmptyReleaseStages), + ("testErrorBeingReportedWhenNilReleaseStages", testErrorBeingReportedWhenNilReleaseStages) ] override func setUp() { - let drop = Droplet() + let drop = Droplet( + arguments: nil, + workDir: nil, + environment: .custom("mock-environment"), + config: nil, + localization: nil, + log: nil + ) let config = ConfigurationMock() self.connectionManager = ConnectionManagerMock(drop: drop, config: config) - self.payloadTransformer = PayloadTransformerMock(drop: drop, config: config) + self.payloadTransformer = PayloadTransformerMock( + environment: drop.environment, + apiKey: "1337" + ) self.reporter = Reporter( drop: drop, config: config, @@ -192,6 +206,90 @@ class ReporterTests: XCTestCase { XCTAssertEqual(self.payloadTransformer.lastPayloadData?.3, Severity.info) } + func testErrorNotReportedWhenEnvironmentNotInNotifyReleaseStages() { + let drop = Droplet( + arguments: nil, + workDir: nil, + environment: .development, //currentEnvironment = "development" + config: nil, + localization: nil, + log: nil + ) + let conf = ConfigurationMock() //notifyReleaseStages = ["mock-environment"] + let repo = Reporter( + drop: drop, + config: conf, + connectionManager: self.connectionManager, + transformer: self.payloadTransformer + ) + try! repo.report(error: Abort.badRequest, request: nil) + XCTAssertNil(self.payloadTransformer.lastPayloadData) + + } + + func testErrorReportedWhenEnvironmentInNotifyReleaseStages() { + let drop = Droplet( + arguments: nil, + workDir: nil, + environment: .custom("mock-environment"), //currentEnvironment = "mock-environment" + config: nil, + localization: nil, + log: nil + ) + let conf = ConfigurationMock() //notifyReleaseStages = ["mock-environment"] + let repo = Reporter( + drop: drop, + config: conf, + connectionManager: self.connectionManager, + transformer: self.payloadTransformer + ) + try! repo.report(error: Abort.badRequest, request: nil) + XCTAssertNotNil(self.payloadTransformer.lastPayloadData) + + } + + func testErrorNotBeingReportedWhenEmptyReleaseStages() { + let drop = Droplet( + arguments: nil, + workDir: nil, + environment: .custom("mock-environment"), //currentEnvironment = "mock-environment" + config: nil, + localization: nil, + log: nil + ) + let conf = ConfigurationMock(releaseStages: []) //notifyReleaseStages = [] + let repo = Reporter( + drop: drop, + config: conf, + connectionManager: self.connectionManager, + transformer: self.payloadTransformer + ) + try! repo.report(error: Abort.badRequest, request: nil) + XCTAssertNil(self.payloadTransformer.lastPayloadData) + + } + + func testErrorBeingReportedWhenNilReleaseStages() { + let drop = Droplet( + arguments: nil, + workDir: nil, + environment: .custom("mock-environment"), //currentEnvironment = "mock-environment" + config: nil, + localization: nil, + log: nil + ) + let conf = ConfigurationMock(releaseStages: nil) //notifyReleaseStages = nil + let repo = Reporter( + drop: drop, + config: conf, + connectionManager: self.connectionManager, + transformer: self.payloadTransformer + ) + try! repo.report(error: Abort.badRequest, request: nil) + XCTAssertNotNil(self.payloadTransformer.lastPayloadData) + + } + // MARK: - Submission func testThatThePayloadGetsSubmitted() { From 7810043bc18f3bd298aa55b9f9dd971aa1be606d Mon Sep 17 00:00:00 2001 From: Valentin Corral Date: Tue, 11 Apr 2017 17:10:21 +0200 Subject: [PATCH 2/2] Comments changes --- .../Mocks/ConfigurationMock.swift | 10 +++---- Tests/BugsnagTests/ReporterTests.swift | 26 +++++++++---------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/Tests/BugsnagTests/Mocks/ConfigurationMock.swift b/Tests/BugsnagTests/Mocks/ConfigurationMock.swift index b3f9465..639e0e7 100644 --- a/Tests/BugsnagTests/Mocks/ConfigurationMock.swift +++ b/Tests/BugsnagTests/Mocks/ConfigurationMock.swift @@ -7,13 +7,11 @@ internal class ConfigurationMock: ConfigurationType { let endpoint = "some-endpoint" let filters: [String] = ["someFilter"] - required init(drop: Droplet) throws { - notifyReleaseStages = [] + required convenience init(drop: Droplet) throws { + self.init() } - init() { - notifyReleaseStages = ["mock-environment"] - } - init(releaseStages: [String]?) { + + init(releaseStages: [String]? = ["mock-environment"]) { notifyReleaseStages = releaseStages } } diff --git a/Tests/BugsnagTests/ReporterTests.swift b/Tests/BugsnagTests/ReporterTests.swift index dddbe5b..2d71032 100644 --- a/Tests/BugsnagTests/ReporterTests.swift +++ b/Tests/BugsnagTests/ReporterTests.swift @@ -24,15 +24,14 @@ class ReporterTests: XCTestCase { ("testErrorBeingReportedWhenNilReleaseStages", testErrorBeingReportedWhenNilReleaseStages) ] - override func setUp() { let drop = Droplet( - arguments: nil, - workDir: nil, - environment: .custom("mock-environment"), - config: nil, - localization: nil, - log: nil + arguments: nil, + workDir: nil, + environment: .custom("mock-environment"), + config: nil, + localization: nil, + log: nil ) let config = ConfigurationMock() self.connectionManager = ConnectionManagerMock(drop: drop, config: config) @@ -181,7 +180,6 @@ class ReporterTests: XCTestCase { waitForExpectations(timeout: 1) } - func testThatFiltersComeFromConfig() { let req = try! Request(method: .get, uri: "some-random-uri") @@ -208,12 +206,12 @@ class ReporterTests: XCTestCase { func testErrorNotReportedWhenEnvironmentNotInNotifyReleaseStages() { let drop = Droplet( - arguments: nil, - workDir: nil, - environment: .development, //currentEnvironment = "development" - config: nil, - localization: nil, - log: nil + arguments: nil, + workDir: nil, + environment: .development, //currentEnvironment = "development" + config: nil, + localization: nil, + log: nil ) let conf = ConfigurationMock() //notifyReleaseStages = ["mock-environment"] let repo = Reporter(