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

Fix URL formation on Windows and URL symbols in custom ID breaking cache storage #134

Merged
merged 9 commits into from Jul 7, 2017

Conversation

@georgiwe
Copy link
Contributor

commented Jul 7, 2017

Description

This PR should fix MLIBZ-1919 and MLIBZ-1677.

Changes

Parse path segments for cache requests manually and remove library from dependencies. Add library for joining URLs and ensure paths don't start with multiple slashes, because that broke the assertions in the tests due to discrepancies between mocked URLs and requested URLs.

Tests

Added new test for "." in _id. Old tests pass.

@thomasconner Other special characters seem to fail. If you want to release today, you might take a look. Otherwise I will work it out on Monday.

georgiwe added 3 commits Jul 7, 2017
MLIBZ-1953 Manually extract data for cache request from URL, instead …
…of using the library "url-pattern". Remove it from package.json, since it's not used anywhere else. Library did not match any parts of the URL when a custom ID was used, which contained any of the characters, which had semantic meaning for a URL (., :, etc).
MLIBZ-1677 Use specialized lib to join URL segments instead of path.j…
…oin and ensure empty strings or slashes are not the first argument of url-join, because it doesn't handle it well (inserts more than one slash).

@georgiwe georgiwe self-assigned this Jul 7, 2017

@georgiwe georgiwe requested review from thomasconner and tejasranade Jul 7, 2017

@codecov-io

This comment has been minimized.

Copy link

commented Jul 7, 2017

Codecov Report

Merging #134 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #134      +/-   ##
=========================================
+ Coverage   90.45%   90.5%   +0.04%     
=========================================
  Files          77      77              
  Lines        6421    6432      +11     
  Branches     1035    1034       -1     
=========================================
+ Hits         5808    5821      +13     
+ Misses        613     611       -2
Impacted Files Coverage Δ
src/request/src/cache.js 99.45% <100%> (+1.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1107bc1...0ab0e43. Read the comment docs.

@thomasconner

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2017

Please see latest commits that allow all special characters.

@thomasconner thomasconner merged commit 845e7d9 into master Jul 7, 2017

1 of 2 checks passed

codeclimate 3 new issues
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@thomasconner thomasconner deleted the dot-in-id branch Jul 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.