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.

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

[BREAKING] TextFilter now requires instantiation #398

Merged
merged 6 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
126 changes: 70 additions & 56 deletions CHANGELOG.md

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ gem "awesome_print"
gem "rubocop"
gem "rubocop-standard"

gem "github_changelog_generator", "~> 1.16"

gem "sorbet-runtime"

group :development, :test do
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ ALLOWLIST = {

pipeline = HTMLPipeline.new \
text_filters: [
HTMLPipeline::MarkdownFilter,
HTMLPipeline::TextFilter::ImageFilter.new,
],
convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new,
sanitization_config: ALLOWLIST
Expand Down Expand Up @@ -199,7 +199,7 @@ the config:
```ruby
pipeline = HTMLPipeline.new \
text_filters: [
HTMLPipeline::MarkdownFilter,
HTMLPipeline::TextFilter::ImageFilter.new,
],
convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new,
sanitization_config: nil
Expand Down
2 changes: 0 additions & 2 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ The following filters were removed:

- `AutolinkFilter`: this is handled by [Commonmarker](https://www.github.com/gjtorikian/commonmarker) and can be disabled/enabled through the `MarkdownFilter`'s `context` hash
- `SanitizationFilter`: this is handled by [Selma](https://www.github.com/gjtorikian/selma); configuration can be done through the `sanitization_config` hash

- `EmailReplyFilter`
- `CamoFilter`
- `TextFilter`

### Changed API

Expand Down
4 changes: 2 additions & 2 deletions lib/html_pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def call(text, context: {}, result: {})

if @text_filters.any?
payload = default_payload({
text_filters: @text_filters.map(&:name),
text_filters: @text_filters.map { |f| f.class.name },
context: context,
result: result,
})
Expand Down Expand Up @@ -204,7 +204,7 @@ def call(text, context: {}, result: {})
# Returns the result of the filter.
def perform_filter(filter, doc, context: {}, result: {})
payload = default_payload({
filter: filter.name,
filter: filter.class.name,
context: context,
result: result,
})
Expand Down
14 changes: 7 additions & 7 deletions lib/html_pipeline/text_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ class HTMLPipeline
class TextFilter < Filter
attr_reader :text

def initialize(text, context: {}, result: {})
raise TypeError, "text must be a String" unless text.is_a?(String)

# Ensure that this is always a string
@text = text.respond_to?(:to_str) ? text.to_str : text.to_s
def initialize(context: {}, result: {})
super(context: context, result: result)
end

class << self
def call(input, context: {}, result: {})
new(input, context: context, result: result).call
def call(text, context: {}, result: {})
raise TypeError, "text must be a String" unless text.is_a?(String)

# Ensure that this is always a string
text = text.respond_to?(:to_str) ? text.to_str : text.to_s
new(context: context, result: result).call(text)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/html_pipeline/text_filter/image_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ class TextFilter
# <img src="http://example.com/test.jpg" alt=""/>.

class ImageFilter < TextFilter
def call
@text.gsub(%r{(https|http)?://.+\.(jpg|jpeg|bmp|gif|png)(\?\S+)?}i) do |match|
def call(text)
text.gsub(%r{(https|http)?://.+\.(jpg|jpeg|bmp|gif|png)(\?\S+)?}i) do |match|
%(<img src="#{match}" alt=""/>)
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/html_pipeline/text_filter/plain_text_input_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ class TextFilter
# Simple filter for plain text input. HTML escapes the text input and wraps it
# in a div.
class PlainTextInputFilter < TextFilter
def call
"<div>#{CGI.escapeHTML(@text)}</div>"
def call(text)
"<div>#{CGI.escapeHTML(text)}</div>"
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/html_pipeline/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

class HTMLPipeline
VERSION = "3.0.3"
VERSION = "3.1.0"
end
3 changes: 0 additions & 3 deletions script/generate_changelog

This file was deleted.

4 changes: 2 additions & 2 deletions test/html_pipeline/convert_filter/markdown_filter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ def setup
"See http://example.org/ for more info"
@code =
"```\n" \
"def hello()" \
" 'world'" \
"def hello() " \
"'world'" \
"end" \
"```"
@header = <<~DOC
Expand Down
26 changes: 21 additions & 5 deletions test/html_pipeline_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
require "test_helper"
require "helpers/mocked_instrumentation_service"
require "html_pipeline/node_filter/image_max_width_filter"
require "html_pipeline/node_filter/mention_filter"
require "html_pipeline/convert_filter/markdown_filter"

class HTMLPipelineTest < Minitest::Test
def setup
@default_context = {}
@pipeline = HTMLPipeline.new(text_filters: [TestTextFilter], default_context: @default_context)
@pipeline = HTMLPipeline.new(text_filters: [TestReverseFilter.new], default_context: @default_context)
end

def test_filter_instrumentation
Expand All @@ -20,7 +22,7 @@ def test_filter_instrumentation

assert(event, "event expected")
assert_equal("call_filter.html_pipeline", event)
assert_equal(TestTextFilter.name, payload[:filter])
assert_equal(TestReverseFilter.name, payload[:filter])
assert_equal(@pipeline.class.name, payload[:pipeline])
assert_equal(body.reverse, payload[:result][:output])
end
Expand All @@ -35,7 +37,8 @@ def test_pipeline_instrumentation

assert(event, "event expected")
assert_equal("call_text_filters.html_pipeline", event)
assert_equal(@pipeline.text_filters.map(&:name), payload[:text_filters])

assert_equal(@pipeline.text_filters.map { |x| x.class.name }, payload[:text_filters])
assert_equal(@pipeline.class.name, payload[:pipeline])
assert_equal(body.reverse, payload[:result][:output])
end
Expand Down Expand Up @@ -73,7 +76,7 @@ def test_setup_instrumentation

def test_incorrect_text_filters
assert_raises(HTMLPipeline::InvalidFilterError) do
HTMLPipeline.new(text_filters: [HTMLPipeline::NodeFilter::MentionFilter], default_context: @default_context)
HTMLPipeline.new(text_filters: [HTMLPipeline::NodeFilter::MentionFilter.new], default_context: @default_context)
end
end

Expand All @@ -86,7 +89,7 @@ def test_incorrect_convert_filter
def test_convert_filter_needed_for_text_and_html_filters
assert_raises(HTMLPipeline::InvalidFilterError) do
HTMLPipeline.new(
text_filters: [TestTextFilter],
text_filters: [TestReverseFilter.new],
node_filters: [
HTMLPipeline::NodeFilter::MentionFilter.new,
],
Expand All @@ -100,4 +103,17 @@ def test_incorrect_node_filters
HTMLPipeline.new(node_filters: [HTMLPipeline::ConvertFilter::MarkdownFilter], default_context: @default_context)
end
end

def test_kitchen_sink
text = "Hey there, @billy. Love to see <marquee>yah</marquee>!"

pipeline = HTMLPipeline.new(
text_filters: [TestReverseFilter.new, YehBolderFilter.new],
convert_filter: HTMLPipeline::ConvertFilter::MarkdownFilter.new,
node_filters: [HTMLPipeline::NodeFilter::MentionFilter.new],
)
result = pipeline.call(text)[:output]

assert_equal("<p>!&gt;eeuqram/eeuqram&lt; ees ot evoL .yllib@ ,ereht <strong>yeH</strong></p>", result)
end
end
15 changes: 10 additions & 5 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,15 @@ module TestHelpers

Minitest::Test.include(TestHelpers)

class TestTextFilter < HTMLPipeline::TextFilter
class << self
def call(input, context: {}, result: {})
input.reverse
end
class TestReverseFilter < HTMLPipeline::TextFilter
def call(input, context: {}, result: {})
input.reverse
end
end

# bolds any instance of the word yeH
class YehBolderFilter < HTMLPipeline::TextFilter
def call(input, context: {}, result: {})
input.gsub("yeH", "**yeH**")
end
end