From cda6b2e4d396e588117a0b07d75863743c7ca26a Mon Sep 17 00:00:00 2001 From: Fabian Zwick Date: Sun, 20 Oct 2024 21:26:50 +0200 Subject: [PATCH] Fix failing tests. --- Source/ID3TagEditor.swift | 9 ++-- Source/Mp3/Mp3FileReader.swift | 61 +++++++++++++++--------- Source/Mp3/Mp3FileReaderFactory.swift | 6 ++- Source/Mp3/Mp3FileWriter.swift | 64 +++++++++++++++++++------- Tests/Mp3/Mp3FileReaderTest.swift | 36 +++++++++------ Tests/Mp3/Mp3FileWriterTest.swift | 4 +- Tests/Parse/ID3TagSizeParserTest.swift | 8 +--- 7 files changed, 122 insertions(+), 66 deletions(-) diff --git a/Source/ID3TagEditor.swift b/Source/ID3TagEditor.swift index 12c7b7b7..c4c2e8c8 100644 --- a/Source/ID3TagEditor.swift +++ b/Source/ID3TagEditor.swift @@ -40,7 +40,10 @@ public class ID3TagEditor { Could throw `CorruptedFile` if the file is corrupted. */ public func read(from path: String) throws -> ID3Tag? { - let mp3 = try mp3FileReader.readID3TagFrom(path: path) + guard let mp3 = try mp3FileReader.readID3TagFrom(path: path) else { + return nil + } + return try self.id3TagParser.parse(mp3: mp3) } @@ -70,9 +73,9 @@ public class ID3TagEditor { ID3 tag). */ public func write(tag: ID3Tag, to path: String, andSaveTo newPath: String? = nil) throws { - let currentId3Tag = try read(from: path) + let currentId3TagData = try mp3FileReader.readID3TagFrom(path: path) let newId3TagData = try id3TagCreator.create(id3Tag: tag) - try mp3FileWriter.write(newId3TagData: newId3TagData, currentId3Tag: currentId3Tag, fromPath: path, toPath: newPath ?? path) + try mp3FileWriter.write(newId3TagData: newId3TagData, currentId3TagData: currentId3TagData, fromPath: path, toPath: newPath ?? path) } /** diff --git a/Source/Mp3/Mp3FileReader.swift b/Source/Mp3/Mp3FileReader.swift index ef8cc159..126dfff6 100644 --- a/Source/Mp3/Mp3FileReader.swift +++ b/Source/Mp3/Mp3FileReader.swift @@ -10,11 +10,17 @@ import Foundation class Mp3FileReader { private let tagSizeParser: TagSizeParser private let id3TagConfiguration: ID3TagConfiguration + private let tagVersionParser: TagVersionParser + private let tagPresence: TagPresence init(tagSizeParser: TagSizeParser, - id3TagConfiguration: ID3TagConfiguration) { + id3TagConfiguration: ID3TagConfiguration, + tagVersionParser: TagVersionParser, + tagPresence: TagPresence) { self.tagSizeParser = tagSizeParser self.id3TagConfiguration = id3TagConfiguration + self.tagVersionParser = tagVersionParser + self.tagPresence = tagPresence } /** @@ -41,41 +47,54 @@ class Mp3FileReader { - parameter path: the path to the mp3 file - - returns: ID3 header data of the file + - returns: ID3 header data or nil, if a tag doesn't exists in the file. - - throws: Could throw `InvalidFileFormat` if an mp3 file doesn't exists at the specified path, or if the file - does not contain the entire ID3 header + - throws: Could throw `InvalidFileFormat` if an mp3 file doesn't exists at the specified path. + Could throw `CorruptedFile` if the file is corrupted. */ - func readID3TagFrom(path: String) throws -> Data { + func readID3TagFrom(path: String) throws -> Data? { let validPath = URL(fileURLWithPath: path) guard validPath.pathExtension.caseInsensitiveCompare("mp3") == ComparisonResult.orderedSame else { throw ID3TagEditorError.invalidFileFormat } - guard let inputStream = InputStream(fileAtPath: path) else { - throw ID3TagEditorError.corruptedFile + let readHandle = try FileHandle(forReadingFrom: URL(fileURLWithPath: path)) + defer { + if #available(iOS 13.0, *) { + try? readHandle.close() + } else { + readHandle.closeFile() + } } - inputStream.open() - defer { inputStream.close() } - let headerSize = id3TagConfiguration.headerSize() - let header = try read(bytesCount: headerSize, fromStream: inputStream) - let headerData = Data(header) as NSData + let header = try read(bytesCount: headerSize, from: readHandle) - let frameSize = tagSizeParser.parse(data: headerData) - let frame = try read(bytesCount: Int(frameSize), fromStream: inputStream) + // Verify that there is a valid ID3 tag to parse the size from + let version = tagVersionParser.parse(mp3: header) + guard tagPresence.isTagPresentIn(mp3: header, version: version) else { + return nil + } - let mp3 = header + frame - return Data(mp3) + let frameSize = tagSizeParser.parse(data: header as NSData) + let frame = try read(bytesCount: Int(frameSize), from: readHandle) + + return header + frame } - private func read(bytesCount: Int, fromStream stream: InputStream) throws -> [UInt8] { - var buffer = [UInt8](repeating: 0, count: bytesCount) - let result = stream.read(&buffer, maxLength: bytesCount) - if result < bytesCount { + private func read(bytesCount: Int, from fileHandle: FileHandle) throws -> Data { + let result = try { + if #available(iOS 13.4, *) { + return try fileHandle.read(upToCount: bytesCount) + } else { + return fileHandle.readData(ofLength: bytesCount) + } + }() + + guard let result, result.count == bytesCount else { throw ID3TagEditorError.corruptedFile } - return buffer + + return result } } diff --git a/Source/Mp3/Mp3FileReaderFactory.swift b/Source/Mp3/Mp3FileReaderFactory.swift index d1a322c9..f72b5236 100644 --- a/Source/Mp3/Mp3FileReaderFactory.swift +++ b/Source/Mp3/Mp3FileReaderFactory.swift @@ -13,8 +13,10 @@ class Mp3FileReaderFactory { let tagSizeParser = ID3TagSizeParser() let id3TagConfiguration = ID3TagConfiguration() let fileReader = Mp3FileReader(tagSizeParser: tagSizeParser, - id3TagConfiguration: id3TagConfiguration) - + id3TagConfiguration: id3TagConfiguration, + tagVersionParser: ID3TagVersionParser(), + tagPresence: ID3TagPresence(id3TagConfiguration: id3TagConfiguration)) + return fileReader } } diff --git a/Source/Mp3/Mp3FileWriter.swift b/Source/Mp3/Mp3FileWriter.swift index 53bd334b..acf08b3b 100644 --- a/Source/Mp3/Mp3FileWriter.swift +++ b/Source/Mp3/Mp3FileWriter.swift @@ -8,7 +8,12 @@ import Foundation class Mp3FileWriter { - func write(newId3TagData: Data, currentId3Tag: ID3Tag?, fromPath: String, toPath: String) throws { + func write(newId3TagData: Data, currentId3TagData: Data?, fromPath: String, toPath: String) throws { + let validPath = URL(fileURLWithPath: toPath) + guard validPath.pathExtension.caseInsensitiveCompare("mp3") == ComparisonResult.orderedSame else { + throw ID3TagEditorError.invalidFileFormat + } + // Create a temporary file for the new mp3 let temporaryPath = { if toPath != fromPath { @@ -30,36 +35,63 @@ class Mp3FileWriter { // Create file handles let readHandle = try FileHandle(forReadingFrom: URL(fileURLWithPath: fromPath)) defer { - readHandle.closeFile() + if #available(iOS 13.0, *) { + try? readHandle.close() + } else { + readHandle.closeFile() + } } - + let writeHandle = try FileHandle(forWritingTo: URL(fileURLWithPath: temporaryPath)) defer { - writeHandle.closeFile() + if #available(iOS 13.0, *) { + try? writeHandle.close() + } else { + writeHandle.closeFile() + } } - + // Seek over the tag of the existing file, then copy the rest in chunks - writeHandle.seekToEndOfFile() - - if let validCurrentId3Tag = currentId3Tag { - let tagSizeWithHeader = UInt64(validCurrentId3Tag.properties.size) + UInt64(ID3TagConfiguration().headerSize()) - readHandle.seek(toFileOffset: tagSizeWithHeader) + if #available(iOS 13.4, *) { + try writeHandle.seekToEnd() } else { - readHandle.seek(toFileOffset: 0) + writeHandle.seekToEndOfFile() + } + + if let currentId3TagData = currentId3TagData { + if #available(iOS 13.0, *) { + try readHandle.seek(toOffset: UInt64(currentId3TagData.count)) + } else { + readHandle.seek(toFileOffset: UInt64(currentId3TagData.count)) + } } var isFinished = false while !isFinished { - autoreleasepool { - let chunk = readHandle.readData(ofLength: 65536) // 64 KB - writeHandle.write(chunk) - isFinished = chunk.isEmpty + try autoreleasepool { + let chunk = try { + if #available(iOS 13.4, *) { + return try readHandle.read(upToCount: 131072) // 128 KB + } else { + return readHandle.readData(ofLength: 131072) // 128 KB + } + }() + + if let chunk, !chunk.isEmpty { + if #available(iOS 13.4, *) { + try writeHandle.write(contentsOf: chunk) + } else { + writeHandle.write(chunk) + } + } else { + isFinished = true + } } } // Replace the file if temporaryPath != toPath { - _ = try FileManager.default.replaceItemAt(URL(fileURLWithPath: toPath), withItemAt: URL(fileURLWithPath: temporaryPath)) + _ = try FileManager.default.replaceItemAt(validPath, withItemAt: URL(fileURLWithPath: temporaryPath)) } } diff --git a/Tests/Mp3/Mp3FileReaderTest.swift b/Tests/Mp3/Mp3FileReaderTest.swift index b9c96e17..91ff4c10 100644 --- a/Tests/Mp3/Mp3FileReaderTest.swift +++ b/Tests/Mp3/Mp3FileReaderTest.swift @@ -11,61 +11,67 @@ import Testing struct Mp3FileReaderTest { @Test func testNotAnMP3FileWhenReadingEntireFile() { let path = PathLoader().pathFor(name: "example-cover", fileType: "jpg") - let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(), - id3TagConfiguration: ID3TagConfiguration()) + let mp3FileReader = Mp3FileReaderFactory.make() #expect(throws: ID3TagEditorError.invalidFileFormat.self) { try mp3FileReader.readFileFrom(path: path) } } @Test func testMP3FileWhenReadingEntireFile() { let path = PathLoader().pathFor(name: "example", fileType: "mp3") - let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(), - id3TagConfiguration: ID3TagConfiguration()) + let mp3FileReader = Mp3FileReaderFactory.make() #expect(throws: Never.self) { try mp3FileReader.readFileFrom(path: path) } } @Test func testNotAnMP3fileWhenReadingID3Tag() { let path = PathLoader().pathFor(name: "example-cover", fileType: "jpg") - let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(), - id3TagConfiguration: ID3TagConfiguration()) + let mp3FileReader = Mp3FileReaderFactory.make() #expect(throws: ID3TagEditorError.invalidFileFormat.self) { try mp3FileReader.readID3TagFrom(path: path) } } @Test func testMP3fileWhenReadingID3Tag() { let path = PathLoader().pathFor(name: "example", fileType: "mp3") - let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(), - id3TagConfiguration: ID3TagConfiguration()) + let mp3FileReader = Mp3FileReaderFactory.make() #expect(throws: Never.self) { try mp3FileReader.readID3TagFrom(path: path) } } @Test func testNonExistentMP3fileWhenReadingID3Tag() { let path = "/non-existent.mp3" - let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(), - id3TagConfiguration: ID3TagConfiguration()) + let mp3FileReader = Mp3FileReaderFactory.make() - #expect(throws: ID3TagEditorError.corruptedFile.self) { try mp3FileReader.readID3TagFrom(path: path) } + #expect(throws: Error.self) { try mp3FileReader.readFileFrom(path: path) } + #expect(throws: Error.self) { try mp3FileReader.readID3TagFrom(path: path) } } @Test func testOnlyReadsID3Tag() throws { let path = PathLoader().pathFor(name: "example", fileType: "mp3") - let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(), - id3TagConfiguration: ID3TagConfiguration()) + let mp3FileReader = Mp3FileReaderFactory.make() - let id3TagData = try mp3FileReader.readID3TagFrom(path: path) + let id3TagData = try #require(try mp3FileReader.readID3TagFrom(path: path)) // 10 bytes Tag + 34213 bytes according to the Tag Size in the file's ID3 Tag #expect(id3TagData.count == 10 + 34213) } + @Test func testIgnoresWhenMissingID3Tag() throws { + let path = PathLoader().pathFor(name: "example-to-be-modified", fileType: "mp3") + let mp3FileReader = Mp3FileReaderFactory.make() + + let id3TagData = try mp3FileReader.readID3TagFrom(path: path) + + // The file has no ID3 tag + #expect(id3TagData == nil) + } + static let allTests = [ ("testNotAnMP3FileWhenReadingEntireFile", testNotAnMP3FileWhenReadingEntireFile), ("testMP3FileWhenReadingEntireFile", testMP3FileWhenReadingEntireFile), ("testNotAnMP3fileWhenReadingID3Tag", testNotAnMP3fileWhenReadingID3Tag), ("testMP3fileWhenReadingID3Tag", testMP3fileWhenReadingID3Tag), ("testNonExistentMP3fileWhenReadingID3Tag", testNonExistentMP3fileWhenReadingID3Tag), - ("testOnlyReadsID3Tag", testOnlyReadsID3Tag) + ("testOnlyReadsID3Tag", testOnlyReadsID3Tag), + ("testIgnoresWhenMissingID3Tag", testIgnoresWhenMissingID3Tag) ] } diff --git a/Tests/Mp3/Mp3FileWriterTest.swift b/Tests/Mp3/Mp3FileWriterTest.swift index eb2a9775..a10f2ac8 100644 --- a/Tests/Mp3/Mp3FileWriterTest.swift +++ b/Tests/Mp3/Mp3FileWriterTest.swift @@ -21,10 +21,10 @@ struct Mp3FileWriterTest { let newId3TagData = try id3TagCreator.create(id3Tag: newTag) let fromPath = PathLoader().pathFor(name: "example", fileType: "mp3") - let currentId3Tag = try ID3TagEditor().read(from: fromPath) + let currentId3TagData = try Mp3FileReaderFactory.make().readID3TagFrom(path: fromPath) let pathMp3Generated = NSHomeDirectory() + "/testWritingWithCurrentTagAndEqualToPath.mp3" - #expect(throws: Never.self) { try Mp3FileWriter().write(newId3TagData: newId3TagData, currentId3Tag: currentId3Tag, fromPath: fromPath, toPath: pathMp3Generated) } + #expect(throws: Never.self) { try Mp3FileWriter().write(newId3TagData: newId3TagData, currentId3TagData: currentId3TagData, fromPath: fromPath, toPath: pathMp3Generated) } let readTag = try ID3TagEditor().read(from: pathMp3Generated) #expect((readTag?.frames[.title] as? ID3FrameWithStringContent)?.content == "Test Title") diff --git a/Tests/Parse/ID3TagSizeParserTest.swift b/Tests/Parse/ID3TagSizeParserTest.swift index 6c9a038f..b19d0b40 100644 --- a/Tests/Parse/ID3TagSizeParserTest.swift +++ b/Tests/Parse/ID3TagSizeParserTest.swift @@ -21,14 +21,8 @@ struct ID3TagSizeParserTest { #expect(ID3TagSizeParser().parse(data: mp3V23) == 245864) } - @Test func parseTagSizeToBeModified() { - let mp3 = NSData(contentsOfFile: PathLoader().pathFor(name: "example-to-be-modified", fileType: "mp3"))! - #expect(ID3TagSizeParser().parse(data: mp3) < mp3.count) - } - static let allTests = [ ("parseTagSizeV2", parseTagSizeV2), - ("parseFrameContentSizeV3", parseTagSizeV3), - ("parseTagSizeToBeModified", parseTagSizeToBeModified) + ("parseFrameContentSizeV3", parseTagSizeV3) ] }