Skip to content
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

Jump to bottom

atom + utf-8 support. #5

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

atom + utf-8 support. #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1ceebf9
utf8, by default.
fliot Oct 14, 2022
80289a0
xmlns:atom by default
fliot Oct 14, 2022
d7058b4
utf8, by default.
fliot Oct 14, 2022
af9ad41
xmlns:atom by default
fliot Oct 14, 2022
3af2c49
xmlns:atom by default
fliot Oct 14, 2022
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
15 changes: 10 additions & 5 deletions fastapi_rss/models/feed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def _generate_tree_list(root: etree.Element, key: str, value: List[dict]) -> Non
if content is not None:
itemroot.text = content
else:
RSSFeed.generate_tree(itemroot, item)
RSSFeed.generate_tree(itemroot, item, {})
Copy link
Owner

Choose a reason for hiding this comment

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

This change won't be necessary (see other comments)


@staticmethod
def _generate_tree_object(root: etree._Element, key: str, value: Union[dict, BaseModel]) -> None:
Expand Down Expand Up @@ -92,7 +92,7 @@ def _generate_tree_default(root: etree._Element, key: str, value: Any) -> None:
element.text = str(value)

@staticmethod
def generate_tree(root: etree.Element, dict_: dict):
def generate_tree(root: etree.Element, dict_: dict, nsmap: dict):
handlers = {
(list, ): RSSFeed._generate_tree_list,
(BaseModel, dict): RSSFeed._generate_tree_object,
Expand All @@ -107,10 +107,15 @@ def generate_tree(root: etree.Element, dict_: dict):
break
else:
RSSFeed._generate_tree_default(root, key, value)
if key == "docs" and 'http://www.w3.org/2005/Atom' in nsmap.values():
atom_link = etree.SubElement(root, '{http://www.w3.org/2005/Atom}link', nsmap=nsmap)
atom_link.set('href', value)
atom_link.set('rel', 'self')
atom_link.set('type', 'application/rss+xml')

def tostring(self, nsmap: Optional[Dict[str, str]] = None):
nsmap = nsmap or {}
nsmap = nsmap or {'atom': 'http://www.w3.org/2005/Atom'}
Copy link
Owner

Choose a reason for hiding this comment

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

Do not default to atom mode. The library must be kept consistent as is (i.e. compliant with the original RSS specification).

In order to ease the implementation I am not opposed to dropping the nsmap kwarg and adding an atom / podcast kwarg to generate the nsmap accordingly.

rss = etree.Element('rss', version='2.0', nsmap=nsmap)
channel = etree.SubElement(rss, 'channel')
RSSFeed.generate_tree(channel, self.dict())
return etree.tostring(rss, pretty_print=True, xml_declaration=True)
RSSFeed.generate_tree(channel, self.dict(), nsmap)
return etree.tostring(rss, pretty_print=True, xml_declaration=True, encoding='UTF-8')
Copy link
Owner

Choose a reason for hiding this comment

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

I should have originally encoded the string as utf-8, I agree, but the original RSS specification didn't mention a specific encoding to use (to be fair, at the time of the spec for RSS 2.0, UTF-8 wasn't widely supported yet).

Encoding as UTF-8 can be the default, but it should be able to be opted out if need be, in order to keep the library consistent with its previous version.

Furthermore, such a change will warrant a major version bump. Please upgrade the version in pyproject.toml to reflect that.

2 changes: 1 addition & 1 deletion tests/test_rss_response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def test_rss_sample_response(client: TestClient, expected_response: str, capsys)
response = client.get('/1')
assert response.status_code == 200
tree = etree.fromstring(response.content)
pretty: str = etree.tostring(tree, pretty_print=True, xml_declaration=True).decode('ascii')
pretty: str = etree.tostring(tree, pretty_print=True, xml_declaration=True).decode('utf-8')
Copy link
Owner

Choose a reason for hiding this comment

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

With my other comment, this change is not needed anymore, instead make a new test specifically for the utf8 encoded version.

for line, expected_line in zip(pretty.splitlines(), expected_response.splitlines()):
assert dedent(line) == dedent(expected_line), f'{line} != {expected_line}'

Expand Down