Два открытых PR предлагают разные подходы к редактированию атрибутов BBCode в сыром тексте поста:
-
#43187 — Add recurring Event successor topic mode представляет
BbcodeRewriter, который идентифицирует отрендеренное событие (Event) и безопасно переписывает соответствующий открывающий тег BBCode во время автоматического переноса (rollover). -
#42932 — Add Event actions to workflows представляет
PrettyText.update_bbcode_attributes, который находит разобранный блок BBCode и обновляет его атрибуты, когда workflow открывает или закрывает событие.
Во время ревью #43187 Martin предложил выделить общую функциональность манипуляции с BBCode из сервиса, специфичного для событий.
Изначально я включил некоторую связанную функциональность на основе токенов из другого PR, но это привело к появлению двух механизмов переписывания внутри #43187. Поскольку API на основе токенов не имело вызовов в production-окружении в той ветке, я удалил его в коммите 6d13371.
API на основе токенов остается независимо полезным для #42932.
Я задумался, следует ли этим двум подходам в конечном итоге сойтись в одном переиспользуемом базовом API, или их разные требования оправдывают сохранение обоих.
Сравнение текущих реализаций
Таблица описывает то, что код реализует в настоящее время, а не предполагает, что какой-либо из подходов является inherently неправильным.
| Возможность | BbcodeRewriter (#43187) |
API на основе токенов (#42932) | Смысл и важность |
|---|---|---|---|
| 1. Использует парсер BBCode Discourse | Сканирует кандидатами открывающие теги, используя parseBBCodeTag, ту же грамматику открывающих тегов, что используется движком Markdown. |
Использует DiscourseMarkdownIt.parse для получения Markdown-токенов и поиска соответствующего блока BBCode. |
Оба подхода понимают синтаксис BBCode, а не полагаются исключительно на независимое регулярное выражение. Они различаются способом идентификации тега для редактирования. |
| 2. Выбор целевого блока BBCode | Временно помечает кандидатами открывающие теги, рендерит пост и идентифицирует кандидата, соответствующий выбранному HTML-элементу, например div.discourse-post-event. |
Выбирает первый разобранный токен BBCode, чей meta.bbcode совпадает с запрашиваемым именем тега. |
Идентичность цели имеет значение. #43187 необходимо связать сырой BBCode с событием, которое Discourse фактически отрендерил. Workflow в #42932 использует первый подходящий разобранный блок события. |
| 3. Обработка отсутствующих или неоднозначных целей | Вызывает TargetNotFound, AmbiguousTarget или UnsupportedRenderer, если цель не может быть надежно идентифицирована. |
Возвращает null, если не существует подходящего разобранного блока; иначе выбирает первое совпадение. |
Поведение при ошибке отличается. #43187 намеренно отказывается от неоднозначных автоматических правок. Выбор первого разобранного блока может быть достаточным, если вызывающая сторона ожидает только один релевантный блок. |
| 4. Проверка соответствия «сырой текст — отрендеренный результат» | Рендерит размеченный контент, проверяет маркер на целевом HTML-элементе и подтверждает, что вставка и удаление маркера не изменяют отрендеренный HTML. | Использует метаданные токенов парсера Markdown для локализации открывающего тега без дополнительной проверки идентичности отрендеренного HTML. | Это защищает от редактирования неверной отрендеренной цели. Это особенно актуально, когда фоновая задача автоматически архивирует существующее событие, но требует дополнительного рендеринга и сложности. |
| 5. Проверка выбранного открывающего тега | Перед переписыванием повторно сканирует предоставленный сырой текст и проверяет, что выбранный кандидат все еще соответствует ожидаемому тексту открывающего тега. | Локализирует соответствующий открывающий тег во время вызова обновления без отдельного сравнения с ожидаемым открывающим текстом. | Это внутренняя проверка согласованности. Она помогает избежать переписывания кандидата, который больше не соответствует предыдущему выбору. Сама по себе она не является защитой от конкурентных правок базы данных. |
| 6. Обработка имен атрибутов и существующих значений | Переписывает выбранные атрибуты, используя границы атрибутов парсера. Также нормализует эквивалентные имена отрендеренных атрибутов, позволяя последовательно удалять варианты, такие как Recurrence и recurrence. |
Использует совпадения токенов атрибутов для замены или удаления запрашиваемых имен и общий сериализатор атрибутов BBCode для вставки значений. | Оба стремятся сохранить нерелевантные атрибуты. Нормализация эквивалентных имен особенно важна при архивировании события, так как оставление псевдонима повторения могло бы привести к тому, что завершенное событие останется повторяющимся. |
| 7. Валидация переписанного тега | Снова разбирает переписанный открывающий тег и проверяет, что его атрибуты соответствуют ожидаемым значениям. Недействительный round-trip вызывает ошибку. | Напрямую создает переписанный открывающий тег, без эквивалентного шага финального разбора и сравнения. | Валидация round-trip выявляет проблемы сериализации, такие как измененные значения атрибутов или некорректные границы. Подход на основе токенов проще, но полагается на свою логику переписывания и тесты, а не на эту дополнительную проверку времени выполнения. |
| 8. Сохранение нерелевантного контента поста | Изменяет выбранный открывающий тег, сохраняя окружающий сырой контент. Также проверяет, что нерелевантные значения атрибутов выживают после переписывания. | Заменяет подстроку выбранного открывающего тега, сохраняя окружающий сырой контент и атрибуты, которые не были запрошены для изменения. | Оба должны избегать непреднамеренных изменений поста. Редакция, связанная с событием, не должна незаметно изменять окружающую дискуссию или нерелевантные атрибуты BBCode. |
| 9. Стоимость реализации | Требует сканирования кандидатов, временных маркеров, рендеринга поста, инспекции HTML, переписывания атрибутов и валидации. | Разбирает Markdown-токены, выбирает первый подходящий открывающий тег и напрямую переписывает его. | Более строгие проверки времени выполнения имеют свою цену. Вопрос заключается в том, должны ли дополнительные меры защиты быть доступны всем вызывающим сторонам, быть опциональными или реализовываться только там, где приложение этого требует. |
Почему требования различаются
Для #43187 перенос наследника запускается автоматически, когда завершается конкретное вхождение. Исходное событие архивируется как неповторяющееся вхождение, и создается новая тема для следующего вхождения.
Если будет изменен неверный блок [event] или переписанные атрибуты больше не представляют предполагаемое событие, ошибка могла бы сохраняться между попытками автоматического переноса.
Для #42932 действие workflow явно запрашивает открытие или закрытие события. Его текущая реализация на основе токенов изменяет первый подходящий разобранный блок события.
Таким образом, подход на основе токенов не обязательно не подходит для этого случая использования. Он просто предоставляет другие гарантии выбора и валидации.
Оба подхода уже разделяют некоторую базовую инфраструктуру, включая parseBBCodeTag и serializeBBCodeAttr.
Возможное направление для общей абстракции
Одна из возможностей — разделить три ответственности:
-
Поиск открывающего тега BBCode — с использованием токенов парсера, явного выбора кандидатов или отображения отрендеренного элемента.
-
Переписывание его атрибутов — с использованием одной общей реализации для границ атрибутов, сериализации и сохранения нерелевантных значений.
-
Валидация результата — опциональное принуждение к идентичности цели, обнаружению неоднозначности, эквивалентности отрендеренного контента или проверкам round-trip атрибутов, в зависимости от вызывающей стороны.
Это позволило бы более простым workflow использовать простой выбор токенов, тогда как более чувствительные операции автоматического редактирования могли бы запрашивать более строгие меры защиты.
Однако это также могло бы внести ненужную сложность по сравнению с поддержанием двух небольших API.
Вопрос для мейнтейнеров
Было бы предпочтительнее, чтобы два PR сохранили свои отдельные API переписывания, учитывая их разные требования к выбору цели?
Или, возможно, общим нижнеуровневым переписывателем атрибутов с настраиваемым выбором цели и валидацией было бы лучшим долгосрочным направлением?
Меня особенно интересует, считаются ли дополнительные проверки безопасности, в настоящее время реализованные в BbcodeRewriter, общепригодной базовой функциональностью, или они должны оставаться специфичными для случаев использования, требующих более сильных гарантий.
Я не предлагаю, чтобы один из PR блокировал другой; цель состоит в том, чтобы избежать ненужного дублирования в базовой инфраструктуре BBCode Discourse по мере развития обеих функций.