Skip to content

Fix misc ddex4.0 and sme processing bugs #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix misc ddex4.0 and sme processing bugs #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1d569fe
Process batches in order to handle purge messages correctly
raymondjacobson Aug 5, 2025
9f0177a
Add signed stream URL for fast playback
raymondjacobson Aug 10, 2025
d65dbbf
Add status column
raymondjacobson Aug 10, 2025
5b76967
Fix issues with ddex4.0, add better ui status
raymondjacobson Aug 20, 2025
12f58f6
Fix failure send
raymondjacobson Aug 20, 2025
3b723ea
Remove logs
raymondjacobson Aug 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"@audius/sdk": "^9.0.0",
"@aws-sdk/client-s3": "3.658.1",
"@aws-sdk/credential-provider-ini": "3.658.1",
"@aws-sdk/s3-request-presigner": "^3.658.1",
"@hono/node-server": "^1.13.1",
"cheerio": "^1.0.0",
"commander": "^12.1.0",
Expand Down
54 changes: 29 additions & 25 deletions src/acknowledgement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,14 @@ function generateAcknowledgementXml({
releases,
isSuccess,
error,
recipientPartyId,
}: {
source: string,
messageId: string,
releases: DDEXRelease[],
isSuccess: boolean,
error?: string
error?: string,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this typical TS? or some other kind of error?

recipientPartyId?: string
}): string {
const $ = cheerio.load('', { xmlMode: true })

Expand All @@ -139,7 +141,8 @@ function generateAcknowledgementXml({

// MessageRecipient (Source)
const messageRecipient = $('<MessageRecipient></MessageRecipient>')
messageRecipient.append($(`<PartyId>${source.toUpperCase()}</PartyId>`))
const resolvedRecipient = recipientPartyId || (source === 'sme' ? 'PADPIDA2007040502I' : source.toUpperCase())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what our prototype/roughness comfort level is in this repo. But this seems like it should be a constant and/or some kind of function that can determine the value. Having the source check for a particular label hard-coded in here feels cryptic and hard to maintain :-D

messageRecipient.append($(`<PartyId>${resolvedRecipient}</PartyId>`))
const recipientPartyName = $('<PartyName></PartyName>')
recipientPartyName.append($(`<FullName>${getSourcePartyName(source)}</FullName>`))
messageRecipient.append(recipientPartyName)
Expand Down Expand Up @@ -175,31 +178,27 @@ function generateAcknowledgementXml({
// ReleaseStatus - different values for success vs error
if (isSuccess) {
releaseStatus.append($('<ReleaseStatus>SuccessfullyIngestedByReleaseDistributor</ReleaseStatus>'))
const acknowledgement = $('<Acknowledgement></Acknowledgement>')
acknowledgement.append($('<MessageType>NewReleaseMessage</MessageType>'))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why it feels wild to me that we're doing all of these lines with string literals for tag names to incrementally build XML. This def feels like something that could eventually be append(Tags.MessageType, Types.NewRelease) etc...
But again, probably not that point in dev for this repo...

acknowledgement.append($(`<MessageId>${messageId}</MessageId>`))
const messageStatus = $('<MessageStatus></MessageStatus>')
messageStatus.append($('<Status>FileOK</Status>'))
acknowledgement.append(messageStatus)
releaseStatus.append(acknowledgement)
} else {
releaseStatus.append($('<ReleaseStatus>ProcessingErrorAtReleaseDistributor</ReleaseStatus>'))

// Add ErrorText for failures (at ReleaseStatus level)
const acknowledgement = $('<Acknowledgement></Acknowledgement>')
acknowledgement.append($('<MessageType>NewReleaseMessage</MessageType>'))
acknowledgement.append($(`<MessageId>${messageId}</MessageId>`))
const messageStatus = $('<MessageStatus></MessageStatus>')
messageStatus.append($('<Status>ResourceCorrupt</Status>'))
if (error) {
releaseStatus.append($(`<ErrorText>${error}</ErrorText>`))
messageStatus.append($(`<StatusMessage>${error}</StatusMessage>`))
}
acknowledgement.append(messageStatus)
releaseStatus.append(acknowledgement)
}

// Acknowledgement
const acknowledgement = $('<Acknowledgement></Acknowledgement>')
acknowledgement.append($('<MessageType>NewReleaseMessage</MessageType>'))
acknowledgement.append($(`<MessageId>${messageId}</MessageId>`))

const messageStatus = $('<MessageStatus></MessageStatus>')
if (isSuccess) {
messageStatus.append($('<Status>FileOK</Status>'))
} else {
// Use ResourceCorrupt for errors as per the example
messageStatus.append($('<Status>ResourceCorrupt</Status>'))
}

acknowledgement.append(messageStatus)
releaseStatus.append(acknowledgement)

root.append(releaseStatus)
}
} else {
Expand Down Expand Up @@ -263,7 +262,8 @@ async function sendAcknowledgement(source: string, xml: string) {
})

if (!statusResponse.ok) {
throw new Error(`Status post failed: ${statusResponse.status} ${statusResponse.statusText}`)
const errText = await statusResponse.text().catch(() => '')
throw new Error(`Status post failed: ${statusResponse.status} ${statusResponse.statusText} ${errText ? '- ' + errText : ''}`)
}

console.log('Acknowledgement XML posted successfully')
Expand Down Expand Up @@ -315,7 +315,8 @@ export async function acknowledgeReleaseSuccess({
source,
messageId,
releases,
isSuccess: true
isSuccess: true,
recipientPartyId: source === 'sme' ? 'PADPIDA2007040502I' : undefined
}
)
console.log(acknowledgementXml)
Expand All @@ -337,12 +338,14 @@ export async function acknowledgeReleaseFailure({
messageId,
messageTimestamp,
error,
releases,
}: {
source: string
xmlUrl: string
messageId: string
messageTimestamp: string
error: string | Error
releases?: DDEXRelease[]
}) {
const errorMessage = error instanceof Error ? error.message : error

Expand All @@ -366,9 +369,10 @@ export async function acknowledgeReleaseFailure({
{
source,
messageId,
releases: [], // Empty releases array for failures
releases: releases || [], // Include releases when available
isSuccess: false,
error: errorMessage
error: errorMessage,
recipientPartyId: source === 'sme' ? 'PADPIDA2007040502I' : undefined
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that I've seen it thrice, definitely should be a helper

}
)
console.log(acknowledgementXml)
Expand Down
107 changes: 94 additions & 13 deletions src/parseDelivery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,27 @@ export async function parseDdexXml(
// Only send acknowledgement for SME
source === 'sme'
) {
await acknowledgeReleaseSuccess({
source,
xmlUrl,
messageId,
messageTimestamp,
releases,
})
const hasErrors = releases.some((r) =>
r.problems.includes('InvalidDealDate')
)
if (hasErrors) {
await acknowledgeReleaseFailure({
source,
xmlUrl,
messageId,
messageTimestamp,
releases,
error: 'InvalidDealDate',
})
} else {
await acknowledgeReleaseSuccess({
source,
xmlUrl,
messageId,
messageTimestamp,
releases,
})
}
}

return releases
Expand Down Expand Up @@ -401,7 +415,7 @@ async function parseReleaseXml(source: string, $: cheerio.CheerioAPI, isDdex40:
// DDEX 4.0 structure: DealList > ReleaseDeal > Deal > DealTerms
$('DealList > ReleaseDeal').each((_, el) => {
const $el = $(el)
const ref = $el.find('ReleaseReference').text()
const refs = $el.find('DealReleaseReference').toArray().map(el => $(el).text());

$el.find('Deal > DealTerms').each((_, el) => {
const $el = $(el)
Expand Down Expand Up @@ -434,8 +448,10 @@ async function parseReleaseXml(source: string, $: cheerio.CheerioAPI, isDdex40:

// add deal
function addDeal(deal: AudiusSupportedDeal) {
releaseDeals[ref] ||= []
releaseDeals[ref].push(deal)
for (const ref of refs) {
releaseDeals[ref] ||= []
releaseDeals[ref].push(deal)
}
}

const common: DealFields = {
Expand Down Expand Up @@ -770,7 +786,38 @@ async function parseReleaseXml(source: string, $: cheerio.CheerioAPI, isDdex40:
const $el = $(el)

const ref = $el.find('ReleaseReference').text()
const deals = releaseDeals[ref] || []
let deals = releaseDeals[ref] || []

// For DDEX 4.0, if the album has no direct deals, aggregate from TrackReleases
if (deals.length === 0 && isDdex40) {
// Build TrackRelease -> Resource mapping
const trackRefToResourceRef: Record<string, string> = {}
$('ReleaseList > TrackRelease').each((_, el) => {
const $tr = $(el)
const trRef = $tr.find('ReleaseReference').text()
const rrRef = $tr.find('ReleaseResourceReference').text()
if (trRef && rrRef) trackRefToResourceRef[trRef] = rrRef
})

// Collect resource refs contained in this album
const releaseResourceRefs = new Set<string>()
$el
.find('ResourceGroup ReleaseResourceReference, ResourceGroupContentItem > ReleaseResourceReference')
.each((_, el) => {
releaseResourceRefs.add($(el).text())
})

// Find TrackReleases that are members of this album
const memberTrackRefs = Object.entries(trackRefToResourceRef)
.filter(([, resRef]) => releaseResourceRefs.has(resRef))
.map(([trRef]) => trRef)

const aggregated = memberTrackRefs.flatMap((r) => releaseDeals[r] || [])
if (aggregated.length) {
deals = aggregated
}
}

const validityStartDate = deals.length
? deals[0].validityStartDate
: undefined
Expand Down Expand Up @@ -812,7 +859,7 @@ async function parseReleaseXml(source: string, $: cheerio.CheerioAPI, isDdex40:
producerCopyrightLine: pline($el),
parentalWarningType: toText($el.find('ParentalWarningType')),

isMainRelease: $el.attr('IsMainRelease') == 'true',
isMainRelease: isDdex40 ? true : $el.attr('IsMainRelease') == 'true',

problems: [],
soundRecordings: [],
Expand Down Expand Up @@ -863,6 +910,40 @@ async function parseReleaseXml(source: string, $: cheerio.CheerioAPI, isDdex40:
release.problems.push('NoDeal')
}

// Validate FirstPublicationDate/recording releaseDate
{
const hasInvalidDate = release.soundRecordings.some((sr) => {
const dateText = sr.releaseDate || ''
const m = dateText.match(/(\d{4})/)
if (!m) return true
const year = parseInt(m[1])
if (!year || year <= 1900) return true
return false
})
if (hasInvalidDate) {
release.problems.push('InvalidFirstPublicationDate')
}
}

// Validate deal start vs OriginalReleaseDate
{
const originalReleaseDateText = $el.find('OriginalReleaseDate').text()
if (originalReleaseDateText) {
const ord = new Date(originalReleaseDateText)
if (!isNaN(ord.getTime())) {
const invalidDeal = release.deals.some((d) => {
if (!d.validityStartDate) return false
const ds = new Date(d.validityStartDate)
if (isNaN(ds.getTime())) return false
return ds < ord
})
if (invalidDeal) {
release.problems.push('InvalidDealDate')
}
}
}
}

return release
})

Expand Down Expand Up @@ -967,7 +1048,7 @@ function resolveAudiusGenre(

// maybe try some edit distance magic?
// for now just log
console.warn(`failed to resolve genre: subgenre=${subgenre} genre=${genre}`)
console.warn(`no matching genre: subgenre=${subgenre} genre=${genre}`)
}

export function parseDuration(dur: string) {
Expand Down
Loading