Skip to content

Commit

Permalink
Fix failing tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
Fabian Zwick committed Oct 20, 2024
1 parent c00a7c7 commit cda6b2e
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 66 deletions.
9 changes: 6 additions & 3 deletions Source/ID3TagEditor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

/**
Expand Down
61 changes: 40 additions & 21 deletions Source/Mp3/Mp3FileReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand All @@ -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
}
}
6 changes: 4 additions & 2 deletions Source/Mp3/Mp3FileReaderFactory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
64 changes: 48 additions & 16 deletions Source/Mp3/Mp3FileWriter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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))
}
}

Expand Down
36 changes: 21 additions & 15 deletions Tests/Mp3/Mp3FileReaderTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
]
}
4 changes: 2 additions & 2 deletions Tests/Mp3/Mp3FileWriterTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 1 addition & 7 deletions Tests/Parse/ID3TagSizeParserTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
]
}

0 comments on commit cda6b2e

Please sign in to comment.