Skip to content

Commit f604fb8

Browse files
bug-opsbug-ops
committed
perf(bindings): optimize string allocations and add edge case tests
- Python: Return &str instead of String for update_period() to avoid allocation - Python: Optimize __repr__ to access fields directly without allocations - Node.js: Simplify Entry conversion using collect() for all Vec fields - Add tests for invalid input handling (bozo pattern) - Add tests for case-insensitive updatePeriod parsing - Add tests for partial syndication metadata All tests passing, clippy clean.
1 parent 574ec7e commit f604fb8

File tree

4 files changed

+127
-59
lines changed

4 files changed

+127
-59
lines changed

‎crates/feedparser-rs-node/__test__/syndication.spec.mjs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,4 +91,57 @@ describe('syndication', () => {
9191
assert.strictEqual(feed.feed.dcPublisher, 'ACME Corp');
9292
assert.strictEqual(feed.feed.dcRights, 'Copyright 2024');
9393
});
94+
95+
it('should handle invalid updatePeriod gracefully (bozo pattern)', () => {
96+
const xml = `<?xml version="1.0"?>
97+
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
98+
xmlns="http://purl.org/rss/1.0/"
99+
xmlns:syn="http://purl.org/rss/1.0/modules/syndication/">
100+
<channel>
101+
<title>Test</title>
102+
<link>https://example.com</link>
103+
<syn:updatePeriod>invalid</syn:updatePeriod>
104+
</channel>
105+
</rdf:RDF>`;
106+
107+
const feed = parse(xml);
108+
// Should not crash, syndication should be undefined or updatePeriod undefined
109+
assert.ok(!feed.feed.syndication || !feed.feed.syndication.updatePeriod);
110+
});
111+
112+
it('should handle case-insensitive updatePeriod', () => {
113+
const xml = `<?xml version="1.0"?>
114+
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
115+
xmlns="http://purl.org/rss/1.0/"
116+
xmlns:syn="http://purl.org/rss/1.0/modules/syndication/">
117+
<channel>
118+
<title>Test</title>
119+
<link>https://example.com</link>
120+
<syn:updatePeriod>HOURLY</syn:updatePeriod>
121+
</channel>
122+
</rdf:RDF>`;
123+
124+
const feed = parse(xml);
125+
assert.ok(feed.feed.syndication);
126+
assert.strictEqual(feed.feed.syndication.updatePeriod, 'hourly');
127+
});
128+
129+
it('should parse feed with partial syndication fields', () => {
130+
const xml = `<?xml version="1.0"?>
131+
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
132+
xmlns="http://purl.org/rss/1.0/"
133+
xmlns:syn="http://purl.org/rss/1.0/modules/syndication/">
134+
<channel>
135+
<title>Test</title>
136+
<link>https://example.com</link>
137+
<syn:updatePeriod>weekly</syn:updatePeriod>
138+
</channel>
139+
</rdf:RDF>`;
140+
141+
const feed = parse(xml);
142+
assert.ok(feed.feed.syndication);
143+
assert.strictEqual(feed.feed.syndication.updatePeriod, 'weekly');
144+
assert.strictEqual(feed.feed.syndication.updateFrequency, undefined);
145+
assert.strictEqual(feed.feed.syndication.updateBase, undefined);
146+
});
94147
});

‎crates/feedparser-rs-node/src/lib.rs‎

Lines changed: 16 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -445,77 +445,39 @@ pub struct Entry {
445445

446446
impl From<CoreEntry> for Entry {
447447
fn from(core: CoreEntry) -> Self {
448-
// Pre-allocate Vec capacity to avoid reallocations
449-
let links_cap = core.links.len();
450-
let content_cap = core.content.len();
451-
let authors_cap = core.authors.len();
452-
let contributors_cap = core.contributors.len();
453-
let tags_cap = core.tags.len();
454-
let enclosures_cap = core.enclosures.len();
455-
let transcripts_cap = core.podcast_transcripts.len();
456-
let persons_cap = core.podcast_persons.len();
457-
458448
Self {
459449
id: core.id,
460450
title: core.title,
461451
title_detail: core.title_detail.map(TextConstruct::from),
462452
link: core.link,
463-
links: {
464-
let mut v = Vec::with_capacity(links_cap);
465-
v.extend(core.links.into_iter().map(Link::from));
466-
v
467-
},
453+
links: core.links.into_iter().map(Link::from).collect(),
468454
summary: core.summary,
469455
summary_detail: core.summary_detail.map(TextConstruct::from),
470-
content: {
471-
let mut v = Vec::with_capacity(content_cap);
472-
v.extend(core.content.into_iter().map(Content::from));
473-
v
474-
},
456+
content: core.content.into_iter().map(Content::from).collect(),
475457
published: core.published.map(|dt| dt.timestamp_millis()),
476458
updated: core.updated.map(|dt| dt.timestamp_millis()),
477459
created: core.created.map(|dt| dt.timestamp_millis()),
478460
expired: core.expired.map(|dt| dt.timestamp_millis()),
479461
author: core.author,
480462
author_detail: core.author_detail.map(Person::from),
481-
authors: {
482-
let mut v = Vec::with_capacity(authors_cap);
483-
v.extend(core.authors.into_iter().map(Person::from));
484-
v
485-
},
486-
contributors: {
487-
let mut v = Vec::with_capacity(contributors_cap);
488-
v.extend(core.contributors.into_iter().map(Person::from));
489-
v
490-
},
463+
authors: core.authors.into_iter().map(Person::from).collect(),
464+
contributors: core.contributors.into_iter().map(Person::from).collect(),
491465
publisher: core.publisher,
492466
publisher_detail: core.publisher_detail.map(Person::from),
493-
tags: {
494-
let mut v = Vec::with_capacity(tags_cap);
495-
v.extend(core.tags.into_iter().map(Tag::from));
496-
v
497-
},
498-
enclosures: {
499-
let mut v = Vec::with_capacity(enclosures_cap);
500-
v.extend(core.enclosures.into_iter().map(Enclosure::from));
501-
v
502-
},
467+
tags: core.tags.into_iter().map(Tag::from).collect(),
468+
enclosures: core.enclosures.into_iter().map(Enclosure::from).collect(),
503469
comments: core.comments,
504470
source: core.source.map(Source::from),
505-
podcast_transcripts: {
506-
let mut v = Vec::with_capacity(transcripts_cap);
507-
v.extend(
508-
core.podcast_transcripts
509-
.into_iter()
510-
.map(PodcastTranscript::from),
511-
);
512-
v
513-
},
514-
podcast_persons: {
515-
let mut v = Vec::with_capacity(persons_cap);
516-
v.extend(core.podcast_persons.into_iter().map(PodcastPerson::from));
517-
v
518-
},
471+
podcast_transcripts: core
472+
.podcast_transcripts
473+
.into_iter()
474+
.map(PodcastTranscript::from)
475+
.collect(),
476+
podcast_persons: core
477+
.podcast_persons
478+
.into_iter()
479+
.map(PodcastPerson::from)
480+
.collect(),
519481
license: core.license,
520482
}
521483
}

‎crates/feedparser-rs-py/src/types/syndication.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ impl PySyndicationMeta {
1818
impl PySyndicationMeta {
1919
/// Update period (hourly, daily, weekly, monthly, yearly)
2020
#[getter]
21-
fn update_period(&self) -> Option<String> {
22-
self.inner.update_period.map(|p| p.as_str().to_string())
21+
fn update_period(&self) -> Option<&str> {
22+
self.inner.update_period.as_ref().map(|p| p.as_str())
2323
}
2424

2525
/// Number of times updated per period
@@ -37,9 +37,9 @@ impl PySyndicationMeta {
3737
fn __repr__(&self) -> String {
3838
format!(
3939
"SyndicationMeta(update_period={:?}, update_frequency={:?}, update_base={:?})",
40-
self.update_period(),
41-
self.update_frequency(),
42-
self.update_base()
40+
self.inner.update_period.as_ref().map(|p| p.as_str()),
41+
self.inner.update_frequency,
42+
self.inner.update_base.as_deref()
4343
)
4444
}
4545
}

‎crates/feedparser-rs-py/tests/test_syndication.py‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,56 @@ def test_dublin_core_fields():
111111
assert d.feed.dc_creator == "John Doe"
112112
assert d.feed.dc_publisher == "ACME Corp"
113113
assert d.feed.dc_rights == "Copyright 2024"
114+
115+
116+
def test_invalid_update_period():
117+
"""Test invalid updatePeriod is handled gracefully (bozo pattern)"""
118+
feed_xml = b"""<?xml version="1.0"?>
119+
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
120+
xmlns="http://purl.org/rss/1.0/"
121+
xmlns:syn="http://purl.org/rss/1.0/modules/syndication/">
122+
<channel>
123+
<title>Test</title>
124+
<link>https://example.com</link>
125+
<syn:updatePeriod>invalid</syn:updatePeriod>
126+
</channel>
127+
</rdf:RDF>"""
128+
d = feedparser_rs.parse(feed_xml)
129+
# Should not crash, syndication should be None or update_period None
130+
assert d.feed.syndication is None or d.feed.syndication.update_period is None
131+
132+
133+
def test_case_insensitive_update_period():
134+
"""Test updatePeriod is case-insensitive"""
135+
feed_xml = b"""<?xml version="1.0"?>
136+
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
137+
xmlns="http://purl.org/rss/1.0/"
138+
xmlns:syn="http://purl.org/rss/1.0/modules/syndication/">
139+
<channel>
140+
<title>Test</title>
141+
<link>https://example.com</link>
142+
<syn:updatePeriod>HOURLY</syn:updatePeriod>
143+
</channel>
144+
</rdf:RDF>"""
145+
d = feedparser_rs.parse(feed_xml)
146+
assert d.feed.syndication is not None
147+
assert d.feed.syndication.update_period == "hourly"
148+
149+
150+
def test_partial_syndication():
151+
"""Test feed with only some syndication fields"""
152+
feed_xml = b"""<?xml version="1.0"?>
153+
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
154+
xmlns="http://purl.org/rss/1.0/"
155+
xmlns:syn="http://purl.org/rss/1.0/modules/syndication/">
156+
<channel>
157+
<title>Test</title>
158+
<link>https://example.com</link>
159+
<syn:updatePeriod>weekly</syn:updatePeriod>
160+
</channel>
161+
</rdf:RDF>"""
162+
d = feedparser_rs.parse(feed_xml)
163+
assert d.feed.syndication is not None
164+
assert d.feed.syndication.update_period == "weekly"
165+
assert d.feed.syndication.update_frequency is None
166+
assert d.feed.syndication.update_base is None

0 commit comments

Comments
 (0)