From d372e8af6eb57a090cc3d542e3f3265aa1a3c68c Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 22 May 2026 14:40:01 -0500 Subject: [PATCH 1/7] :robot: Apply refactoring for text information frame --- src/id3v2/frames/textInformationFrame.ts | 125 ++++++++---------- test-unit/id3v2/textInformationFrameTests.ts | 38 +++++- .../id3v2/userTextInformationFrameTests.ts | 51 ++++++- 3 files changed, 139 insertions(+), 75 deletions(-) diff --git a/src/id3v2/frames/textInformationFrame.ts b/src/id3v2/frames/textInformationFrame.ts index abeee0f..ed2c544 100644 --- a/src/id3v2/frames/textInformationFrame.ts +++ b/src/id3v2/frames/textInformationFrame.ts @@ -148,16 +148,6 @@ export class TextInformationFrame extends Frame { * @protected */ protected _encoding: StringType = Id3v2Settings.defaultEncoding; - /** - * Raw data contents in the current instance. - * @protected - */ - protected _rawData: ByteVector; - /** - * ID3v2 version of the current instance. - * @protected - */ - protected _rawVersion: number; /** * Decoded text contained in the current instance. * @protected @@ -222,14 +212,12 @@ export class TextInformationFrame extends Frame { * current instance. The value must be reassigned for the value to change. */ public get text(): string[] { - this.parseRawData(); return this._textFields.slice(); } /** * Sets the text contained in the current instance. */ public set text(value: string[]) { - this.parseRawData(); this._textFields = value ? value.slice() : []; } @@ -237,7 +225,6 @@ export class TextInformationFrame extends Frame { * Gets the text encoding to use when rendering the current instance. */ public get textEncoding(): StringType { - this.parseRawData(); return this._encoding; } /** @@ -245,7 +232,6 @@ export class TextInformationFrame extends Frame { * This value will be overridden if {@link Id3v2Settings.forceDefaultEncoding} is `true`. */ public set textEncoding(value: StringType) { - this.parseRawData(); this._encoding = value; } @@ -274,10 +260,6 @@ export class TextInformationFrame extends Frame { public clone(): Frame { const frame = TextInformationFrame.fromIdentifier(this.frameId, this._encoding); frame._textFields = this._textFields.slice(); - if (this._rawData) { - frame._rawData = this._rawData.toByteVector(); - } - frame._rawVersion = this._rawVersion; return frame; } @@ -323,7 +305,6 @@ export class TextInformationFrame extends Frame { * Returns a text representation of the current instance by combining the text with semicolons. */ public toString(): string { - this.parseRawData(); return this.text.join("; "); } @@ -333,32 +314,18 @@ export class TextInformationFrame extends Frame { /** @inheritDoc */ protected parseFields(data: ByteVector, version: number): void { - this._rawData = data; - this._rawVersion = version; - } - - /** - * Performs the actual parsing of the raw data. - * Because of the high parsing cost and relatively low usage of the class {@link parseFields} - * only stores the field data so it can be parsed on demand. Whenever a property or method is - * called which requires the data, this method is called, and only on the first call does it - * actually parse the data. - */ - protected parseRawData(): void { - if (!this._rawData) { + if (data.length === 0) { + this._textFields = []; return; } - const data = this._rawData; - this._rawData = undefined; - // Read the string data type (first byte of the field data) this._encoding = data.get(0); const fieldList = []; const delim = ByteVector.getTextDelimiter(this._encoding); - if (this._rawVersion > 3 || this.frameId === FrameIdentifiers.TXXX) { + if (version > 3 || this.frameId === FrameIdentifiers.TXXX) { fieldList.push(...data.subarray(1).toStrings(this._encoding)); } else if (data.length > 1 && !data.containsAt(delim, 1)) { let value = data.subarray(1).toString(this._encoding); @@ -377,23 +344,12 @@ export class TextInformationFrame extends Frame { } } - // Bad tags may have one or more null characters at the end of a string, resulting in - // empty strings at the end of the FieldList. Strip them off. - while (fieldList.length !== 0 - && (!fieldList[fieldList.length - 1] || fieldList[fieldList.length - 1].length === 0)) { - fieldList.splice(fieldList.length - 1, 1); - } - this._textFields = fieldList; } /** @inheritDoc */ protected renderFields(version: number): ByteVector { - if (this._rawData && this._rawVersion === version) { - return this._rawData; - } - - const encoding = TextInformationFrame.correctEncoding(this.textEncoding, version); + const encoding = TextInformationFrame.correctEncoding(this._encoding, version); const v = ByteVector.empty(); let text = this._textFields; @@ -433,6 +389,8 @@ export class TextInformationFrame extends Frame { } export class UserTextInformationFrame extends TextInformationFrame { + private _description: string; + // #region Constructors private constructor(header: Id3v2FrameHeader) { @@ -450,7 +408,7 @@ export class UserTextInformationFrame extends TextInformationFrame { ): UserTextInformationFrame { const frame = new UserTextInformationFrame(new Id3v2FrameHeader(FrameIdentifiers.TXXX)); frame._encoding = encoding; - frame.description = description; + frame._description = description; return frame; } @@ -489,8 +447,7 @@ export class UserTextInformationFrame extends TextInformationFrame { * Gets the description stored in the current instance. */ public get description(): string { - const text = super.text; - return text.length > 0 ? text[0] : undefined; + return this._description; } /** * Sets the description stored in the current instance. @@ -498,13 +455,7 @@ export class UserTextInformationFrame extends TextInformationFrame { * @param value Description to store in the current instance. */ public set description(value: string) { - let text = super.text; - if (text.length > 0) { - text[0] = value; - } else { - text = [ value ]; - } - super.text = text; + this._description = value; } /** @@ -513,21 +464,14 @@ export class UserTextInformationFrame extends TextInformationFrame { * current instance. The value must be reassigned for the value to change. */ public get text(): string[] { - const text = super.text; - if (text.length < 2) { - return []; - } - - return text.slice(1); + return this._textFields.slice(); } /** * Sets the text contained in the current instance. * @param value Array of text values to store in the current instance */ public set text(value: string[]) { - const newValue = [this.description]; - newValue.push(... value); - super.text = newValue; + this._textFields = value ? value.slice() : []; } // #endregion @@ -554,12 +498,8 @@ export class UserTextInformationFrame extends TextInformationFrame { /** @inheritDoc */ public clone(): Frame { - const frame = UserTextInformationFrame.fromDescription(undefined, this._encoding); + const frame = UserTextInformationFrame.fromDescription(this._description, this._encoding); frame._textFields = this._textFields.slice(); - if (this._rawData) { - frame._rawData = this._rawData.toByteVector(); - } - frame._rawVersion = this._rawVersion; return frame; } @@ -568,5 +508,46 @@ export class UserTextInformationFrame extends TextInformationFrame { return `[${this.description}] ${super.toString()}`; } + /** @inheritDoc */ + protected parseFields(data: ByteVector, version: number): void { + Guards.byte(version, "version"); + + if (data.length === 0) { + this._description = undefined; + this._textFields = []; + return; + } + + this._encoding = data.get(0); + const fields = data.subarray(1).toStrings(this._encoding); + this._description = fields.length > 0 ? fields[0] : undefined; + this._textFields = fields.slice(1); + } + + /** @inheritDoc */ + protected renderFields(version: number): ByteVector { + if (!this._description && this._textFields.length === 0) { + return ByteVector.empty(); + } + + const encoding = TextInformationFrame.correctEncoding(this._encoding, version); + const v = ByteVector.empty(); + v.addByte(encoding); + v.addByteVector(ByteVector.fromString(this._description ?? "", encoding)); + + for (const text of this._textFields) { + v.addByteVector(ByteVector.getTextDelimiter(encoding)); + if (text) { + v.addByteVector(ByteVector.fromString(text, encoding)); + } + } + + if (this._textFields.length === 0) { + v.addByteVector(ByteVector.getTextDelimiter(encoding)); + } + + return v; + } + // #endregion } diff --git a/test-unit/id3v2/textInformationFrameTests.ts b/test-unit/id3v2/textInformationFrameTests.ts index ea2d6c9..62ebe8e 100644 --- a/test-unit/id3v2/textInformationFrameTests.ts +++ b/test-unit/id3v2/textInformationFrameTests.ts @@ -310,7 +310,7 @@ const getTestFrame = (): TextInformationFrame => { } @test - public render_notRead_returnsRawData() { + public render_eagerLoaded_returnsRenderedFields() { // Arrange const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); header.frameSize = 8; @@ -331,7 +331,41 @@ const getTestFrame = (): TextInformationFrame => { Testers.bvEqual(output, data); } - // @TODO: Test for render version not same as input w/o read forces a read + @test + public render_eagerLoaded_preservesTrailingEmptyValues() { + // Arrange + let header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); + header.frameSize = 10; + const data = ByteVector.concatenate( + header.render(4), + StringType.Latin1, + ByteVector.fromString("fux", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("bux", StringType.Latin1), + 0x0, + 0x0 + ); + const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 4); + + // Act + const output = frame.render(4); + + // Assert + assert.ok(output); + + header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); + header.frameSize = 9; + const expected = ByteVector.concatenate( + header.render(4), + StringType.Latin1, + ByteVector.fromString("fux", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("bux", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1) + ); + + Testers.bvEqual(output, expected); + } @test public render_readV4NotTxxx() { diff --git a/test-unit/id3v2/userTextInformationFrameTests.ts b/test-unit/id3v2/userTextInformationFrameTests.ts index a1306fe..89ceae1 100644 --- a/test-unit/id3v2/userTextInformationFrameTests.ts +++ b/test-unit/id3v2/userTextInformationFrameTests.ts @@ -6,7 +6,7 @@ import Id3v2Settings from "../../src/id3v2/id3v2Settings"; import {UserTextInformationFrame} from "../../src/id3v2/frames/textInformationFrame"; import {ByteVector, StringType} from "../../src/byteVector"; import {Frame, FrameClassType} from "../../src/id3v2/frames/frame"; -import {Id3v2FrameHeader} from "../../src/id3v2/frames/frameHeader"; +import {Id3v2FrameFlags, Id3v2FrameHeader} from "../../src/id3v2/frames/frameHeader"; import {FrameIdentifiers} from "../../src/id3v2/frameIdentifiers"; import {Testers} from "../utilities/testers"; @@ -215,4 +215,53 @@ const getTestFrame = (): UserTextInformationFrame => { assert.deepStrictEqual(frame.text, output.text); assert.strictEqual(frame.textEncoding, output.textEncoding); } + + @test + public render_usesDescriptionAndTextFields() { + // Arrange + const frame = UserTextInformationFrame.fromDescription("foo", StringType.Latin1); + frame.text = ["bar", "baz"]; + + // Act + const result = frame.render(4); + + // Assert + assert.isOk(result); + + const expected = ByteVector.concatenate( + FrameIdentifiers.TXXX.render(4), + ByteVector.fromUint(12), + ByteVector.fromUshort(Id3v2FrameFlags.None), + StringType.Latin1, + ByteVector.fromString("foo", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("bar", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("baz", StringType.Latin1) + ); + Testers.bvEqual(result, expected); + } + + @test + public render_withoutDescriptionWithText() { + // Arrange + const frame = UserTextInformationFrame.fromDescription(undefined, StringType.Latin1); + frame.text = ["foo"]; + + // Act + const result = frame.render(4); + + // Assert + assert.isOk(result); + + const expected = ByteVector.concatenate( + FrameIdentifiers.TXXX.render(4), + ByteVector.fromUint(5), + ByteVector.fromUshort(Id3v2FrameFlags.None), + StringType.Latin1, + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("foo", StringType.Latin1) + ); + Testers.bvEqual(result, expected); + } } From 5f56f73607ea43f63f9751bb43b350877317ae5a Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 22 May 2026 22:04:25 -0500 Subject: [PATCH 2/7] :robot: Tests! --- src/id3v2/frames/textInformationFrame.ts | 1 + test-unit/id3v2/textInformationFrameTests.ts | 112 +++++++++++- .../id3v2/userTextInformationFrameTests.ts | 169 +++++++++++++++++- 3 files changed, 277 insertions(+), 5 deletions(-) diff --git a/src/id3v2/frames/textInformationFrame.ts b/src/id3v2/frames/textInformationFrame.ts index ed2c544..9125ee5 100644 --- a/src/id3v2/frames/textInformationFrame.ts +++ b/src/id3v2/frames/textInformationFrame.ts @@ -491,6 +491,7 @@ export class UserTextInformationFrame extends TextInformationFrame { caseSensitive: boolean = true ): UserTextInformationFrame { Guards.truthy(frames, "frames"); + Guards.truthy(description, "description"); const comparison = caseSensitive ? StringComparison.caseSensitive : StringComparison.caseInsensitive; return frames.find((f) => comparison(f.description, description)); diff --git a/test-unit/id3v2/textInformationFrameTests.ts b/test-unit/id3v2/textInformationFrameTests.ts index 62ebe8e..ee17f37 100644 --- a/test-unit/id3v2/textInformationFrameTests.ts +++ b/test-unit/id3v2/textInformationFrameTests.ts @@ -1,6 +1,4 @@ -// noinspection JSUnusedLocalSymbols Used extensibely to force a read - -import {suite, test} from "@testdeck/mocha"; +import {params, suite, test} from "@testdeck/mocha"; import {assert} from "chai"; import FrameConstructorTests from "./frameConstructorTests"; @@ -9,7 +7,7 @@ import PropertyTests from "../utilities/propertyTests"; import {TextInformationFrame} from "../../src/id3v2/frames/textInformationFrame"; import {ByteVector, StringType} from "../../src/byteVector"; import {Frame, FrameClassType} from "../../src/id3v2/frames/frame"; -import {Id3v2FrameHeader} from "../../src/id3v2/frames/frameHeader"; +import {Id3v2FrameFlags, Id3v2FrameHeader} from "../../src/id3v2/frames/frameHeader"; import {FrameIdentifier, FrameIdentifiers} from "../../src/id3v2/frameIdentifiers"; import {Testers} from "../utilities/testers"; @@ -48,6 +46,12 @@ const getTestFrame = (): TextInformationFrame => { assert.strictEqual(frame.textEncoding, Id3v2Settings.defaultEncoding); } + @test + public fromIdentifier_falsyIdentifier() { + // Act/Assert + Testers.testTruthy((v: FrameIdentifier) => { TextInformationFrame.fromIdentifier(v); }); + } + @test public fromIdentifier_withEncoding_returnsFrameWithProvidedEncoding() { // Act @@ -207,6 +211,21 @@ const getTestFrame = (): TextInformationFrame => { PropertyTests.propertyRoundTrip((v) => { frame.text = v; }, () => frame.text, ["bux", "fux"]); } + @params(undefined, "undefined") + @params(null, "null") + @params([], "empty_array") + @params(["bar"], "truthy") + public setText_values(value: string[]) { + // Arrange + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOP); + + // Act + frame.text = value; + + // Assert + assert.deepStrictEqual(frame.text, value ?? []); + } + @test public setEncoding_notRead() { // Arrange @@ -267,6 +286,18 @@ const getTestFrame = (): TextInformationFrame => { Testers.testTruthy((v: FrameIdentifier) => { TextInformationFrame.findTextInformationFrame([], v); }); } + @test + public find_emptyFrames_returnsUndefined() { + // Arrange + const frames: TextInformationFrame[] = []; + + // Act + const output = TextInformationFrame.findTextInformationFrame(frames, FrameIdentifiers.TCOM); + + // Assert + assert.isUndefined(output); + } + @test public find_frameExists() { // Arrange @@ -283,6 +314,20 @@ const getTestFrame = (): TextInformationFrame => { assert.equal(output, frames[1]); } + @test + public find_frameExists_returnsFirstMatch() { + // Arrange + const frame1 = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOM); + const frame2 = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOM); + const frames = [frame1, frame2]; + + // Act + const output = TextInformationFrame.findTextInformationFrame(frames, FrameIdentifiers.TCOM); + + // Assert + assert.strictEqual(output, frame1); + } + @test public find_frameDoesNotExist() { // Arrange @@ -309,6 +354,50 @@ const getTestFrame = (): TextInformationFrame => { assert.throws(() => { frame.render(0x100); }); } + @test + public render_withoutText() { + // Arrange + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOP, StringType.Latin1); + + // Act + const result = frame.render(4); + + // Assert + assert.isOk(result); + + const expected = ByteVector.concatenate( + FrameIdentifiers.TCOP.render(4), + ByteVector.fromUint(1), + ByteVector.fromUshort(Id3v2FrameFlags.None), + StringType.Latin1 + ); + Testers.bvEqual(result, expected); + } + + @test + public render_withText() { + // Arrange + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOP, StringType.Latin1); + frame.text = ["foo", "bar"]; + + // Act + const result = frame.render(4); + + // Assert + assert.isOk(result); + + const expected = ByteVector.concatenate( + FrameIdentifiers.TCOP.render(4), + ByteVector.fromUint(8), + ByteVector.fromUshort(Id3v2FrameFlags.None), + StringType.Latin1, + ByteVector.fromString("foo", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("bar", StringType.Latin1) + ); + Testers.bvEqual(result, expected); + } + @test public render_eagerLoaded_returnsRenderedFields() { // Arrange @@ -491,4 +580,19 @@ const getTestFrame = (): TextInformationFrame => { assert.ok(output); Testers.bvEqual(output, data); } + + @params([[], ""], "empty") + @params([["foo"], "foo"], "single") + @params([["foo", "bar"], "foo; bar"], "multiple") + public toString_returnsText([text, expected]: [string[], string]) { + // Arrange + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOP); + frame.text = text; + + // Act + const result = frame.toString(); + + // Assert + assert.strictEqual(result, expected); + } } diff --git a/test-unit/id3v2/userTextInformationFrameTests.ts b/test-unit/id3v2/userTextInformationFrameTests.ts index 89ceae1..b4e9b3d 100644 --- a/test-unit/id3v2/userTextInformationFrameTests.ts +++ b/test-unit/id3v2/userTextInformationFrameTests.ts @@ -1,4 +1,4 @@ -import {suite, test} from "@testdeck/mocha"; +import {params, suite, test} from "@testdeck/mocha"; import {assert} from "chai"; import FrameConstructorTests from "./frameConstructorTests"; @@ -47,6 +47,18 @@ const getTestFrame = (): UserTextInformationFrame => { Id3v2_UserInformationFrame_ConstructorTests.assertFrame(frame, "foo", [], StringType.UTF16); } + @params([undefined, StringType.Latin1], "undefined") + @params([null, StringType.Latin1], "null") + @params(["", StringType.Latin1], "empty_string") + @params(["foo", StringType.Latin1], "truthy") + public fromDescription_descriptionValues([description, encoding]: [string, StringType]) { + // Act + const frame = UserTextInformationFrame.fromDescription(description, encoding); + + // Assert + Id3v2_UserInformationFrame_ConstructorTests.assertFrame(frame, description, [], encoding); + } + @test public fromOffsetRawData_returnsFrame() { // Assert @@ -99,6 +111,23 @@ const getTestFrame = (): UserTextInformationFrame => { assert.deepStrictEqual(frame.text, ["bar"]); } + @params(undefined, "undefined") + @params(null, "null") + @params("", "empty_string") + @params("fux", "truthy") + public setDescription_values(value: string) { + // Arrange + const frame = UserTextInformationFrame.fromDescription("foo"); + frame.text = ["bar"]; + + // Act + frame.description = value; + + // Assert + assert.strictEqual(frame.description, value); + assert.deepStrictEqual(frame.text, ["bar"]); + } + @test public getText() { // Arrange @@ -125,6 +154,34 @@ const getTestFrame = (): UserTextInformationFrame => { assert.strictEqual(frame.description, "foo"); assert.deepStrictEqual(frame.text, ["bux", "qux"]); } + + @params(undefined, "undefined") + @params(null, "null") + @params([], "empty_array") + @params(["bux"], "truthy") + public setText_values(value: string[]) { + // Arrange + const frame = UserTextInformationFrame.fromDescription("foo"); + + // Act + frame.text = value; + + // Assert + assert.strictEqual(frame.description, "foo"); + assert.deepStrictEqual(frame.text, value ?? []); + } + + @test + public setEncoding() { + // Arrange + const frame = UserTextInformationFrame.fromDescription("foo"); + + // Act + frame.textEncoding = StringType.UTF8; + + // Assert + assert.strictEqual(frame.textEncoding, StringType.UTF8); + } } @suite class Id3v2_UserTextInformationFrame_MethodTests { @@ -136,6 +193,27 @@ const getTestFrame = (): UserTextInformationFrame => { }); } + @test + public findUserTextInformationFrame_falsyDescription() { + // Arrange + const frames = [UserTextInformationFrame.fromDescription("foo")]; + + // Act/Assert + Testers.testTruthy((v: string) => { UserTextInformationFrame.findUserTextInformationFrame(frames, v); }); + } + + @test + public findUserTextInformationFrame_emptyFrames_returnsUndefined() { + // Arrange + const frames: UserTextInformationFrame[] = []; + + // Act + const output = UserTextInformationFrame.findUserTextInformationFrame(frames, "foo"); + + // Assert + assert.isUndefined(output); + } + @test public findUserTextInformationFrame_frameWithCaseSensitiveDescriptionDoesNotExist() { // Arrange @@ -198,6 +276,20 @@ const getTestFrame = (): UserTextInformationFrame => { assert.strictEqual(output, frames[0]); } + @test + public findUserTextInformationFrame_match_returnsFirstMatch() { + // Arrange + const frame1 = UserTextInformationFrame.fromDescription("foo"); + const frame2 = UserTextInformationFrame.fromDescription("foo"); + const frames = [frame1, frame2]; + + // Act + const output = UserTextInformationFrame.findUserTextInformationFrame(frames, "foo"); + + // Assert + assert.strictEqual(output, frame1); + } + @test public clone_returnsCopy() { // Arrange @@ -242,6 +334,19 @@ const getTestFrame = (): UserTextInformationFrame => { Testers.bvEqual(result, expected); } + @test + public render_withoutDescriptionWithoutText() { + // Arrange + const frame = UserTextInformationFrame.fromDescription(undefined, StringType.Latin1); + + // Act + const result = frame.render(4); + + // Assert + assert.isOk(result); + assert.strictEqual(result.length, 0); + } + @test public render_withoutDescriptionWithText() { // Arrange @@ -264,4 +369,66 @@ const getTestFrame = (): UserTextInformationFrame => { ); Testers.bvEqual(result, expected); } + + @test + public render_withDescriptionWithoutText() { + // Arrange + const frame = UserTextInformationFrame.fromDescription("foo", StringType.Latin1); + + // Act + const result = frame.render(4); + + // Assert + assert.isOk(result); + + const expected = ByteVector.concatenate( + FrameIdentifiers.TXXX.render(4), + ByteVector.fromUint(5), + ByteVector.fromUshort(Id3v2FrameFlags.None), + StringType.Latin1, + ByteVector.fromString("foo", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1) + ); + Testers.bvEqual(result, expected); + } + + @test + public render_withDescriptionWithText_twoByteEncoding() { + // Arrange + const frame = UserTextInformationFrame.fromDescription("foo", StringType.UTF16LE); + frame.text = ["bar"]; + + // Act + const result = frame.render(4); + + // Assert + assert.isOk(result); + + const expected = ByteVector.concatenate( + FrameIdentifiers.TXXX.render(4), + ByteVector.fromUint(15), + ByteVector.fromUshort(Id3v2FrameFlags.None), + StringType.UTF16LE, + ByteVector.fromString("foo", StringType.UTF16LE), + ByteVector.getTextDelimiter(StringType.UTF16LE), + ByteVector.fromString("bar", StringType.UTF16LE) + ); + Testers.bvEqual(result, expected); + } + + @params(["", [] as string[]], "empty_string") + @params(["foo", [] as string[]], "foo_empty_text") + @params(["", ["foo"]], "empty_string_foo") + @params(["foo", ["bar"]], "foo_bar") + public toString_returnsText([description, text]: [string, string[]]) { + // Arrange + const frame = UserTextInformationFrame.fromDescription(description); + frame.text = text; + + // Act + const result = frame.toString(); + + // Assert + assert.strictEqual(result, `[${description}] ${text.join("; ")}`); + } } From 969e398278adee5722da9872be63130efa3e1845 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Fri, 22 May 2026 23:48:21 -0500 Subject: [PATCH 3/7] Rewrite the render method for the text information frame and rework the tests for it --- src/id3v2/frames/textInformationFrame.ts | 46 +-- test-unit/id3v2/textInformationFrameTests.ts | 292 ++++++------------- 2 files changed, 110 insertions(+), 228 deletions(-) diff --git a/src/id3v2/frames/textInformationFrame.ts b/src/id3v2/frames/textInformationFrame.ts index 9125ee5..8b80a70 100644 --- a/src/id3v2/frames/textInformationFrame.ts +++ b/src/id3v2/frames/textInformationFrame.ts @@ -276,6 +276,7 @@ export class TextInformationFrame extends Frame { return super.render(version); } + // @TODO: This code should be taken out when we migrate to immutable tag versions. const text = this.toString(); if (text.length < 10 || text[4] !== "-" || text[7] !== "-") { return super.render(version); @@ -325,7 +326,7 @@ export class TextInformationFrame extends Frame { const fieldList = []; const delim = ByteVector.getTextDelimiter(this._encoding); - if (version > 3 || this.frameId === FrameIdentifiers.TXXX) { + if (version > 3) { fieldList.push(...data.subarray(1).toStrings(this._encoding)); } else if (data.length > 1 && !data.containsAt(delim, 1)) { let value = data.subarray(1).toString(this._encoding); @@ -349,40 +350,25 @@ export class TextInformationFrame extends Frame { /** @inheritDoc */ protected renderFields(version: number): ByteVector { - const encoding = TextInformationFrame.correctEncoding(this._encoding, version); - const v = ByteVector.empty(); - let text = this._textFields; - - v.addByte(encoding); - - // Main processing - const isTxxx = this.frameId === FrameIdentifiers.TXXX; - if (version > 3 || isTxxx) { - if (isTxxx) { - if (text.length === 0) { - text = [null, null]; - } else if (text.length === 1) { - text = [text[0], null]; - } - } + const truthyFields = this._textFields.filter(tf => !!tf); + if (truthyFields.length === 0) { + return ByteVector.empty(); + } - for (let i = 0; i < text.length; i++) { - // Since the field list is null delimited, if this is not the first element in the - // list, append the appropriate delimiter for this encoding. - if (i !== 0) { - v.addByteVector(ByteVector.getTextDelimiter(encoding)); - } + const encoding = TextInformationFrame.correctEncoding(this._encoding, version); - if (text[i]) { - v.addByteVector(ByteVector.fromString(text[i], encoding)); - } - } + let fieldBytes; + if (version > 3) { + // v4 frames have each field separated by a delimiter + const truthyVectors = truthyFields.map(tf => ByteVector.fromString(tf, encoding)); + fieldBytes = ByteVector.join(ByteVector.getTextDelimiter(encoding), truthyVectors); } else { - // Fields that have slashes in them and fields that don't - v.addByteVector(ByteVector.fromString(text.join("/"), encoding)); + // v3, v2 frames have multiple fields separated by a / + const joinedFields = this._textFields.join("/"); + fieldBytes = ByteVector.fromString(joinedFields, encoding); } - return v; + return ByteVector.concatenate(encoding, fieldBytes); } // #endregion diff --git a/test-unit/id3v2/textInformationFrameTests.ts b/test-unit/id3v2/textInformationFrameTests.ts index ee17f37..5211412 100644 --- a/test-unit/id3v2/textInformationFrameTests.ts +++ b/test-unit/id3v2/textInformationFrameTests.ts @@ -62,86 +62,85 @@ const getTestFrame = (): TextInformationFrame => { } @test - public fromOffsetRawData_emptyFrameByLength_returnsEmptyFrame() { + public fromOffsetRawData_emptyFrame_returnsEmptyFrame() { // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 1; + const bodyBytes = ByteVector.empty(); + const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP, Id3v2FrameFlags.None, bodyBytes.length); const data = ByteVector.concatenate( 0x00, 0x00, header.render(3), - StringType.Latin1 + bodyBytes ); // Act const frame = TextInformationFrame.fromOffsetRawData(data, 2, header, 3); // Assert - Id3v2_TextInformationFrame_ConstructorTests.assertFrame(frame, FrameIdentifiers.TCOP, []); + assert.isOk(frame); + assert.strictEqual(frame.frameId, FrameIdentifiers.TCOP); + assert.deepEqual(frame.text, []); + assert.strictEqual(frame.textEncoding, Id3v2Settings.defaultEncoding); } @test - public fromOffsetRawData_emptyFrameByContents_returnsEmptyFrame() { + public fromOffsetRawData_v3NotSplitType_returnsSingleTextField() { // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 3; + const bodyBytes = ByteVector.concatenate( + StringType.Latin1, + ByteVector.fromString("fux/bux", StringType.Latin1) + ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TALB, Id3v2FrameFlags.None, bodyBytes.length); const data = ByteVector.concatenate( 0x00, 0x00, header.render(3), - StringType.Latin1, - 0x0, 0x0 + bodyBytes ); // Act const frame = TextInformationFrame.fromOffsetRawData(data, 2, header, 3); // Assert - Id3v2_TextInformationFrame_ConstructorTests.assertFrame(frame, FrameIdentifiers.TCOP, []); + Id3v2_TextInformationFrame_ConstructorTests.assertFrame(frame, FrameIdentifiers.TALB, ["fux/bux"]); } @test - public fromOffsetRawData_txxx_returnsFrameSplitByDelimiter() { + public fromOffsetRawData_v3SplitType_returnsFrameSplitBySlash() { // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TXXX); - header.frameSize = 17; + const bodyBytes = ByteVector.concatenate( + StringType.Latin1, + ByteVector.fromString("fux/bux", StringType.Latin1) + ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TCOM, Id3v2FrameFlags.None, bodyBytes.length); const data = ByteVector.concatenate( 0x00, 0x00, - header.render(4), - StringType.UTF16BE, - ByteVector.fromString("fux", StringType.UTF16BE), - ByteVector.getTextDelimiter(StringType.UTF16BE), - ByteVector.fromString("bux", StringType.UTF16BE), - 0x0, 0x0, // Extra nulls to trigger null stripping logic + header.render(3), + bodyBytes ); // Act - const frame = TextInformationFrame.fromOffsetRawData(data, 2, header, 4); + const frame = TextInformationFrame.fromOffsetRawData(data, 2, header, 3); // Assert - Id3v2_TextInformationFrame_ConstructorTests.assertFrame( - frame, - FrameIdentifiers.TXXX, - ["fux", "bux"], - StringType.UTF16BE - ); + Id3v2_TextInformationFrame_ConstructorTests.assertFrame(frame, FrameIdentifiers.TCOM, ["fux", "bux"]); } @test - public fromOffsetRawData_v3SplitType_returnsFrameSplitBySlash() { + public fromOffsetRawData_v3WithNullBytes_discardsDataAfterNull() { // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TCOM); - header.frameSize = 8; - const data = ByteVector.concatenate( - 0x00, 0x00, - header.render(3), + const bodyBytes = ByteVector.concatenate( StringType.Latin1, - ByteVector.fromString("fux/bux", StringType.Latin1) + ByteVector.fromString("foo", StringType.Latin1), + 0x00, + ByteVector.fromString("bar", StringType.Latin1) ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TCOM, Id3v2FrameFlags.None, bodyBytes.length); + const data = ByteVector.concatenate(header.render(3), bodyBytes); // Act - const frame = TextInformationFrame.fromOffsetRawData(data, 2, header, 3); + const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 3); // Assert - Id3v2_TextInformationFrame_ConstructorTests.assertFrame(frame, FrameIdentifiers.TCOM, ["fux", "bux"]); + Id3v2_TextInformationFrame_ConstructorTests.assertFrame(frame, FrameIdentifiers.TCOM, ["foo"]); } @test @@ -343,214 +342,113 @@ const getTestFrame = (): TextInformationFrame => { assert.isUndefined(output); } - @test - public render_invalidVersion_throws() { + @params(-1, "negative") + @params(1.23, "float") + @params(0x100, "too_big") + public render_invalidVersion_throws(version: number) { // Arrange const frame = getTestFrame(); // Act/Assert - assert.throws(() => { frame.render(-1); }); - assert.throws(() => { frame.render(1.23); }); - assert.throws(() => { frame.render(0x100); }); + assert.throws(() => frame.render(version)); } - @test - public render_withoutText() { + @params(2, "v2") + @params(3, "v3") + @params(4, "v4") + public render_empty_returnEmptyVector(version: number) { // Arrange - const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOP, StringType.Latin1); + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TALB); + frame.text = []; // Act - const result = frame.render(4); + const result = frame.render(version); // Assert assert.isOk(result); - - const expected = ByteVector.concatenate( - FrameIdentifiers.TCOP.render(4), - ByteVector.fromUint(1), - ByteVector.fromUshort(Id3v2FrameFlags.None), - StringType.Latin1 - ); - Testers.bvEqual(result, expected); + assert.strictEqual(result.length, 0); } - @test - public render_withText() { + @params(2, "v2") + @params(3, "v3") + @params(4, "v4") + public render_falsyValues_returnEmptyVector(version: number) { // Arrange - const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOP, StringType.Latin1); - frame.text = ["foo", "bar"]; + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TALB); + frame.text = [undefined, null, ""]; // Act - const result = frame.render(4); + const result = frame.render(version); // Assert assert.isOk(result); - - const expected = ByteVector.concatenate( - FrameIdentifiers.TCOP.render(4), - ByteVector.fromUint(8), - ByteVector.fromUshort(Id3v2FrameFlags.None), - StringType.Latin1, - ByteVector.fromString("foo", StringType.Latin1), - ByteVector.getTextDelimiter(StringType.Latin1), - ByteVector.fromString("bar", StringType.Latin1) - ); - Testers.bvEqual(result, expected); - } - - @test - public render_eagerLoaded_returnsRenderedFields() { - // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 8; - const data = ByteVector.concatenate( - header.render(4), - Id3v2Settings.defaultEncoding, - ByteVector.fromString("fux", StringType.Latin1), - ByteVector.getTextDelimiter(StringType.Latin1), - ByteVector.fromString("bux", StringType.Latin1), - ); - const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 4); - - // Act - const output = frame.render(4); - - // Assert - assert.ok(output); - Testers.bvEqual(output, data); + assert.strictEqual(result.length, 0); } @test - public render_eagerLoaded_preservesTrailingEmptyValues() { + public render_v4MultiByteEncoding() { // Arrange - let header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 10; - const data = ByteVector.concatenate( - header.render(4), - StringType.Latin1, - ByteVector.fromString("fux", StringType.Latin1), - ByteVector.getTextDelimiter(StringType.Latin1), - ByteVector.fromString("bux", StringType.Latin1), - 0x0, - 0x0 - ); - const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 4); + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TALB); + frame.text = ["foo", "bar", "baz"] + frame.textEncoding = StringType.UTF16BE; // Act - const output = frame.render(4); + const result = frame.render(4); // Assert - assert.ok(output); + assert.isOk(result); - header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 9; const expected = ByteVector.concatenate( - header.render(4), - StringType.Latin1, - ByteVector.fromString("fux", StringType.Latin1), - ByteVector.getTextDelimiter(StringType.Latin1), - ByteVector.fromString("bux", StringType.Latin1), - ByteVector.getTextDelimiter(StringType.Latin1) - ); - - Testers.bvEqual(output, expected); - } - - @test - public render_readV4NotTxxx() { - // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 15; - const data = ByteVector.concatenate( - header.render(4), + FrameIdentifiers.TALB.render(4), + ByteVector.fromUint(23), + ByteVector.fromUshort(Id3v2FrameFlags.None), StringType.UTF16BE, - ByteVector.fromString("fux", StringType.UTF16BE), + ByteVector.fromString("foo", StringType.UTF16BE), ByteVector.getTextDelimiter(StringType.UTF16BE), - ByteVector.fromString("bux", StringType.UTF16BE), - ); - const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 4); - const _ = frame.text; // Force a read - - // Act - const output = frame.render(4); - - // Assert - assert.ok(output); - Testers.bvEqual(output, data); - } - - @test - public render_readV3IsTxxxEmpty() { - // Arrange - let header = new Id3v2FrameHeader(FrameIdentifiers.TXXX); - header.frameSize = 1; - const data = ByteVector.concatenate( - header.render(3), - StringType.UTF8 - ); - const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 3); - const _ = frame.text; // Force a read - - // Act - const output = frame.render(3); - - // Assert - assert.ok(output); - - header = new Id3v2FrameHeader(FrameIdentifiers.TXXX); - header.frameSize = 3; - const expected = ByteVector.concatenate( - header.render(3), - StringType.UTF16, - ByteVector.getTextDelimiter(StringType.UTF16) + ByteVector.fromString("bar", StringType.UTF16BE), + ByteVector.getTextDelimiter(StringType.UTF16BE), + ByteVector.fromString("baz", StringType.UTF16BE) ); - - Testers.bvEqual(output, expected); + Testers.bvEqual(result, expected); } @test - public render_readV3IsTxxxSingleValue() { + public render_v4SingleByteEncoding() { // Arrange - let header = new Id3v2FrameHeader(FrameIdentifiers.TXXX); - header.frameSize = 7; - const data = ByteVector.concatenate( - header.render(3), - StringType.UTF16BE, - ByteVector.fromString("fux", StringType.UTF16BE) - ); - const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 3); - const _ = frame.text; // Force a read + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TALB); + frame.text = ["foo", "bar", "baz"] + frame.textEncoding = StringType.Latin1; // Act - const output = frame.render(3); + const result = frame.render(4); // Assert - assert.ok(output); + assert.isOk(result); - header = new Id3v2FrameHeader(FrameIdentifiers.TXXX); - header.frameSize = 9; const expected = ByteVector.concatenate( - header.render(3), - StringType.UTF16BE, - ByteVector.fromString("fux", StringType.UTF16BE), - ByteVector.getTextDelimiter(StringType.UTF16BE) + FrameIdentifiers.TALB.render(4), + ByteVector.fromUint(12), + ByteVector.fromUshort(Id3v2FrameFlags.None), + StringType.Latin1, + ByteVector.fromString("foo", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("bar", StringType.Latin1), + ByteVector.getTextDelimiter(StringType.Latin1), + ByteVector.fromString("baz", StringType.Latin1) ); - - Testers.bvEqual(output, expected); + Testers.bvEqual(result, expected); } @test - public render_readV3WithSplit() { + public render_v3WithSplit() { // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 15; - const data = ByteVector.concatenate( - header.render(3), + const bodyBytes = ByteVector.concatenate( StringType.UTF16BE, ByteVector.fromString("fux/bux", StringType.UTF16BE) ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP, Id3v2FrameFlags.None, bodyBytes.length); + const data = ByteVector.concatenate(header.render(3), bodyBytes); const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 3); - const _ = frame.text; // Force a read // Act const output = frame.render(3); @@ -561,17 +459,15 @@ const getTestFrame = (): TextInformationFrame => { } @test - public render_readV3WithoutSplit() { + public render_v3WithoutSplit() { // Arrange - const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP); - header.frameSize = 7; - const data = ByteVector.concatenate( - header.render(3), + const bodyBytes = ByteVector.concatenate( StringType.UTF16BE, ByteVector.fromString("fux", StringType.UTF16BE) ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TCOP, Id3v2FrameFlags.None, bodyBytes.length); + const data = ByteVector.concatenate(header.render(3), bodyBytes); const frame = TextInformationFrame.fromOffsetRawData(data, 0, header, 3); - const _ = frame.text; // Force a read // Act const output = frame.render(3); From fcd84ea0190ecac0ba02052a398badb634bfa091 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Sat, 23 May 2026 00:27:21 -0500 Subject: [PATCH 4/7] Robot still can't write tests well... --- src/id3v2/frames/textInformationFrame.ts | 36 +++--- test-unit/id3v2/textInformationFrameTests.ts | 12 +- .../id3v2/userTextInformationFrameTests.ts | 118 ++++++++++++++++-- test-unit/id3v2/userUrlLinkFrameTests.ts | 50 ++------ 4 files changed, 139 insertions(+), 77 deletions(-) diff --git a/src/id3v2/frames/textInformationFrame.ts b/src/id3v2/frames/textInformationFrame.ts index 8b80a70..fb169f2 100644 --- a/src/id3v2/frames/textInformationFrame.ts +++ b/src/id3v2/frames/textInformationFrame.ts @@ -432,33 +432,25 @@ export class UserTextInformationFrame extends TextInformationFrame { /** * Gets the description stored in the current instance. */ - public get description(): string { - return this._description; - } + public get description(): string { return this._description; } /** * Sets the description stored in the current instance. * There should only be one frame with the specified description per tag. * @param value Description to store in the current instance. */ - public set description(value: string) { - this._description = value; - } + public set description(value: string) { this._description = value; } /** * Gets the text contained in the current instance. * NOTE: Modifying the contents of the returned value will not modify the contents of the * current instance. The value must be reassigned for the value to change. */ - public get text(): string[] { - return this._textFields.slice(); - } + public get text(): string[] { return this._textFields.slice(); } /** * Sets the text contained in the current instance. * @param value Array of text values to store in the current instance */ - public set text(value: string[]) { - this._textFields = value ? value.slice() : []; - } + public set text(value: string[]) { this._textFields = value ? value.slice() : []; } // #endregion @@ -496,19 +488,29 @@ export class UserTextInformationFrame extends TextInformationFrame { } /** @inheritDoc */ - protected parseFields(data: ByteVector, version: number): void { - Guards.byte(version, "version"); - + protected parseFields(data: ByteVector, _version: number): void { if (data.length === 0) { this._description = undefined; this._textFields = []; return; } + // Text encoding $xx + // Description $00 (00) + // Value + this._encoding = data.get(0); + const fields = data.subarray(1).toStrings(this._encoding); - this._description = fields.length > 0 ? fields[0] : undefined; - this._textFields = fields.slice(1); + if (fields.length < 2) { + // Ill-formed frame, assume an undefined description + this._description = undefined; + this._textFields = fields; + } else { + // Well-formed frame, field 1 is description, field 2+ is data + this._description = fields[0]; + this._textFields = fields.slice(1); + } } /** @inheritDoc */ diff --git a/test-unit/id3v2/textInformationFrameTests.ts b/test-unit/id3v2/textInformationFrameTests.ts index 5211412..d00b45a 100644 --- a/test-unit/id3v2/textInformationFrameTests.ts +++ b/test-unit/id3v2/textInformationFrameTests.ts @@ -30,6 +30,12 @@ const getTestFrame = (): TextInformationFrame => { return TextInformationFrame.fromOffsetRawData; } + @test + public fromIdentifier_falsyIdentifier() { + // Act/Assert + Testers.testTruthy((v: FrameIdentifier) => { TextInformationFrame.fromIdentifier(v); }); + } + @test public fromIdentifier_noEncoding_returnsFrameWithDefaultEncoding() { // Act @@ -46,12 +52,6 @@ const getTestFrame = (): TextInformationFrame => { assert.strictEqual(frame.textEncoding, Id3v2Settings.defaultEncoding); } - @test - public fromIdentifier_falsyIdentifier() { - // Act/Assert - Testers.testTruthy((v: FrameIdentifier) => { TextInformationFrame.fromIdentifier(v); }); - } - @test public fromIdentifier_withEncoding_returnsFrameWithProvidedEncoding() { // Act diff --git a/test-unit/id3v2/userTextInformationFrameTests.ts b/test-unit/id3v2/userTextInformationFrameTests.ts index b4e9b3d..8aff06c 100644 --- a/test-unit/id3v2/userTextInformationFrameTests.ts +++ b/test-unit/id3v2/userTextInformationFrameTests.ts @@ -29,34 +29,126 @@ const getTestFrame = (): UserTextInformationFrame => { return UserTextInformationFrame.fromOffsetRawData; } + @params(undefined, "undefined") + @params(null, "null") + @params("", "empty_string") + @params("foo", "truthy") + public fromDescription_withoutEncoding(description: string) { + // Act + const frame = UserTextInformationFrame.fromDescription(description); + + // Assert + Id3v2_UserInformationFrame_ConstructorTests.assertFrame(frame, description, [], Id3v2Settings.defaultEncoding); + } + + @params(undefined, "undefined") + @params(null, "null") + @params("", "empty_string") + @params("foo", "truthy") + public fromDescription_withEncoding(description: string) { + // Act + const frame = UserTextInformationFrame.fromDescription(description, StringType.UTF16BE); + + // Assert + Id3v2_UserInformationFrame_ConstructorTests.assertFrame(frame, description, [], StringType.UTF16BE); + } + @test - public fromDescription_noEncoding_returnsFrameWithDefaultEncoding() { + public fromOffsetRawData_emptyFrame_returnsEmptyFrame() { + // Arrange + const bodyBytes = ByteVector.empty(); + const header = new Id3v2FrameHeader(FrameIdentifiers.TXXX, Id3v2FrameFlags.None, bodyBytes.length); + const data = ByteVector.concatenate( + 0x00, 0x00, + header.render(3), + bodyBytes + ); + // Act - const frame = UserTextInformationFrame.fromDescription("foo"); + const frame = UserTextInformationFrame.fromOffsetRawData(data, 2, header, 3); // Assert - Id3v2_UserInformationFrame_ConstructorTests.assertFrame(frame, "foo", [], Id3v2Settings.defaultEncoding); + assert.isOk(frame); + assert.strictEqual(frame.description, undefined); + assert.strictEqual(frame.frameId, FrameIdentifiers.TXXX); + assert.deepEqual(frame.text, []); + assert.strictEqual(frame.textEncoding, Id3v2Settings.defaultEncoding); + } + + @params(StringType.Latin1, "latin1") + @params(StringType.UTF16BE, "utf16be") + public fromOffsetRawData_wellFormed(encoding: StringType) { + // Arrange + const bodyBytes = ByteVector.concatenate( + encoding, + ByteVector.fromString("foo", encoding), + ByteVector.getTextDelimiter(encoding), + ByteVector.fromString("bar", encoding) + ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TXXX, Id3v2FrameFlags.None, bodyBytes.length); + const data = ByteVector.concatenate(header.render(4), bodyBytes); + + // Act + const output = UserTextInformationFrame.fromOffsetRawData(data, 0, header, 4); + + // Assert + assert.isOk(output); + assert.equal(output.frameClassType, FrameClassType.UserTextInformationFrame); + assert.strictEqual(output.frameId, FrameIdentifiers.TXXX); + + assert.strictEqual(output.description, "foo"); + assert.deepEqual(output.text, ["bar"]) + assert.strictEqual(output.textEncoding, encoding); } @test - public fromDescription_withEncoding_returnsFrameWithProvidedEncoding() { + public fromOffsetRawData_illFormedOneField() { + // Arrange + const bodyBytes = ByteVector.concatenate( + StringType.UTF16BE, + ByteVector.fromString("foo", StringType.UTF16BE) + ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TXXX, Id3v2FrameFlags.None, bodyBytes.length); + const data = ByteVector.concatenate(header.render(4), bodyBytes); + // Act - const frame = UserTextInformationFrame.fromDescription("foo", StringType.UTF16); + const output = UserTextInformationFrame.fromOffsetRawData(data, 0, header, 4); // Assert - Id3v2_UserInformationFrame_ConstructorTests.assertFrame(frame, "foo", [], StringType.UTF16); + assert.isOk(output); + assert.equal(output.frameClassType, FrameClassType.UserTextInformationFrame); + assert.strictEqual(output.frameId, FrameIdentifiers.TXXX); + + assert.strictEqual(output.description, undefined); + assert.deepEqual(output.text, ["foo"]) + assert.strictEqual(output.textEncoding, StringType.UTF16BE); } - @params([undefined, StringType.Latin1], "undefined") - @params([null, StringType.Latin1], "null") - @params(["", StringType.Latin1], "empty_string") - @params(["foo", StringType.Latin1], "truthy") - public fromDescription_descriptionValues([description, encoding]: [string, StringType]) { + @test + public fromOffsetRawData_illFormedThreeFields() { + // Arrange + const bodyBytes = ByteVector.concatenate( + StringType.UTF16BE, + ByteVector.fromString("foo", StringType.UTF16BE), + ByteVector.getTextDelimiter(StringType.UTF16BE), + ByteVector.fromString("bar", StringType.UTF16BE), + ByteVector.getTextDelimiter(StringType.UTF16BE), + ByteVector.fromString("baz", StringType.UTF16BE), + ); + const header = new Id3v2FrameHeader(FrameIdentifiers.TXXX, Id3v2FrameFlags.None, bodyBytes.length); + const data = ByteVector.concatenate(header.render(4), bodyBytes); + // Act - const frame = UserTextInformationFrame.fromDescription(description, encoding); + const output = UserTextInformationFrame.fromOffsetRawData(data, 0, header, 4); // Assert - Id3v2_UserInformationFrame_ConstructorTests.assertFrame(frame, description, [], encoding); + assert.isOk(output); + assert.equal(output.frameClassType, FrameClassType.UserTextInformationFrame); + assert.strictEqual(output.frameId, FrameIdentifiers.TXXX); + + assert.strictEqual(output.description, "foo"); + assert.deepEqual(output.text, ["bar", "baz"]) + assert.strictEqual(output.textEncoding, StringType.UTF16BE); } @test diff --git a/test-unit/id3v2/userUrlLinkFrameTests.ts b/test-unit/id3v2/userUrlLinkFrameTests.ts index 349c640..5c16b58 100644 --- a/test-unit/id3v2/userUrlLinkFrameTests.ts +++ b/test-unit/id3v2/userUrlLinkFrameTests.ts @@ -39,50 +39,18 @@ import {UserUrlLinkFrame} from "../../src/id3v2/frames/urlLinkFrame"; assert.throws(() => UserUrlLinkFrame.fromOffsetRawData(data, 0, header, 4)); } - @test - public fromOffsetRawData_latin1() { - // Arrange - const bodyBytes = ByteVector.concatenate( - StringType.Latin1, - ByteVector.fromString("foo", StringType.Latin1), - ByteVector.getTextDelimiter(StringType.Latin1), - ByteVector.fromString("bar", StringType.Latin1) - ); - const header = new Id3v2FrameHeader(FrameIdentifiers.WXXX, Id3v2FrameFlags.None, bodyBytes.length); - const data = ByteVector.concatenate( - 0x00, 0x00, - header.render(4), - bodyBytes - ); - - // Act - const output = UserUrlLinkFrame.fromOffsetRawData(data, 2, header, 4); - - // Assert - assert.ok(output); - assert.equal(output.frameClassType, FrameClassType.UserUrlLinkFrame); - assert.strictEqual(output.frameId, FrameIdentifiers.WXXX); - - assert.strictEqual(output.description, "foo"); - assert.strictEqual(output.text, "bar"); - assert.equal(output.textEncoding, StringType.Latin1); - } - - @test - public fromOffsetRawData_utf16() { + @params(StringType.Latin1, "latin1") + @params(StringType.UTF16BE, "utf16be") + public fromOffsetRawData_wellFormed(encoding: StringType) { // Arrange const bodyBytes = ByteVector.concatenate( - StringType.UTF16BE, - ByteVector.fromString("foo", StringType.UTF16BE), - ByteVector.getTextDelimiter(StringType.UTF16BE), - ByteVector.fromString("bar", StringType.Latin1) + encoding, + ByteVector.fromString("foo", encoding), + ByteVector.getTextDelimiter(encoding), + ByteVector.fromString("bar", encoding) ); const header = new Id3v2FrameHeader(FrameIdentifiers.WXXX, Id3v2FrameFlags.None, bodyBytes.length); - const data = ByteVector.concatenate( - 0x00, 0x00, - header.render(4), - bodyBytes - ); + const data = ByteVector.concatenate(header.render(4), bodyBytes); // Act const output = UserUrlLinkFrame.fromOffsetRawData(data, 2, header, 4); @@ -94,7 +62,7 @@ import {UserUrlLinkFrame} from "../../src/id3v2/frames/urlLinkFrame"; assert.strictEqual(output.description, "foo"); assert.strictEqual(output.text, "bar"); - assert.strictEqual(output.textEncoding, StringType.UTF16BE); + assert.equal(output.textEncoding, encoding); } @test From bcffac93cc07a567375023d5c9cc0efea3ec4ecb Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Sat, 23 May 2026 00:42:25 -0500 Subject: [PATCH 5/7] Adding a join method to ByteVector --- src/byteVector.ts | 22 ++++++ test-unit/byteVectorConstructorTests.ts | 89 +++++++++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/src/byteVector.ts b/src/byteVector.ts index c2e40f7..f70ecb3 100644 --- a/src/byteVector.ts +++ b/src/byteVector.ts @@ -600,6 +600,28 @@ export class ByteVector { return new ByteVector(bytes); } + /** + * Creates a {@link ByteVector} composed up of the `values` with `separator` inserted between + * each element. + * @param separator {@link ByteVector} to insert between elements + * @param values List of {@link ByteVector} to join with the separator + */ + public static join(separator: ByteVector, values: ByteVector[]): ByteVector { + Guards.truthy(separator, "separator"); + Guards.truthy(values, "values"); + + const vectors = []; + for (let i = 0; i < values.length; i++) { + if (i !== 0) { + vectors.push(separator); + } + + vectors.push(values[i]); + } + + return ByteVector.concatenate(... vectors); + } + // #endregion // #region Properties diff --git a/test-unit/byteVectorConstructorTests.ts b/test-unit/byteVectorConstructorTests.ts index 1177e79..3795cc4 100644 --- a/test-unit/byteVectorConstructorTests.ts +++ b/test-unit/byteVectorConstructorTests.ts @@ -1984,6 +1984,95 @@ const assert = Chai.assert; ); } + @test + public join_falsySeparator_throws() { + // Act / Assert + Testers.testTruthy((s: ByteVector) => ByteVector.join(s, [])); + } + + @test + public join_falsyValues_throws() { + // Act / Assert + Testers.testTruthy((v: ByteVector[]) => ByteVector.join(ByteVector.empty(), v)); + } + + @test + public join_emptyValues() { + // Act + const output = ByteVector.join(ByteVector.fromSize(1), []); + + // Assert + assert.isOk(output); + assert.strictEqual(output.length, 0); + } + + @test + public join_emptySeparator() { + // Arrange + const bv1 = ByteVector.fromString("foo", StringType.Latin1); + const bv2 = ByteVector.fromString("bar", StringType.Latin1); + + // Act + const output = ByteVector.join(ByteVector.empty(), [bv1, bv2]); + + // Assert + assert.isOk(output); + + const expected = ByteVector.fromString("foobar", StringType.Latin1); + Testers.bvEqual(output, expected); + } + + @test + public join_oneVectors() { + // Arrange + const bv1 = ByteVector.fromString("foo", StringType.Latin1); + const sep = ByteVector.fromString(",", StringType.Latin1); + + // Act + const output = ByteVector.join(sep, [bv1]); + + // Assert + assert.isOk(output); + + const expected = ByteVector.fromString("foo", StringType.Latin1); + Testers.bvEqual(output, expected); + } + + @test + public join_twoVectors() { + // Arrange + const bv1 = ByteVector.fromString("foo", StringType.Latin1); + const bv2 = ByteVector.fromString("bar", StringType.Latin1); + const sep = ByteVector.fromString(",", StringType.Latin1); + + // Act + const output = ByteVector.join(sep, [bv1, bv2]); + + // Assert + assert.isOk(output); + + const expected = ByteVector.fromString("foo,bar", StringType.Latin1); + Testers.bvEqual(output, expected); + } + + @test + public join_threeVectors() { + // Arrange + const bv1 = ByteVector.fromString("foo", StringType.Latin1); + const bv2 = ByteVector.fromString("bar", StringType.Latin1); + const bv3 = ByteVector.fromString("baz", StringType.Latin1); + const sep = ByteVector.fromString(",", StringType.Latin1); + + // Act + const output = ByteVector.join(sep, [bv1, bv2, bv3]); + + // Assert + assert.isOk(output); + + const expected = ByteVector.fromString("foo,bar,baz", StringType.Latin1); + Testers.bvEqual(output, expected); + } + private static equalContents(bv: ByteVector, expected: ArrayLike): void { assert.strictEqual(bv.length, expected.length); for (let i = 0; i < bv.length; i++) { From ba3cfb5f6de91d3d80704d7ab54506ffa15c16b1 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Sat, 23 May 2026 00:48:52 -0500 Subject: [PATCH 6/7] Fix a test --- src/byteVector.ts | 2 +- test-unit/id3v2/frameFactoryTests.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/byteVector.ts b/src/byteVector.ts index f70ecb3..e3ac41e 100644 --- a/src/byteVector.ts +++ b/src/byteVector.ts @@ -907,7 +907,7 @@ export class ByteVector { */ public get(index: number): number { Guards.uint(index, "index"); - Guards.lessThanInclusive(index, this.length - 1, "index"); + Guards.lessThanInclusive(index, this.length - 1, "index"); // @TODO: This behaves weird when vector is empty return this._bytes[index]; } diff --git a/test-unit/id3v2/frameFactoryTests.ts b/test-unit/id3v2/frameFactoryTests.ts index 64b5fc0..aa722be 100644 --- a/test-unit/id3v2/frameFactoryTests.ts +++ b/test-unit/id3v2/frameFactoryTests.ts @@ -109,7 +109,9 @@ import {SynchronizedTextType, TimestampFormat} from "../../src/id3v2/utilTypes"; @test public createFrame_fromData_textFrame() { // Arrange - const data = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOM).render(4); + const frame = TextInformationFrame.fromIdentifier(FrameIdentifiers.TCOM); + frame.text = ["foo"]; + const data = frame.render(4); // Act const output = Id3v2FrameFactory.createFrame(data, undefined, 0, 4, false); From 8bdedac9c913c93d43cbfc1fd2d226722892a1d1 Mon Sep 17 00:00:00 2001 From: Ben Russell Date: Sat, 23 May 2026 01:06:12 -0500 Subject: [PATCH 7/7] Fix mor test --- test-unit/id3v2/id3v2TagTests.ts | 1 + test-unit/id3v2/userUrlLinkFrameTests.ts | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test-unit/id3v2/id3v2TagTests.ts b/test-unit/id3v2/id3v2TagTests.ts index 87b3aa2..b0345c8 100644 --- a/test-unit/id3v2/id3v2TagTests.ts +++ b/test-unit/id3v2/id3v2TagTests.ts @@ -1938,6 +1938,7 @@ const getTestTagHeader = (version: number, flags: Id3v2TagHeaderFlags, tagSize: Id3v2Settings.strictFrameForVersion = true; const frame1 = TextInformationFrame.fromIdentifier(FrameIdentifiers.TYER); + frame1.text = ["foo"]; tag.frames.push(frame1); try { diff --git a/test-unit/id3v2/userUrlLinkFrameTests.ts b/test-unit/id3v2/userUrlLinkFrameTests.ts index 5c16b58..5210a0e 100644 --- a/test-unit/id3v2/userUrlLinkFrameTests.ts +++ b/test-unit/id3v2/userUrlLinkFrameTests.ts @@ -41,19 +41,19 @@ import {UserUrlLinkFrame} from "../../src/id3v2/frames/urlLinkFrame"; @params(StringType.Latin1, "latin1") @params(StringType.UTF16BE, "utf16be") - public fromOffsetRawData_wellFormed(encoding: StringType) { + public fromOffsetRawData_wellFormed(encoding: StringType = StringType.UTF16BE) { // Arrange const bodyBytes = ByteVector.concatenate( encoding, ByteVector.fromString("foo", encoding), ByteVector.getTextDelimiter(encoding), - ByteVector.fromString("bar", encoding) + ByteVector.fromString("bar", StringType.Latin1) ); const header = new Id3v2FrameHeader(FrameIdentifiers.WXXX, Id3v2FrameFlags.None, bodyBytes.length); const data = ByteVector.concatenate(header.render(4), bodyBytes); // Act - const output = UserUrlLinkFrame.fromOffsetRawData(data, 2, header, 4); + const output = UserUrlLinkFrame.fromOffsetRawData(data, 0, header, 4); // Assert assert.ok(output);