From 4cc5ac06a1db735983a42b31d710426162ea33f8 Mon Sep 17 00:00:00 2001 From: RomanTysiachnik Date: Fri, 8 Feb 2019 19:01:56 +0200 Subject: [PATCH 1/2] Added parsing of skip offset for VAST ad --- PlayerCore | 2 +- .../project.pbxproj | 8 +++--- .../advertisements/MidrollDetectorTests.swift | 1 + sources/advertisements/VAST1.xml | 4 +-- sources/advertisements/VASTModel.swift | 1 + sources/advertisements/VASTParser.swift | 28 +++++++++++++++++++ sources/advertisements/VASTParserTests.swift | 18 ++++++++++-- ...nishVRMGroupProcessingControllerTest.swift | 1 + .../ParseVRMItemControllerTest.swift | 1 + .../VRMProcessingControllerTest.swift | 1 + .../VRMSelectFinalResultControllerTest.swift | 10 ++++--- sources/advertisements/VerifyBuldTests.swift | 1 + 12 files changed, 63 insertions(+), 13 deletions(-) diff --git a/PlayerCore b/PlayerCore index 1dc6d5f..0f96e27 160000 --- a/PlayerCore +++ b/PlayerCore @@ -1 +1 @@ -Subproject commit 1dc6d5f7e98498ce8e6b2d30b189d843f599a9c4 +Subproject commit 0f96e27d2ddf6147ca1395dba030083da5f7f139 diff --git a/VerizonVideoPartnerSDK.xcodeproj/project.pbxproj b/VerizonVideoPartnerSDK.xcodeproj/project.pbxproj index 705191d..ef20760 100644 --- a/VerizonVideoPartnerSDK.xcodeproj/project.pbxproj +++ b/VerizonVideoPartnerSDK.xcodeproj/project.pbxproj @@ -1070,15 +1070,15 @@ DE1685D71CE7308500492A90 /* vast parser */ = { isa = PBXGroup; children = ( + DEF02B511D1EC0AD001F64EE /* VASTModel.swift */, DE1685DC1CE732D600492A90 /* VASTParser.swift */, DE1685E61CE76A5D00492A90 /* VASTParserTests.swift */, - DE1685E41CE768E600492A90 /* VASTExampleCDATA.xml */, - DE1FD9CE1CE7737900C0B7BC /* VASTMissedCorrectType.xml */, DE4539411D09644D00317632 /* XMLParser.swift */, - DEF02B511D1EC0AD001F64EE /* VASTModel.swift */, DEF02B581D22970A001F64EE /* VAST1.xml */, - 06BF541620A062E300B98A6D /* VASTVpaid.xml */, + DE1685E41CE768E600492A90 /* VASTExampleCDATA.xml */, + DE1FD9CE1CE7737900C0B7BC /* VASTMissedCorrectType.xml */, 067209D921B6BE710086CDBE /* VASTVerificationInExtension.xml */, + 06BF541620A062E300B98A6D /* VASTVpaid.xml */, B418F5461ECB267B00C9131D /* VASTWrapper.xml */, 067209DA21B6BE710086CDBE /* VASTWrapperWithExtension.xml */, ); diff --git a/sources/advertisements/MidrollDetectorTests.swift b/sources/advertisements/MidrollDetectorTests.swift index 58cded5..3235c71 100644 --- a/sources/advertisements/MidrollDetectorTests.swift +++ b/sources/advertisements/MidrollDetectorTests.swift @@ -57,6 +57,7 @@ class MidrollDetectorTests: QuickSpec { scalable: true, maintainAspectRatio: true)], vpaidMediaFiles: [], + skipOffset: .none, clickthrough: nil, adParameters: nil, pixels: .init(), diff --git a/sources/advertisements/VAST1.xml b/sources/advertisements/VAST1.xml index ffd2030..e7ee1d5 100644 --- a/sources/advertisements/VAST1.xml +++ b/sources/advertisements/VAST1.xml @@ -21,8 +21,8 @@ - - 00:00:30 + + 02:00:30 diff --git a/sources/advertisements/VASTModel.swift b/sources/advertisements/VASTModel.swift index e2c3136..f751481 100644 --- a/sources/advertisements/VASTModel.swift +++ b/sources/advertisements/VASTModel.swift @@ -23,6 +23,7 @@ extension PlayerCore.Ad.VASTModel { adVerifications: self.adVerifications + verifications, mp4MediaFiles: mp4MediaFiles, vpaidMediaFiles: vpaidMediaFiles, + skipOffset: skipOffset, clickthrough: clickthrough, adParameters: adParameters, pixels: self.pixels.merge(with: pixels), diff --git a/sources/advertisements/VASTParser.swift b/sources/advertisements/VASTParser.swift index 96fcc1f..d464326 100644 --- a/sources/advertisements/VASTParser.swift +++ b/sources/advertisements/VASTParser.swift @@ -48,6 +48,7 @@ enum VASTParser { var adParameters: String? var mp4MediaFiles: [PlayerCore.Ad.VASTModel.MP4MediaFile] = [] var vpaidMediaFiles: [PlayerCore.Ad.VASTModel.VPAIDMediaFile] = [] + var skipOffset: PlayerCore.Ad.VASTModel.SkipOffset = .none } struct AdVerification { @@ -134,6 +135,16 @@ enum VASTParser { } delegate.didStartElement = .some { (name, attr) -> Void in + if let skipOffset = attr["skipoffset"] { + if skipOffset.contains("%") { + guard let value = Int(skipOffset.replacingOccurrences(of: "%", with: "")) else { return } + inlineContext.skipOffset = .percentage(value) + } else if skipOffset.contains(":") { + guard let value = parseTime(from: skipOffset) else { return } + inlineContext.skipOffset = .time(value) + } + } + switch name { case "Extensions": delegateStack.push(XML.Delegate(setup: { delegate in @@ -449,6 +460,7 @@ enum VASTParser { with: adId, to: { context in delegateStack.pop() + guard result == nil else { fatalError("Result overwrite detected") } guard !context.vpaidMediaFiles.isEmpty || !context.mp4MediaFiles.isEmpty else { return } @@ -466,6 +478,7 @@ enum VASTParser { let model = PlayerCore.Ad.VASTModel(adVerifications: adVerifications, mp4MediaFiles: context.mp4MediaFiles, vpaidMediaFiles: context.vpaidMediaFiles, + skipOffset: context.skipOffset, clickthrough: context.clickthroughURL, adParameters: context.adParameters, pixels: context.pixels, @@ -491,6 +504,21 @@ enum VASTParser { return delegateStack } + + static func parseTime(from string: String) -> Double? { + let components = string.components(separatedBy: ":") + guard components.count == 3 else { return nil } + return components + .compactMap(Double.init) + .enumerated() + .map { + let multipliers: [Double] = [3600, 60, 1] + return $0.element * multipliers[$0.offset] + } + .reduce(0.0, +) + } + + //swiftlint:disable line_length //swiftlint:enable function_body_length //swiftlint:enable cyclomatic_complexity diff --git a/sources/advertisements/VASTParserTests.swift b/sources/advertisements/VASTParserTests.swift index b5fb9d5..99adccf 100644 --- a/sources/advertisements/VASTParserTests.swift +++ b/sources/advertisements/VASTParserTests.swift @@ -27,7 +27,7 @@ class VASTParserTests: XCTestCase { guard case let .inline(inlineModel) = model else { return XCTFail() } guard let url = inlineModel.mp4MediaFiles.first?.url.absoluteString else { return XCTFail() } XCTAssertEqual(url, - "https://dev.example.com/videos/2018/video_example_1280x720.mp4") + "https://dev.example.com/videos/2018/video_example_1280x720.mp4") XCTAssertEqual(inlineModel.id, "4203085") } @@ -69,7 +69,7 @@ class VASTParserTests: XCTestCase { let vast = getVAST(atPath: "VAST1") guard let model = VASTParser.parseFrom(string: vast) else { return XCTFail() } guard case .inline(let inlineModel) = model else { return XCTFail() } - + XCTAssertEqual(inlineModel.adVerifications.count, 1) guard let adVerification = inlineModel.adVerifications.first else { return XCTFail("Missing AdVerification") } @@ -124,4 +124,18 @@ class VASTParserTests: XCTestCase { XCTAssertEqual(vpaidModel.pixels.close.first?.absoluteString, url + "close.gif") XCTAssertEqual(vpaidModel.pixels.collapse.first?.absoluteString, url + "collapse.gif") } + + func testParseAdSkipInTime() { + let vast = getVAST(atPath: "VAST1") + guard let model = VASTParser.parseFrom(string: vast) else { return XCTFail("Failed to parse VAST VPAID xml") } + guard case let .inline(vpaidModel) = model else { return XCTFail() } + XCTAssertEqual(vpaidModel.skipOffset, .time(3663.123)) + } + + func testParseAdSkipInPersentage() { + let vast = getVAST(atPath: "VASTExampleCDATA") + guard let model = VASTParser.parseFrom(string: vast) else { return XCTFail("Failed to parse VAST VPAID xml") } + guard case let .inline(vpaidModel) = model else { return XCTFail() } + XCTAssertEqual(vpaidModel.skipOffset, .percentage(32)) + } } diff --git a/sources/advertisements/VRM New Core/Controllers/FinishVRMGroupProcessingControllerTest.swift b/sources/advertisements/VRM New Core/Controllers/FinishVRMGroupProcessingControllerTest.swift index 395cef9..1fbe16a 100644 --- a/sources/advertisements/VRM New Core/Controllers/FinishVRMGroupProcessingControllerTest.swift +++ b/sources/advertisements/VRM New Core/Controllers/FinishVRMGroupProcessingControllerTest.swift @@ -41,6 +41,7 @@ class FinishVRMGroupProcessingControllerTest: XCTestCase { adModel = .init(adVerifications: [], mp4MediaFiles: [], vpaidMediaFiles: [], + skipOffset: .none, clickthrough: nil, adParameters: nil, pixels: .init(), diff --git a/sources/advertisements/VRM New Core/Controllers/ParseVRMItemControllerTest.swift b/sources/advertisements/VRM New Core/Controllers/ParseVRMItemControllerTest.swift index 7d6b78f..0e22513 100644 --- a/sources/advertisements/VRM New Core/Controllers/ParseVRMItemControllerTest.swift +++ b/sources/advertisements/VRM New Core/Controllers/ParseVRMItemControllerTest.swift @@ -45,6 +45,7 @@ class ParseVRMItemControllerTest: XCTestCase { let result = PlayerCore.Ad.VASTModel(adVerifications: [], mp4MediaFiles: [], vpaidMediaFiles: [], + skipOffset: .none, clickthrough: nil, adParameters: nil, pixels: AdPixels(), diff --git a/sources/advertisements/VRM New Core/Controllers/VRMProcessingControllerTest.swift b/sources/advertisements/VRM New Core/Controllers/VRMProcessingControllerTest.swift index 8c4257c..bc7b07d 100644 --- a/sources/advertisements/VRM New Core/Controllers/VRMProcessingControllerTest.swift +++ b/sources/advertisements/VRM New Core/Controllers/VRMProcessingControllerTest.swift @@ -41,6 +41,7 @@ class VRMProcessingControllerTest: XCTestCase { adModel = .init(adVerifications: [], mp4MediaFiles: [], vpaidMediaFiles: [], + skipOffset: .none, clickthrough: nil, adParameters: nil, pixels: .init(), diff --git a/sources/advertisements/VRM New Core/Controllers/VRMSelectFinalResultControllerTest.swift b/sources/advertisements/VRM New Core/Controllers/VRMSelectFinalResultControllerTest.swift index b5f6700..b1218cb 100644 --- a/sources/advertisements/VRM New Core/Controllers/VRMSelectFinalResultControllerTest.swift +++ b/sources/advertisements/VRM New Core/Controllers/VRMSelectFinalResultControllerTest.swift @@ -29,14 +29,16 @@ class VRMSelectFinalResultControllerTest: XCTestCase { adModel1 = .init(adVerifications: [], mp4MediaFiles: [], vpaidMediaFiles: [], - clickthrough: nil, - adParameters: nil, - pixels: .init(), - id: "id1") + skipOffset: .none, + clickthrough: nil, + adParameters: nil, + pixels: .init(), + id: "id1") adModel2 = .init(adVerifications: [], mp4MediaFiles: [], vpaidMediaFiles: [], + skipOffset: .none, clickthrough: nil, adParameters: nil, pixels: .init(), diff --git a/sources/advertisements/VerifyBuldTests.swift b/sources/advertisements/VerifyBuldTests.swift index 27c8451..3a74726 100644 --- a/sources/advertisements/VerifyBuldTests.swift +++ b/sources/advertisements/VerifyBuldTests.swift @@ -16,6 +16,7 @@ class VerifyBuldTests: XCTestCase { vpaidMediaFiles: [Ad.VASTModel.VPAIDMediaFile(url: url, scalable: false, maintainAspectRatio: true)], + skipOffset: .none, clickthrough: nil, adParameters: "", pixels: AdPixels(), From 7079d04a8b02ebd107940cd40a4d2ca5493a575d Mon Sep 17 00:00:00 2001 From: RomanTysiachnik Date: Mon, 11 Feb 2019 16:13:10 +0200 Subject: [PATCH 2/2] Implemented logic for invalid time and added test for it --- sources/advertisements/VASTParser.swift | 73 +++++++++++++++---- sources/advertisements/VASTParserTests.swift | 13 +++- .../VASTVerificationInExtension.xml | 2 +- 3 files changed, 69 insertions(+), 19 deletions(-) diff --git a/sources/advertisements/VASTParser.swift b/sources/advertisements/VASTParser.swift index d464326..8bebdb9 100644 --- a/sources/advertisements/VASTParser.swift +++ b/sources/advertisements/VASTParser.swift @@ -137,11 +137,13 @@ enum VASTParser { delegate.didStartElement = .some { (name, attr) -> Void in if let skipOffset = attr["skipoffset"] { if skipOffset.contains("%") { - guard let value = Int(skipOffset.replacingOccurrences(of: "%", with: "")) else { return } - inlineContext.skipOffset = .percentage(value) + if let value = Int(skipOffset.replacingOccurrences(of: "%", with: "")) { + inlineContext.skipOffset = .percentage(value) + } } else if skipOffset.contains(":") { - guard let value = parseTime(from: skipOffset) else { return } - inlineContext.skipOffset = .time(value) + if let value = VASTTime(with: skipOffset)?.seconds { + inlineContext.skipOffset = .time(Double(value)) + } } } @@ -505,19 +507,60 @@ enum VASTParser { return delegateStack } - static func parseTime(from string: String) -> Double? { - let components = string.components(separatedBy: ":") - guard components.count == 3 else { return nil } - return components - .compactMap(Double.init) - .enumerated() - .map { - let multipliers: [Double] = [3600, 60, 1] - return $0.element * multipliers[$0.offset] + struct VASTTime { + enum Time { + case hours([String]) + case minutes([String]) + case seconds([String]) + + private var maxValue: Int { + switch self { + case .hours: return 99 + case .minutes, .seconds: return 59 + } + } + private var multiplier: Int { + switch self { + case .hours: return 3600 + case .minutes: return 60 + case .seconds: return 1 + } + } + private var index: Int { + switch self { + case .hours: return 0 + case .minutes: return 1 + case .seconds: return 2 + } + } + private var stringTime: String { + switch self { + case .hours(let value): return value[self.index] + case .minutes(let value): return value[self.index] + case .seconds(let value): return value[self.index] + } + } + + var resultInSeconds: Int? { + guard let roundedSeconds = Double(self.stringTime)?.rounded() else { return nil } + let result = Int(roundedSeconds) + guard result <= maxValue else { return nil } + return result * multiplier } - .reduce(0.0, +) + + } + + let seconds: Int + + init?(with time: String) { + let components = time.components(separatedBy: ":") + guard components.count == 3, + let hours = Time.hours(components).resultInSeconds, + let minutes = Time.minutes(components).resultInSeconds, + let seconds = Time.seconds(components).resultInSeconds else { return nil } + self.seconds = hours + minutes + seconds + } } - //swiftlint:disable line_length //swiftlint:enable function_body_length diff --git a/sources/advertisements/VASTParserTests.swift b/sources/advertisements/VASTParserTests.swift index 99adccf..bc83e14 100644 --- a/sources/advertisements/VASTParserTests.swift +++ b/sources/advertisements/VASTParserTests.swift @@ -125,17 +125,24 @@ class VASTParserTests: XCTestCase { XCTAssertEqual(vpaidModel.pixels.collapse.first?.absoluteString, url + "collapse.gif") } - func testParseAdSkipInTime() { + func testParseSkipOffsetInTime() { let vast = getVAST(atPath: "VAST1") guard let model = VASTParser.parseFrom(string: vast) else { return XCTFail("Failed to parse VAST VPAID xml") } guard case let .inline(vpaidModel) = model else { return XCTFail() } - XCTAssertEqual(vpaidModel.skipOffset, .time(3663.123)) + XCTAssertEqual(vpaidModel.skipOffset, .time(3663)) } - func testParseAdSkipInPersentage() { + func testParseSkipOffsetInPersentage() { let vast = getVAST(atPath: "VASTExampleCDATA") guard let model = VASTParser.parseFrom(string: vast) else { return XCTFail("Failed to parse VAST VPAID xml") } guard case let .inline(vpaidModel) = model else { return XCTFail() } XCTAssertEqual(vpaidModel.skipOffset, .percentage(32)) } + + func testParseNotValidSkipOffsetInTime() { + let vast = getVAST(atPath: "VASTVerificationInExtension") + guard let model = VASTParser.parseFrom(string: vast) else { return XCTFail("Failed to parse VAST VPAID xml") } + guard case let .inline(vpaidModel) = model else { return XCTFail() } + XCTAssertEqual(vpaidModel.skipOffset, .none) + } } diff --git a/sources/advertisements/VASTVerificationInExtension.xml b/sources/advertisements/VASTVerificationInExtension.xml index 703a778..69e345c 100644 --- a/sources/advertisements/VASTVerificationInExtension.xml +++ b/sources/advertisements/VASTVerificationInExtension.xml @@ -10,7 +10,7 @@ - +