Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
discourse/spec/lib/onebox/json_ld_spec.rb

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

63 lines
2.4 KiB
Ruby
Raw Normal View History

Add support for JSON LD in Onebox (#17007) * FIX: Fix a bug that is accessing the values in a hash wrongly and write tests I decided to write tests in order to be confident in my refactor that's in the next commit. Meanwhile I have discovered a potential bug. The `title_attr` key was accessed as a string, but all the keys are actually symbols so it was never evaluated to be true. irb(main):025:0> d = {key: 'value'} => {:key=>"value"} irb(main):026:0> d['key'] => nil irb(main):027:0> d[:key] => "value" * DEV: Extract methods for readability I will be adding a new method following the conventions in place for adding a new normalizer. And this will make the readability of the `raw` block even more difficult; so I am extracting self contained private methods beforehand. * FEATURE: Parse JSON-LD and introduce Movie object JSON LD data is very easily transferable to Ruby objects because they contain types. If these types are mapped to Ruby objects, it is also better to make all the parsed data very explicit and easily extendable. JSON-LD has many more standardized item types, with a full list here: https://schema.org/docs/full.html However in order to decrease the scope, I only adapted the movie type. * DEV: Change inheritance between normalizers Normalizers are not supposed to have an inheritance relationships amongst each other. They are all normalizers, but all normalizing separate protocols. This is why I chose to extract a parent class and relieve Open Graph off that responsibility. Removing the parent class altogether could also a possibility, but I am keeping the scope limited to having a more accurate representation of the normalizers while making it easier to add a new one. * Lint changes * Bring back the Oembed OpenGraph inheritance There is one test that caught that this inheritance was necessary. I still think modelling wise this inheritance shouldn't exist, but this can be tackled separately. * Return empty hash if the json received is invalid Before this change if there was a parsing error with JSON it would throw an exception. The goal of this commit is to rescue that exception and then log a warning. I chose to use Discourse's logger wrapper `warn_exception` to have the backtrace and not just used Rails logger. I considered raising an `InvalidParameters` error however if the JSON here is invalid it should not block showing of the Onebox, so logging is enough. * Prep to support more JSONLD schema types with case * Extract mustache template object created from JSONLD
2022-06-13 11:32:34 -04:00
# frozen_string_literal: true
require 'onebox/json_ld'
describe Onebox::JsonLd do
it 'logs warning and returns an empty hash if received json is invalid' do
invalid_json = "{\"@type\":invalid-json}"
doc = Nokogiri::HTML("<script type=\"application/ld+json\">#{invalid_json}</script>")
Discourse.expects(:warn_exception).with(
FIX: Handle sites with more than 1 JSON-LD element (#17095) A followup to #17007
2022-06-14 20:55:55 -04:00
instance_of(JSON::ParserError), { message: "Error parsing JSON-LD: #{invalid_json}" }
Add support for JSON LD in Onebox (#17007) * FIX: Fix a bug that is accessing the values in a hash wrongly and write tests I decided to write tests in order to be confident in my refactor that's in the next commit. Meanwhile I have discovered a potential bug. The `title_attr` key was accessed as a string, but all the keys are actually symbols so it was never evaluated to be true. irb(main):025:0> d = {key: 'value'} => {:key=>"value"} irb(main):026:0> d['key'] => nil irb(main):027:0> d[:key] => "value" * DEV: Extract methods for readability I will be adding a new method following the conventions in place for adding a new normalizer. And this will make the readability of the `raw` block even more difficult; so I am extracting self contained private methods beforehand. * FEATURE: Parse JSON-LD and introduce Movie object JSON LD data is very easily transferable to Ruby objects because they contain types. If these types are mapped to Ruby objects, it is also better to make all the parsed data very explicit and easily extendable. JSON-LD has many more standardized item types, with a full list here: https://schema.org/docs/full.html However in order to decrease the scope, I only adapted the movie type. * DEV: Change inheritance between normalizers Normalizers are not supposed to have an inheritance relationships amongst each other. They are all normalizers, but all normalizing separate protocols. This is why I chose to extract a parent class and relieve Open Graph off that responsibility. Removing the parent class altogether could also a possibility, but I am keeping the scope limited to having a more accurate representation of the normalizers while making it easier to add a new one. * Lint changes * Bring back the Oembed OpenGraph inheritance There is one test that caught that this inheritance was necessary. I still think modelling wise this inheritance shouldn't exist, but this can be tackled separately. * Return empty hash if the json received is invalid Before this change if there was a parsing error with JSON it would throw an exception. The goal of this commit is to rescue that exception and then log a warning. I chose to use Discourse's logger wrapper `warn_exception` to have the backtrace and not just used Rails logger. I considered raising an `InvalidParameters` error however if the JSON here is invalid it should not block showing of the Onebox, so logging is enough. * Prep to support more JSONLD schema types with case * Extract mustache template object created from JSONLD
2022-06-13 11:32:34 -04:00
)
json_ld = described_class.new(doc)
expect(json_ld.data).to eq({})
end
FIX: Handle sites with more than 1 JSON-LD element (#17095) A followup to #17007
2022-06-14 20:55:55 -04:00
it 'returns an empty hash if there is no JSON-LD script tag' do
Add support for JSON LD in Onebox (#17007) * FIX: Fix a bug that is accessing the values in a hash wrongly and write tests I decided to write tests in order to be confident in my refactor that's in the next commit. Meanwhile I have discovered a potential bug. The `title_attr` key was accessed as a string, but all the keys are actually symbols so it was never evaluated to be true. irb(main):025:0> d = {key: 'value'} => {:key=>"value"} irb(main):026:0> d['key'] => nil irb(main):027:0> d[:key] => "value" * DEV: Extract methods for readability I will be adding a new method following the conventions in place for adding a new normalizer. And this will make the readability of the `raw` block even more difficult; so I am extracting self contained private methods beforehand. * FEATURE: Parse JSON-LD and introduce Movie object JSON LD data is very easily transferable to Ruby objects because they contain types. If these types are mapped to Ruby objects, it is also better to make all the parsed data very explicit and easily extendable. JSON-LD has many more standardized item types, with a full list here: https://schema.org/docs/full.html However in order to decrease the scope, I only adapted the movie type. * DEV: Change inheritance between normalizers Normalizers are not supposed to have an inheritance relationships amongst each other. They are all normalizers, but all normalizing separate protocols. This is why I chose to extract a parent class and relieve Open Graph off that responsibility. Removing the parent class altogether could also a possibility, but I am keeping the scope limited to having a more accurate representation of the normalizers while making it easier to add a new one. * Lint changes * Bring back the Oembed OpenGraph inheritance There is one test that caught that this inheritance was necessary. I still think modelling wise this inheritance shouldn't exist, but this can be tackled separately. * Return empty hash if the json received is invalid Before this change if there was a parsing error with JSON it would throw an exception. The goal of this commit is to rescue that exception and then log a warning. I chose to use Discourse's logger wrapper `warn_exception` to have the backtrace and not just used Rails logger. I considered raising an `InvalidParameters` error however if the JSON here is invalid it should not block showing of the Onebox, so logging is enough. * Prep to support more JSONLD schema types with case * Extract mustache template object created from JSONLD
2022-06-13 11:32:34 -04:00
doc = Nokogiri::HTML("<script type=\"something else\"></script>")
json_ld = described_class.new(doc)
expect(json_ld.data).to eq({})
end
FIX: Handle sites with more than 1 JSON-LD element (#17095) A followup to #17007
2022-06-14 20:55:55 -04:00
it 'returns an empty hash if there is no JSON-LD data' do
Add support for JSON LD in Onebox (#17007) * FIX: Fix a bug that is accessing the values in a hash wrongly and write tests I decided to write tests in order to be confident in my refactor that's in the next commit. Meanwhile I have discovered a potential bug. The `title_attr` key was accessed as a string, but all the keys are actually symbols so it was never evaluated to be true. irb(main):025:0> d = {key: 'value'} => {:key=>"value"} irb(main):026:0> d['key'] => nil irb(main):027:0> d[:key] => "value" * DEV: Extract methods for readability I will be adding a new method following the conventions in place for adding a new normalizer. And this will make the readability of the `raw` block even more difficult; so I am extracting self contained private methods beforehand. * FEATURE: Parse JSON-LD and introduce Movie object JSON LD data is very easily transferable to Ruby objects because they contain types. If these types are mapped to Ruby objects, it is also better to make all the parsed data very explicit and easily extendable. JSON-LD has many more standardized item types, with a full list here: https://schema.org/docs/full.html However in order to decrease the scope, I only adapted the movie type. * DEV: Change inheritance between normalizers Normalizers are not supposed to have an inheritance relationships amongst each other. They are all normalizers, but all normalizing separate protocols. This is why I chose to extract a parent class and relieve Open Graph off that responsibility. Removing the parent class altogether could also a possibility, but I am keeping the scope limited to having a more accurate representation of the normalizers while making it easier to add a new one. * Lint changes * Bring back the Oembed OpenGraph inheritance There is one test that caught that this inheritance was necessary. I still think modelling wise this inheritance shouldn't exist, but this can be tackled separately. * Return empty hash if the json received is invalid Before this change if there was a parsing error with JSON it would throw an exception. The goal of this commit is to rescue that exception and then log a warning. I chose to use Discourse's logger wrapper `warn_exception` to have the backtrace and not just used Rails logger. I considered raising an `InvalidParameters` error however if the JSON here is invalid it should not block showing of the Onebox, so logging is enough. * Prep to support more JSONLD schema types with case * Extract mustache template object created from JSONLD
2022-06-13 11:32:34 -04:00
doc = Nokogiri::HTML("<script type=\"application/ld+json\"></script>")
json_ld = described_class.new(doc)
expect(json_ld.data).to eq({})
end
FIX: Handle sites with more than 1 JSON-LD element (#17095) A followup to #17007
2022-06-14 20:55:55 -04:00
it 'returns an empty hash if the type of JSON-LD data is not Movie' do
Add support for JSON LD in Onebox (#17007) * FIX: Fix a bug that is accessing the values in a hash wrongly and write tests I decided to write tests in order to be confident in my refactor that's in the next commit. Meanwhile I have discovered a potential bug. The `title_attr` key was accessed as a string, but all the keys are actually symbols so it was never evaluated to be true. irb(main):025:0> d = {key: 'value'} => {:key=>"value"} irb(main):026:0> d['key'] => nil irb(main):027:0> d[:key] => "value" * DEV: Extract methods for readability I will be adding a new method following the conventions in place for adding a new normalizer. And this will make the readability of the `raw` block even more difficult; so I am extracting self contained private methods beforehand. * FEATURE: Parse JSON-LD and introduce Movie object JSON LD data is very easily transferable to Ruby objects because they contain types. If these types are mapped to Ruby objects, it is also better to make all the parsed data very explicit and easily extendable. JSON-LD has many more standardized item types, with a full list here: https://schema.org/docs/full.html However in order to decrease the scope, I only adapted the movie type. * DEV: Change inheritance between normalizers Normalizers are not supposed to have an inheritance relationships amongst each other. They are all normalizers, but all normalizing separate protocols. This is why I chose to extract a parent class and relieve Open Graph off that responsibility. Removing the parent class altogether could also a possibility, but I am keeping the scope limited to having a more accurate representation of the normalizers while making it easier to add a new one. * Lint changes * Bring back the Oembed OpenGraph inheritance There is one test that caught that this inheritance was necessary. I still think modelling wise this inheritance shouldn't exist, but this can be tackled separately. * Return empty hash if the json received is invalid Before this change if there was a parsing error with JSON it would throw an exception. The goal of this commit is to rescue that exception and then log a warning. I chose to use Discourse's logger wrapper `warn_exception` to have the backtrace and not just used Rails logger. I considered raising an `InvalidParameters` error however if the JSON here is invalid it should not block showing of the Onebox, so logging is enough. * Prep to support more JSONLD schema types with case * Extract mustache template object created from JSONLD
2022-06-13 11:32:34 -04:00
doc = Nokogiri::HTML("<script type=\"application/ld+json\">{\"@type\":\"Something Else\",\"aggregateRating\":{\"@type\":\"AggregateRating\",\"ratingCount\":806928,\"bestRating\":10,\"worstRating\":1}}</script>")
json_ld = described_class.new(doc)
expect(json_ld.data).to eq({})
end
it 'correctly normalizes the properties' do
doc = Nokogiri::HTML(onebox_response('imdb'))
json_ld = described_class.new(doc)
expect(json_ld.data).to eq(expected_movie_hash)
end
FIX: Handle sites with more than 1 JSON-LD element (#17095) A followup to #17007
2022-06-14 20:55:55 -04:00
it 'does not fail when there is more than one JSON-LD element' do
doc = Nokogiri::HTML(onebox_response('imdb'))
doc.css("body")[0] << "<script type=\"application/ld+json\">{\"@context\":\"http://schema.org\",\"@type\":\"WebPage\",\"url\":\"https:\/\/imdb.com\",\"description\":\"Movies\"}</script>"
Discourse.expects(:warn_exception).never
json_ld = described_class.new(doc)
expect(json_ld.data).to eq(expected_movie_hash)
end
Add support for JSON LD in Onebox (#17007) * FIX: Fix a bug that is accessing the values in a hash wrongly and write tests I decided to write tests in order to be confident in my refactor that's in the next commit. Meanwhile I have discovered a potential bug. The `title_attr` key was accessed as a string, but all the keys are actually symbols so it was never evaluated to be true. irb(main):025:0> d = {key: 'value'} => {:key=>"value"} irb(main):026:0> d['key'] => nil irb(main):027:0> d[:key] => "value" * DEV: Extract methods for readability I will be adding a new method following the conventions in place for adding a new normalizer. And this will make the readability of the `raw` block even more difficult; so I am extracting self contained private methods beforehand. * FEATURE: Parse JSON-LD and introduce Movie object JSON LD data is very easily transferable to Ruby objects because they contain types. If these types are mapped to Ruby objects, it is also better to make all the parsed data very explicit and easily extendable. JSON-LD has many more standardized item types, with a full list here: https://schema.org/docs/full.html However in order to decrease the scope, I only adapted the movie type. * DEV: Change inheritance between normalizers Normalizers are not supposed to have an inheritance relationships amongst each other. They are all normalizers, but all normalizing separate protocols. This is why I chose to extract a parent class and relieve Open Graph off that responsibility. Removing the parent class altogether could also a possibility, but I am keeping the scope limited to having a more accurate representation of the normalizers while making it easier to add a new one. * Lint changes * Bring back the Oembed OpenGraph inheritance There is one test that caught that this inheritance was necessary. I still think modelling wise this inheritance shouldn't exist, but this can be tackled separately. * Return empty hash if the json received is invalid Before this change if there was a parsing error with JSON it would throw an exception. The goal of this commit is to rescue that exception and then log a warning. I chose to use Discourse's logger wrapper `warn_exception` to have the backtrace and not just used Rails logger. I considered raising an `InvalidParameters` error however if the JSON here is invalid it should not block showing of the Onebox, so logging is enough. * Prep to support more JSONLD schema types with case * Extract mustache template object created from JSONLD
2022-06-13 11:32:34 -04:00
private
def expected_movie_hash
{
name: 'Rudy',
image: 'https://m.media-amazon.com/images/M/MV5BZGUzMDU1YmQtMzBkOS00MTNmLTg5ZDQtZjY5Njk4Njk2MmRlXkEyXkFqcGdeQXVyNjc1NTYyMjg@._V1_.jpg',
description: 'Rudy has always been told that he was too small to play college football. But he is determined to overcome the odds and fulfill his dream of playing for Notre Dame.',
rating: 7.5,
genres: ['Biography', 'Drama', 'Sport'],
duration: '01:54'
}
end
end