Improve MathMan url construction

Fixes: CNVS-38278

Originally we were using Addressable::URI#merge to build up the url,
merge in Addressable isn't like merge in most URL parsers, instead of
handling merging the path component it just clobbers it. Instead, we
want to use #join which exhibits the behavior we want.

Test Plan:
* Configure Math Man to use a URL with a path component
* Rendering equations should work

Change-Id: I6f62daaf4dc2a36e39c8f00d2a974c3ba4032c3f
Reviewed-on: https://gerrit.instructure.com/119741
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Tucker McKnight <tmcknight@instructure.com>
Product-Review: Tyler Pickett <tpickett@instructure.com>
This commit is contained in:
Tyler Pickett 2017-07-19 15:55:32 -05:00
parent 7c7c827f8b
commit 9493ad0fa8
2 changed files with 12 additions and 3 deletions

View File

@ -19,7 +19,7 @@ require "addressable/uri"
module MathMan
def self.url_for(latex:, target:)
uri = base_url.merge(path: "/#{target}")
uri = base_url.join(target.to_s)
uri.query = "tex=#{latex}"
uri.to_s
end
@ -45,7 +45,9 @@ module MathMan
def base_url
with_plugin_settings do |plugin_settings|
Addressable::URI.parse(plugin_settings[:base_url])
Addressable::URI.parse(plugin_settings[:base_url]).tap do |uri|
uri.path << '/' unless uri.path.end_with?('/')
end
end
end

View File

@ -21,7 +21,8 @@ describe MathMan do
let(:latex) do
'\sqrt{25}+12^{12}'
end
let(:service_url) { 'http://www.mml-service.com' }
# we explicitly don't want a trailing slash here for the url tests
let(:service_url) { 'http://www.mml-service.com/beta' }
let(:use_for_mml) { false }
let(:use_for_svg) { false }
@ -41,6 +42,12 @@ describe MathMan do
end
describe '.url_for' do
it 'must retain the path from base_url setting' do
url = MathMan.url_for(latex: latex, target: :mml)
parsed = Addressable::URI.parse(url)
expect(parsed.path).to eq ('/beta/mml')
end
it 'should include target string in generated url' do
expect(MathMan.url_for(latex: latex, target: :mml)).to match(/mml/)
expect(MathMan.url_for(latex: latex, target: :svg)).to match(/svg/)