refactor(render): этап 6 — косметика (#33, #34)
Docker Image CI / build (pull_request) Waiting to run
Docker Image CI / build (pull_request) Waiting to run
Финальный этап render-pipeline рефактора. Чисто косметика, вывод байт-в-байт неизменен (golden-оракул нетронут, 0 изменений в tests/test_data/). 1. `_wrap_post_html(body, footer)` — единая обёртка поста вместо трёх байт-идентичных копий (_format_html + обе ветки _render_messages_groups). 2. Инлайн-стили 400px/200px/max-width → именованные константы. 3. `_trim_messages_groups` заинлайнен как `[:limit]`; импорты в test_stage4_eventloop.py/test_group_ids.py перевязаны на _render_pipeline без потери покрытия тримминга. 4. Устаревшие комментарии «stage-4»/«4.4» переписаны под текущую границу санитайза. 5. Фильтр exclude_flags → list-comprehension (эквивалент по де Моргану); фильтр exclude_text и его excluded_post-debug-лог сохранены дословно. Гейт: pytest 443 passed (без дельты); test_stage0_golden зелёный под PYTHONHASHSEED 0/1; ни одной golden-фикстуры не изменено. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+39
-23
@@ -175,6 +175,23 @@ MEDIA_SOURCES: Dict[Any, Callable[[Message], Tuple[Any, Optional[str]]]] = {
|
||||
}
|
||||
|
||||
|
||||
# Inline media-sizing style literals, named so the renderers below read as intent
|
||||
# rather than magic numbers. Values are substituted verbatim into the style strings,
|
||||
# so the emitted bytes are unchanged.
|
||||
MEDIA_MAX_HEIGHT_PX = "400px" # standard image/video max-height
|
||||
MEDIA_MAX_HEIGHT_SMALL_PX = "200px" # looping/sticker media max-height
|
||||
MEDIA_MAX_WIDTH_PX = "400px" # audio player max-width
|
||||
|
||||
|
||||
def _wrap_post_html(body: str, footer: str) -> str:
|
||||
"""Shared post-HTML shell: the message-body + message-footer div pair joined by a
|
||||
single newline. Used by PostParser._format_html (single/debug post) and by both
|
||||
branches of rss_generator._render_messages_groups (single message and merged
|
||||
group), so the emitted byte structure stays identical across all three call sites."""
|
||||
return (f'<div class="message-body">{body}</div>\n'
|
||||
f'<div class="message-footer">{footer}</div>')
|
||||
|
||||
|
||||
@dataclass
|
||||
class RenderCtx:
|
||||
"""Everything a renderer needs. _generate_html_media assembles it (URL signing,
|
||||
@@ -193,34 +210,34 @@ class RenderCtx:
|
||||
# byte-for-byte fidelity is the stage-5a contract.
|
||||
def _render_img_400(ctx: 'RenderCtx') -> List[str]:
|
||||
return [f'<img src="{ctx.url}" style="max-width:100%; width:auto; height:auto;'
|
||||
f'max-height:400px; object-fit:contain;">']
|
||||
f'max-height:{MEDIA_MAX_HEIGHT_PX}; object-fit:contain;">']
|
||||
|
||||
|
||||
def _render_video_400(ctx: 'RenderCtx') -> List[str]:
|
||||
return [f'<video controls src="{ctx.url}" style="max-width:100%; width:auto;'
|
||||
f'height:auto; max-height:400px;"></video>']
|
||||
f'height:auto; max-height:{MEDIA_MAX_HEIGHT_PX};"></video>']
|
||||
|
||||
|
||||
def _render_audio(ctx: 'RenderCtx') -> List[str]:
|
||||
return [f'<audio controls style="width:100%; max-width:400px;">'
|
||||
return [f'<audio controls style="width:100%; max-width:{MEDIA_MAX_WIDTH_PX};">'
|
||||
f'<source src="{ctx.url}" type="{ctx.mime}"></audio>',
|
||||
'<br>']
|
||||
|
||||
|
||||
def _render_video_loop_200(ctx: 'RenderCtx') -> List[str]:
|
||||
return [f'<video controls autoplay loop muted src="{ctx.url}"'
|
||||
f'style="max-width:100%; width:auto; height:auto; max-height:200px;'
|
||||
f'style="max-width:100%; width:auto; height:auto; max-height:{MEDIA_MAX_HEIGHT_SMALL_PX};'
|
||||
f'object-fit:contain;"></video>']
|
||||
|
||||
|
||||
def _render_img_200_sticker(ctx: 'RenderCtx') -> List[str]:
|
||||
return [f'<img src="{ctx.url}" alt="Sticker {ctx.emoji}" style="max-width:100%;'
|
||||
f'width:auto; height:auto; max-height:200px; object-fit:contain;">']
|
||||
f'width:auto; height:auto; max-height:{MEDIA_MAX_HEIGHT_SMALL_PX}; object-fit:contain;">']
|
||||
|
||||
|
||||
def _render_video_loop_400(ctx: 'RenderCtx') -> List[str]:
|
||||
return [f'<video controls autoplay loop muted src="{ctx.url}"'
|
||||
f'style="max-width:100%; width:auto; height:auto; max-height:400px;'
|
||||
f'style="max-width:100%; width:auto; height:auto; max-height:{MEDIA_MAX_HEIGHT_PX};'
|
||||
f'object-fit:contain;"></video>']
|
||||
|
||||
|
||||
@@ -732,8 +749,7 @@ class PostParser:
|
||||
# through bleach — escape it before embedding, same as raw_message below.
|
||||
title_escaped = html.escape(str(data["html"]["title"]))
|
||||
html_content.append(f'<div class="title">Title: {title_escaped}</div><br>')
|
||||
html_content.append(f'<div class="message-body">{data["html"]["body"]}</div>')
|
||||
html_content.append(f'<div class="message-footer">{data["html"]["footer"]}</div>')
|
||||
html_content.append(_wrap_post_html(data["html"]["body"], data["html"]["footer"]))
|
||||
|
||||
# Add raw JSON debug output if debug is enabled.
|
||||
# raw_message is the full str(message) serialization and may contain user-controlled
|
||||
@@ -776,17 +792,17 @@ class PostParser:
|
||||
than once.
|
||||
"""
|
||||
# Compute html body once — avoids triple _generate_html_body calls.
|
||||
# The internal per-fragment sanitize passes were removed (4.4); sanitize
|
||||
# exactly once per output boundary here when requested.
|
||||
# The internal per-fragment sanitize passes are gone; sanitize runs exactly
|
||||
# once at the output boundary here when requested, never inside body/footer.
|
||||
html_body = self._generate_html_body(message)
|
||||
# NOTE (stage-4 4.4 consequence): flags are now extracted from the PRE-sanitize
|
||||
# body (the per-fragment sanitize that used to run inside _generate_html_body was
|
||||
# removed). Legitimate links are unaffected — bleach keeps whitelisted
|
||||
# <a href="http(s)://…"> — so link/foreign_channel/mention flags are identical for
|
||||
# normal content. They can differ ONLY for URL-like text that bleach would strip
|
||||
# (e.g. a URL inside a disallowed attribute); flags are non-security (used for
|
||||
# exclude_flags filtering / display), so this edge divergence is accepted rather
|
||||
# than re-adding a per-message sanitize pass that 4.4 deliberately eliminated.
|
||||
# NOTE: flags are extracted from the PRE-sanitize body (there is no longer a
|
||||
# per-fragment sanitize inside _generate_html_body). Legitimate links are
|
||||
# unaffected — bleach keeps whitelisted <a href="http(s)://…"> — so
|
||||
# link/foreign_channel/mention flags are identical for normal content. They can
|
||||
# differ ONLY for URL-like text that bleach would strip (e.g. a URL inside a
|
||||
# disallowed attribute); flags are non-security (used for exclude_flags filtering
|
||||
# / display), so this edge divergence is accepted rather than re-adding the
|
||||
# per-message sanitize pass that was deliberately eliminated.
|
||||
flags = self._extract_flags(message, html_body=html_body)
|
||||
footer = self.generate_html_footer(message, flags_list=flags)
|
||||
if sanitize:
|
||||
@@ -930,9 +946,9 @@ class PostParser:
|
||||
if message.forward_origin: content_body.append(f"--- Forwarded post end ---") # Forward info end
|
||||
content_body.append(f'</div><br>')
|
||||
|
||||
# NOTE: sanitize is NOT applied here. Sanitization happens exactly once per
|
||||
# NOTE: sanitize is NOT applied here. Sanitization happens exactly once at the
|
||||
# output boundary (process_message for single-post/JSON; in rss_generator the
|
||||
# per-post pass in _render_pipeline for BOTH RSS and HTML). See the map (4.4).
|
||||
# per-post pass in _render_pipeline for BOTH RSS and HTML).
|
||||
html_body = '\n'.join(content_body)
|
||||
return html_body
|
||||
|
||||
@@ -1017,7 +1033,7 @@ class PostParser:
|
||||
content_media.append('</div>')
|
||||
|
||||
# Not sanitized here — this fragment is embedded in the body and sanitized
|
||||
# once at the output boundary (see the 4.4 sanitize coverage map).
|
||||
# once at the output boundary (single sanitize pass, see process_message).
|
||||
html_media = '\n'.join(content_media)
|
||||
return html_media
|
||||
|
||||
@@ -1098,7 +1114,7 @@ class PostParser:
|
||||
flags_html = self._format_flags(current_flags)
|
||||
content_footer.append('<br>' + flags_html)
|
||||
|
||||
# Not sanitized here — sanitized once at the output boundary (4.4 coverage map):
|
||||
# Not sanitized here — sanitized once at the output boundary:
|
||||
# process_message for single-post/JSON; per-post in _render_pipeline for feeds
|
||||
# (both RSS and HTML).
|
||||
html_footer = '\n'.join(content_footer)
|
||||
@@ -1193,7 +1209,7 @@ class PostParser:
|
||||
parts.append(' | '.join(links))
|
||||
|
||||
# Raw fragment — embedded in the footer, sanitized once at the output
|
||||
# boundary (see the 4.4 sanitize coverage map). Not sanitized here.
|
||||
# boundary (single sanitize pass, see process_message). Not sanitized here.
|
||||
result_html = '<br>'.join(parts) if parts else None
|
||||
return result_html if result_html else None
|
||||
|
||||
|
||||
+12
-34
@@ -19,7 +19,7 @@ from typing import Optional
|
||||
from feedgen.feed import FeedGenerator
|
||||
from pyrogram import errors, Client
|
||||
from pyrogram.types import Message
|
||||
from post_parser import PostParser
|
||||
from post_parser import PostParser, _wrap_post_html
|
||||
from config import get_settings
|
||||
from tg_throttle import tg_rpc_bounded
|
||||
from sanitizer import sanitize_html
|
||||
@@ -175,17 +175,6 @@ def _create_messages_groups(messages: list[Message], group_ids: dict[int, str |
|
||||
|
||||
return processing_groups
|
||||
|
||||
def _trim_messages_groups(messages_groups: list[list[Message]], limit: int):
|
||||
"""
|
||||
Trim messages groups to limit
|
||||
|
||||
Plain synchronous function (contains no await): runs inside the render thread.
|
||||
"""
|
||||
if len(messages_groups) > limit: # Trim groups if they exceed the specified limit
|
||||
messages_groups = messages_groups[:limit]
|
||||
|
||||
return messages_groups
|
||||
|
||||
def _render_messages_groups(messages_groups: list[list[Message]],
|
||||
post_parser: PostParser,
|
||||
exclude_flags: str | None = None,
|
||||
@@ -211,12 +200,8 @@ def _render_messages_groups(messages_groups: list[list[Message]],
|
||||
# pass in _render_pipeline (both RSS and HTML), so each fragment is
|
||||
# sanitized exactly once.
|
||||
message_data = post_parser.process_message(one_message, include_raw=False, sanitize=False)
|
||||
html_parts = [
|
||||
f'<div class="message-body">{message_data["html"]["body"]}</div>',
|
||||
f'<div class="message-footer">{message_data["html"]["footer"]}</div>'
|
||||
]
|
||||
rendered_posts.append({
|
||||
'html': '\n'.join(html_parts),
|
||||
'html': _wrap_post_html(message_data["html"]["body"], message_data["html"]["footer"]),
|
||||
'date': message_data['date'],
|
||||
'message_id': message_data['message_id'],
|
||||
'title': message_data['html']['title'],
|
||||
@@ -255,13 +240,8 @@ def _render_messages_groups(messages_groups: list[list[Message]],
|
||||
# date (registry §3.7), matching a single post of the same message.
|
||||
footer_html = post_parser.generate_html_footer(main_raw, flags_list=merged_flags)
|
||||
|
||||
html_parts = [
|
||||
f'<div class="message-body">{combined_html_body}</div>',
|
||||
f'<div class="message-footer">{footer_html}</div>'
|
||||
]
|
||||
|
||||
rendered_posts.append({
|
||||
'html': '\n'.join(html_parts),
|
||||
'html': _wrap_post_html(combined_html_body, footer_html),
|
||||
'date': main_message['date'],
|
||||
'message_id': main_message['message_id'],
|
||||
'title': main_message['html']['title'],
|
||||
@@ -277,16 +257,13 @@ def _render_messages_groups(messages_groups: list[list[Message]],
|
||||
# Filter posts by exclude_flags
|
||||
if exclude_flags:
|
||||
exclude_flag_list = [flag.strip() for flag in exclude_flags.split(',')] # Split comma-separated flags into list
|
||||
filtered_posts = []
|
||||
for post in rendered_posts:
|
||||
# If "all" is specified and the post has any flags, exclude the post.
|
||||
if "all" in exclude_flag_list and post['flags']:
|
||||
continue
|
||||
# Exclude post if any flag in the exclude list is present in the post's flags.
|
||||
if any(flag in post['flags'] for flag in exclude_flag_list):
|
||||
continue
|
||||
filtered_posts.append(post)
|
||||
rendered_posts = filtered_posts
|
||||
rendered_posts = [
|
||||
post for post in rendered_posts
|
||||
# Keep a post unless "all" is requested and it carries any flag, or any of
|
||||
# its flags appears in the exclude list.
|
||||
if not ("all" in exclude_flag_list and post['flags'])
|
||||
and not any(flag in post['flags'] for flag in exclude_flag_list)
|
||||
]
|
||||
|
||||
# Filter posts by exclude_text
|
||||
if exclude_text:
|
||||
@@ -336,7 +313,8 @@ def _render_pipeline(messages: list[Message],
|
||||
message_groups = _create_messages_groups(messages, group_ids)
|
||||
else:
|
||||
message_groups = _create_messages_groups(messages)
|
||||
message_groups = _trim_messages_groups(message_groups, limit)
|
||||
# Trim groups if they exceed the requested limit (slice is a no-op when shorter).
|
||||
message_groups = message_groups[:limit]
|
||||
posts = _render_messages_groups(message_groups, post_parser, exclude_flags, exclude_text)
|
||||
# Sanitize each surviving (post-filter) post exactly once, here in the worker
|
||||
# thread — no per-post thread hop / per-post CSSSanitizer anymore. Per-post
|
||||
|
||||
@@ -244,7 +244,7 @@ async def test_none_date_post_renders_and_lands_at_feed_end(monkeypatch):
|
||||
@pytest.mark.asyncio
|
||||
async def test_none_date_group_survives_limit_slice(monkeypatch):
|
||||
# §3.12 retention: the group sort key gives None-date groups float('inf') so they sort
|
||||
# as NEWEST and deterministically survive the [:limit] slice in _trim_messages_groups.
|
||||
# as NEWEST and deterministically survive the [:limit] slice in _render_pipeline.
|
||||
# This test applies REAL limit pressure (limit < number of groups) so the slice actually
|
||||
# drops groups — otherwise reverting 'inf' to 0.0 (or any small key) would go unnoticed.
|
||||
# With 5 dated posts + 1 None-date post and limit=3, the surviving 3 groups must be the
|
||||
|
||||
@@ -36,7 +36,6 @@ from rss_generator import (
|
||||
_render_pipeline,
|
||||
_compute_time_based_group_ids,
|
||||
_create_messages_groups,
|
||||
_trim_messages_groups,
|
||||
_render_messages_groups,
|
||||
)
|
||||
|
||||
@@ -91,8 +90,10 @@ def _co_names(func):
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_render_functions_are_sync():
|
||||
# _trim_messages_groups was inlined into _render_pipeline as a `[:limit]` slice
|
||||
# (render-pipeline cosmetics stage); the trimming path is now covered via _render_pipeline.
|
||||
for fn in (_compute_time_based_group_ids, _create_messages_groups,
|
||||
_trim_messages_groups, _render_messages_groups, _render_pipeline):
|
||||
_render_messages_groups, _render_pipeline):
|
||||
assert not asyncio.iscoroutinefunction(fn), f"{fn.__name__} must be a plain sync function"
|
||||
|
||||
|
||||
@@ -154,7 +155,7 @@ def test_render_path_has_no_asyncio_side_effects():
|
||||
banned = {"create_task", "get_running_loop", "to_thread", "ensure_future"}
|
||||
funcs = [
|
||||
_render_pipeline, _compute_time_based_group_ids, _create_messages_groups,
|
||||
_trim_messages_groups, _render_messages_groups,
|
||||
_render_messages_groups,
|
||||
PostParser.process_message, PostParser._generate_html_body,
|
||||
PostParser._generate_html_media, PostParser.generate_html_footer,
|
||||
PostParser._reactions_views_links, PostParser._save_media_file_ids,
|
||||
|
||||
Reference in New Issue
Block a user