fix feed url not being updated when it changes in fetched feed#1096
fix feed url not being updated when it changes in fetched feed#1096xn-7492 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates feed parsing so the resulting ParsedFeed.feed_url can reflect the feed’s declared self/canonical URL (when present) instead of always using the request URL, enabling the app to update stored feed URLs when the fetched feed indicates a change.
Changes:
- Derive
ParsedFeed.feed_urlfromGoFeed.feedLinkwhen available (fallback to request URL otherwise). - Update Android instrumented tests to reflect the new
feed_urlbehavior. - Minor whitespace cleanup in embedded RSS test fixtures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/src/main/java/com/nononsenseapps/feeder/model/FeedParser.kt | Uses the parsed feed’s feedLink to populate ParsedFeed.feed_url so downstream sync can detect/feed URL changes. |
| app/src/androidTest/java/com/nononsenseapps/feeder/model/FeedParserTest.kt | Adjusts assertions and fixtures to match the updated feed_url selection logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| home_page_url = link?.let { relativeLinkIntoAbsolute(url, it) }, | ||
| // Keep original URL to maintain authentication data and/or tokens in query params | ||
| feed_url = url.toString(), | ||
| feed_url = feedLink?.let { relativeLinkIntoAbsolute(url, it) } ?: url.toString(), |
There was a problem hiding this comment.
feed_url is now derived from feedLink via relativeLinkIntoAbsolute, which can return the original (potentially invalid) feedLink string on parse failure. That makes ParsedFeed.feed_url no longer guaranteed to be a valid URL (previously it always fell back to url.toString()), and downstream code (e.g. sync storing the feed URL) may end up persisting an invalid URL. Consider validating/parsing the resolved feedLink and falling back to the request URL when it can’t be parsed (and optionally treating blank/whitespace-only feedLink as missing).
| assertEquals("http://ANON.com/sub", feed.getOrNull()!!.home_page_url) | ||
| assertEquals("http://anon.com/rss", feed.getOrNull()!!.feed_url) | ||
| assertEquals("http://ANON.com/rss", feed.getOrNull()!!.feed_url) |
There was a problem hiding this comment.
This assertion makes the test sensitive to the exact casing of the host in feed_url ("ANON.com" vs "anon.com"), even though host casing is not semantically meaningful and may vary depending on how the URL was produced (OkHttp vs java.net.URL). To keep the test robust, consider normalizing before comparing (e.g., parse as URL and compare host.lowercase() + path, or compare via URI), rather than asserting the raw string form.
Currently the feed xml url doen't get updated when it changes in the fetched feed.
RssLocalSync already prevents updating links that contain auth or query params.