Repository: chicio/ID3TagEditor (swift)

Issue:
Reading ID3 tags of large files
**Is your feature request related to a problem? Please describe.**
- Because you're using `Data(contentsOf: validPath)` inside `Mp3FileReader`, the entire file gets loaded into memory

**Describe the solution you'd like**
- According to documentation, `InputStream` is meant to be used in this case.
- https://developer.apple.com/documentation/foundation/inputstream
- Since the ID3 tags are at the start of the file, it's enough to only read `Int(tagSize) + id3TagConfiguration.headerSize()` worth
- This would involve _some_ amount of code change, to first peak into the file to get the tagSize, and then perform a second read to read the entire ID3 header

**Additional context**
- I'm writing an Audiobook player for myself, and wanted to read the ID3 tags to show some additional information, when viewing the files / playing them.
- Tested this with a 500MB audiobook, all 500 MB of it get loaded into RAM.
- Looked at the tagSize + id3TagConfiguration.headerSize() values, it would be enough to read 588549 bytes, or ~5MB

In fact, if I modify the code, and hardcode this value, it behaves exactly as before:

```swift
    func readFrom(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.invalidFileFormat
        }

        inputStream.open()

        var buffer = [UInt8](repeating: 0, count: 588549)
        let result = inputStream.read(&buffer, maxLength: 588549)

        return Data(buffer)
}
```

----

- Have you ever thought about this problem?
- Are you interested in a contribution that would change this?
    - It would involve some slight refactor to accommodate the 2 read events.

Pull request description:
When reading ID3 Tags, only read the ID3 Header, rather than the entire file.

## Description
- Add a method to Mp3FileReader that only reads the ID3 Header: `readID3HeaderFrom(path:)`
- Rename existing method to `readFileFrom(path:)`
- Modify ID3TagEditor to only read the ID3 header, when reading the file
- Maintain existing reading functionality, when writing ID3 tags

## Motivation and Context
See #98 for the motivation.

During implementation, I discovered that the old reading functionality still need to be retained, for the case where ID3 Tags are being saved:
- When modifying the tags, the the length of the ID3 tags could become shorter or longer
- This would impact there rest of the file, because the actual audio content would need to be shifted
- Which is not possible
- So you need to read the entire file, modify it and shift it around in-memory, when write out the entire file

## How Has This Been Tested?
- Through the Demo app
- Through my own app, that uses ID3TagEditor
- Through unit tests

## Types of changes
- [ ] Bug fix :bug: (non-breaking change which fixes an issue)
- [X] New feature :sparkles: (non-breaking change which adds functionality)
- [ ] Breaking change :boom: (fix or feature that would cause existing functionality to change)

## Checklist:
- [X] My code follows the code style of this project :beers:.
- [ ] My change requires a change to the documentation :bulb: and I have updated the documentation accordingly.
- [X] I have read the [CONTRIBUTING](https://github.com/chicio/ID3TagEditor/blob/master/CONTRIBUTING.md) document :busts_in_silhouette:.
- [X] I have added tests to cover my changes :tada:.
- [X] All new and existing tests passed :white_check_mark:.

Closes #98

Patch:
diff --git a/.github/workflows/build-watchos.yml b/.github/workflows/build-watchos.yml
index 7191d053..3613e33f 100644
--- a/.github/workflows/build-watchos.yml
+++ b/.github/workflows/build-watchos.yml
@@ -17,7 +17,7 @@ jobs:
         uses: actions/checkout@v2
       - name: Build watchOS framework
         run: |
-           set -o pipefail && xcodebuild -project ID3TagEditor.xcodeproj -scheme "ID3TagEditor watchOS" clean build CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO -destination "platform=watchOS Simulator,name=Apple Watch Series 4 - 44mm,OS=latest" | xcpretty
+           set -o pipefail && xcodebuild -project ID3TagEditor.xcodeproj -scheme "ID3TagEditor watchOS" clean build CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO -destination "platform=watchOS Simulator,name=Apple Watch Series 8 (45mm),OS=9.1" | xcpretty
       - name: Build watchOS Demo
         run: |
           set -o pipefail && xcodebuild -project Demo/Demo.xcodeproj -scheme "Demo watchOS"  clean build CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NO | xcpretty
diff --git a/ID3TagEditor.xcodeproj/project.pbxproj b/ID3TagEditor.xcodeproj/project.pbxproj
index 68d05ada..efaac85b 100644
--- a/ID3TagEditor.xcodeproj/project.pbxproj
+++ b/ID3TagEditor.xcodeproj/project.pbxproj
@@ -720,6 +720,10 @@
 		5FAF8DAF21CED92F0049798C /* MockSynchsafeEncoder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5FAF8DAE21CED92F0049798C /* MockSynchsafeEncoder.swift */; };
 		5FAF8DB021CED92F0049798C /* MockSynchsafeEncoder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5FAF8DAE21CED92F0049798C /* MockSynchsafeEncoder.swift */; };
 		5FAF8DB121CED92F0049798C /* MockSynchsafeEncoder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5FAF8DAE21CED92F0049798C /* MockSynchsafeEncoder.swift */; };
+		6AE2D8AF296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6AE2D8AE296CBB3500CEE5DF /* Mp3FileReaderFactory.swift */; };
+		6AE2D8B0296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6AE2D8AE296CBB3500CEE5DF /* Mp3FileReaderFactory.swift */; };
+		6AE2D8B1296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6AE2D8AE296CBB3500CEE5DF /* Mp3FileReaderFactory.swift */; };
+		6AE2D8B2296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6AE2D8AE296CBB3500CEE5DF /* Mp3FileReaderFactory.swift */; };
 		C506714C03EAB37AC337B983 /* MockFrameFromStringContentCreator.swift in Sources */ = {isa = PBXBuildFile; fileRef = C506715FBCEE562BFB182459 /* MockFrameFromStringContentCreator.swift */; };
 		C50671C64692909C1AD1AC5D /* MockID3FrameCreatorsChain.swift in Sources */ = {isa = PBXBuildFile; fileRef = C506701F77DD635B85857590 /* MockID3FrameCreatorsChain.swift */; };
 		C50671DE6CD2C6BD107AB811 /* ID3TagProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = C50673F2ECE134CC71C9E573 /* ID3TagProperties.swift */; };
@@ -1017,6 +1021,7 @@
 		5F6A0D2B251E3F2200989D7E /* PathLoaderXcodeProj.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PathLoaderXcodeProj.swift; sourceTree = "<group>"; };
 		5FAF8DA621CED8600049798C /* example-v4-additional-data.mp3 */ = {isa = PBXFileReference; lastKnownFileType = audio.mp3; path = "example-v4-additional-data.mp3"; sourceTree = "<group>"; };
 		5FAF8DAE21CED92F0049798C /* MockSynchsafeEncoder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockSynchsafeEncoder.swift; sourceTree = "<group>"; };
+		6AE2D8AE296CBB3500CEE5DF /* Mp3FileReaderFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Mp3FileReaderFactory.swift; sourceTree = "<group>"; };
 		C506700658BCCA8D6CD0E270 /* MockFrameContentSizeCalculator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockFrameContentSizeCalculator.swift; sourceTree = "<group>"; };
 		C506701F77DD635B85857590 /* MockID3FrameCreatorsChain.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockID3FrameCreatorsChain.swift; sourceTree = "<group>"; };
 		C5067081F30C2572649940F5 /* ID3FrameConfigurationTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ID3FrameConfigurationTest.swift; sourceTree = "<group>"; };
@@ -1237,6 +1242,7 @@
 				452832272044C7BE00458375 /* Mp3WithID3TagBuilder.swift */,
 				450967152100BDC700A9CAA6 /* Mp3FileReader.swift */,
 				4509671F2100C3E600A9CAA6 /* Mp3FileWriter.swift */,
+				6AE2D8AE296CBB3500CEE5DF /* Mp3FileReaderFactory.swift */,
 			);
 			path = Mp3;
 			sourceTree = "<group>";
@@ -1977,6 +1983,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				457A8E1D255F1F8E00C708F4 /* FrameFlagsCreator.swift in Sources */,
+				6AE2D8AF296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */,
 				4528327A2044C7BE00458375 /* ID3TagConfiguration.swift in Sources */,
 				457A8E19255F1F8E00C708F4 /* ID3LocalizedFrameCreator.swift in Sources */,
 				457A8D60255F1F1C00C708F4 /* PaddingAdder.swift in Sources */,
@@ -2174,6 +2181,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				457A8E1E255F1F8E00C708F4 /* FrameFlagsCreator.swift in Sources */,
+				6AE2D8B0296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */,
 				4528327B2044C7BE00458375 /* ID3TagConfiguration.swift in Sources */,
 				457A8E1A255F1F8E00C708F4 /* ID3LocalizedFrameCreator.swift in Sources */,
 				457A8D61255F1F1C00C708F4 /* PaddingAdder.swift in Sources */,
@@ -2371,6 +2379,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				457A8E1F255F1F8E00C708F4 /* FrameFlagsCreator.swift in Sources */,
+				6AE2D8B1296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */,
 				45541B8020598FA60025A8BF /* ID3TagEditor.swift in Sources */,
 				457A8E1B255F1F8E00C708F4 /* ID3LocalizedFrameCreator.swift in Sources */,
 				457A8D62255F1F1C00C708F4 /* PaddingAdder.swift in Sources */,
@@ -2568,6 +2577,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				457A8E20255F1F8E00C708F4 /* FrameFlagsCreator.swift in Sources */,
+				6AE2D8B2296CBB3500CEE5DF /* Mp3FileReaderFactory.swift in Sources */,
 				45F833EB205905EF0046C804 /* ID3TagEditor.swift in Sources */,
 				457A8E1C255F1F8E00C708F4 /* ID3LocalizedFrameCreator.swift in Sources */,
 				457A8D63255F1F1C00C708F4 /* PaddingAdder.swift in Sources */,
diff --git a/Source/ID3TagEditor.swift b/Source/ID3TagEditor.swift
index c8d950de..a101f90b 100644
--- a/Source/ID3TagEditor.swift
+++ b/Source/ID3TagEditor.swift
@@ -21,7 +21,7 @@ public class ID3TagEditor {
      */
     public init() {
         self.id3TagParser = ID3TagParserFactory.make()
-        self.mp3FileReader = Mp3FileReader()
+        self.mp3FileReader = Mp3FileReaderFactory.make()
         self.mp3FileWriter = Mp3FileWriter()
         self.mp3WithID3TagBuilder = Mp3WithID3TagBuilder(id3TagCreator: ID3TagCreatorFactory.make(),
                                                          id3TagConfiguration: ID3TagConfiguration())
@@ -38,7 +38,7 @@ public class ID3TagEditor {
      Could throw `CorruptedFile` if the file is corrupted.
      */
     public func read(from path: String) throws -> ID3Tag? {
-        let mp3 = try mp3FileReader.readFrom(path: path)
+        let mp3 = try mp3FileReader.readID3TagFrom(path: path)
         return try self.id3TagParser.parse(mp3: mp3)
     }
 
@@ -68,7 +68,7 @@ public class ID3TagEditor {
      ID3 tag).
      */
     public func write(tag: ID3Tag, to path: String, andSaveTo newPath: String? = nil) throws {
-        let mp3 = try mp3FileReader.readFrom(path: path)
+        let mp3 = try mp3FileReader.readFileFrom(path: path)
         let currentTag = try self.id3TagParser.parse(mp3: mp3)
         let mp3WithId3Tag = try mp3WithID3TagBuilder.build(mp3: mp3, newId3Tag: tag, currentId3Tag: currentTag)
         try mp3FileWriter.write(mp3: mp3WithId3Tag, path: newPath ?? path)
diff --git a/Source/Mp3/Mp3FileReader.swift b/Source/Mp3/Mp3FileReader.swift
index 7a6f7daa..6c376f0b 100644
--- a/Source/Mp3/Mp3FileReader.swift
+++ b/Source/Mp3/Mp3FileReader.swift
@@ -8,12 +8,73 @@
 import Foundation
 
 class Mp3FileReader {
-    func readFrom(path: String) throws -> Data {
+    private let tagSizeParser: TagSizeParser
+    private let id3TagConfiguration: ID3TagConfiguration
+
+    init(tagSizeParser: TagSizeParser,
+         id3TagConfiguration: ID3TagConfiguration) {
+        self.tagSizeParser = tagSizeParser
+        self.id3TagConfiguration = id3TagConfiguration
+    }
+
+    /**
+      Read the entire mp3 file at path
+
+      - parameter path: the path to the mp3 file
+
+      - returns: mp3 data of the file
+
+      - throws: Could throw `InvalidFileFormat` if an mp3 file doesn't exists at the specified path.
+     */
+    func readFileFrom(path: String) throws -> Data {
         let validPath = URL(fileURLWithPath: path)
         guard validPath.pathExtension.caseInsensitiveCompare("mp3") == ComparisonResult.orderedSame else {
             throw ID3TagEditorError.invalidFileFormat
         }
+
         let mp3 = try Data(contentsOf: validPath)
         return mp3
     }
+
+    /**
+      Read only the ID3 header of the mp3 file at path
+
+      - parameter path: the path to the mp3 file
+
+      - returns: ID3 header data of 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
+     */
+    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
+        }
+
+        inputStream.open()
+
+        let headerSize = id3TagConfiguration.headerSize()
+        let header = try read(bytesCount: headerSize, fromStream: inputStream)
+        let headerData = Data(header) as NSData
+
+        let frameSize = tagSizeParser.parse(data: headerData)
+        let frame = try read(bytesCount: Int(frameSize), fromStream: inputStream)
+
+        let mp3 = header + frame
+        return Data(mp3)
+    }
+
+    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 {
+            throw ID3TagEditorError.corruptedFile
+        }
+        return buffer
+    }
 }
diff --git a/Source/Mp3/Mp3FileReaderFactory.swift b/Source/Mp3/Mp3FileReaderFactory.swift
new file mode 100644
index 00000000..d1a322c9
--- /dev/null
+++ b/Source/Mp3/Mp3FileReaderFactory.swift
@@ -0,0 +1,20 @@
+//
+//  Mp3FileReaderFactory.swift
+//  ID3TagEditor
+//
+//  Created by Zsolt Kovacs on 09.01.23.
+//  Copyright © 2023 Fabrizio Duroni. All rights reserved.
+//
+
+import Foundation
+
+class Mp3FileReaderFactory {
+    static func make() -> Mp3FileReader {
+        let tagSizeParser = ID3TagSizeParser()
+        let id3TagConfiguration = ID3TagConfiguration()
+        let fileReader = Mp3FileReader(tagSizeParser: tagSizeParser,
+                                       id3TagConfiguration: id3TagConfiguration)
+
+        return fileReader
+    }
+}
diff --git a/Source/Parse/ID3TagParser.swift b/Source/Parse/ID3TagParser.swift
index c41b7941..d2dfc355 100644
--- a/Source/Parse/ID3TagParser.swift
+++ b/Source/Parse/ID3TagParser.swift
@@ -8,10 +8,10 @@
 import Foundation
 
 class ID3TagParser {
+    private let tagSizeParser: TagSizeParser
+    private let id3TagConfiguration: ID3TagConfiguration
     private let tagVersionParser: TagVersionParser
     private let tagPresence: TagPresence
-    private let tagSizeParser: TagSizeParser
-    private var id3TagConfiguration: ID3TagConfiguration
     private let framesParser: ID3FramesParser
 
     init(tagVersionParser: TagVersionParser,


Test patch:
diff --git a/Tests/Mp3/Mp3FileReaderTest.swift b/Tests/Mp3/Mp3FileReaderTest.swift
index 7295711b..90253b78 100644
--- a/Tests/Mp3/Mp3FileReaderTest.swift
+++ b/Tests/Mp3/Mp3FileReaderTest.swift
@@ -9,22 +9,63 @@ import XCTest
 @testable import ID3TagEditor
 
 class Mp3FileReaderTest: XCTestCase {
-    func testNotAnMp3file() {
+    func testNotAnMP3FileWhenReadingEntireFile() {
         let path = PathLoader().pathFor(name: "example-cover", fileType: "jpg")
-        let mp3FileReader = Mp3FileReader()
+        let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(),
+                                          id3TagConfiguration: ID3TagConfiguration())
 
-        XCTAssertThrowsError(try mp3FileReader.readFrom(path: path))
+        XCTAssertThrowsError(try mp3FileReader.readFileFrom(path: path))
     }
 
-    func testMp3File() {
+    func testMP3FileWhenReadingEntireFile() {
         let path = PathLoader().pathFor(name: "example", fileType: "mp3")
-        let mp3FileReader = Mp3FileReader()
+        let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(),
+                                          id3TagConfiguration: ID3TagConfiguration())
 
-        XCTAssertNoThrow(try mp3FileReader.readFrom(path: path))
+        XCTAssertNoThrow(try mp3FileReader.readFileFrom(path: path))
+    }
+
+    func testNotAnMP3fileWhenReadingID3Tag() {
+        let path = PathLoader().pathFor(name: "example-cover", fileType: "jpg")
+        let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(),
+                                          id3TagConfiguration: ID3TagConfiguration())
+
+        XCTAssertThrowsError(try mp3FileReader.readID3TagFrom(path: path))
+    }
+
+    func testMP3fileWhenReadingID3Tag() {
+        let path = PathLoader().pathFor(name: "example", fileType: "mp3")
+        let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(),
+                                          id3TagConfiguration: ID3TagConfiguration())
+
+        XCTAssertNoThrow(try mp3FileReader.readID3TagFrom(path: path))
+    }
+
+    func testNonExistentMP3fileWhenReadingID3Tag() {
+        let path = "/non-existent.mp3"
+        let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(),
+                                          id3TagConfiguration: ID3TagConfiguration())
+
+        XCTAssertThrowsError(try mp3FileReader.readID3TagFrom(path: path))
+    }
+
+    func testOnlyReadsID3Tag() throws {
+        let path = PathLoader().pathFor(name: "example", fileType: "mp3")
+        let mp3FileReader = Mp3FileReader(tagSizeParser: ID3TagSizeParser(),
+                                          id3TagConfiguration: ID3TagConfiguration())
+
+        let id3TagData = try mp3FileReader.readID3TagFrom(path: path)
+
+        // 10 bytes Tag + 34213 bytes according to the Tag Size in the file's ID3 Tag
+        XCTAssertEqual(id3TagData.count, 10 + 34213)
     }
 
     static let allTests = [
-        ("testNotAnMp3file", testNotAnMp3file),
-        ("testMp3File", testMp3File)
+        ("testNotAnMP3FileWhenReadingEntireFile", testNotAnMP3FileWhenReadingEntireFile),
+        ("testMP3FileWhenReadingEntireFile", testMP3FileWhenReadingEntireFile),
+        ("testNotAnMP3fileWhenReadingID3Tag", testNotAnMP3fileWhenReadingID3Tag),
+        ("testMP3fileWhenReadingID3Tag", testMP3fileWhenReadingID3Tag),
+        ("testNonExistentMP3fileWhenReadingID3Tag", testNonExistentMP3fileWhenReadingID3Tag),
+        ("testOnlyReadsID3Tag", testOnlyReadsID3Tag)
     ]
 }