[MAGNOLIA-7158] Host-based mappings require a fallback toUri Created: 25/Sep/17 Updated: 13/Dec/17 Resolved: 08/Nov/17 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | Virtual URI mappings |
| Affects Version/s: | 5.5.6 |
| Fix Version/s: | 5.5.8, 5.6 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Mikaël Geljić | Assignee: | Hieu Nguyen Duc |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 2d 0.5h | ||
| Original Estimate: | 1d | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Acceptance criteria: |
Empty
|
||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||
| Date of First Response: | |||||||||
| Epic Link: | Virtual URI mappings | ||||||||
| Sprint: | Saigon 120, Saigon 121 | ||||||||
| Story Points: | 2 | ||||||||
| Description |
|
Since the Virtual URI mapping API extraction/revamp, HostBasedVirtualUriMapping now requires the toUri property to be configured, in order for the mapping to be considered "valid", and effectively work (same goes for the regex sub-class). Was not the case before we introduced the sanity-checks (#isValid method), which was also ported to the old/deprecated mappings. DefaultVirtualUriMapping mandates having both fromUri and toUri configured; this should be overridden for the host-based mappings, so that only fromUri is mandatory, and toUri is left optional. |
| Comments |
| Comment by Hieu Nguyen Duc [ 31/Oct/17 ] |
Current status before fixingGiven this config without fallback toUri
class: info.magnolia.virtualuri.mapping.HostBasedVirtualUriMapping
fromUri: /go
mappings:
de:
host: travel.de
toUri: redirect:/de/index.html
Enter http://travel.de:8085/go But the Problems tab shows "config is incomplete and mapping is disabled" (?!) The same warning turns up with old HostBasedVirtualURIMapping ReasonDefaultVirtualUriMapping#isValid requires toUri but HostBasedVirtualUriMapping#tryToMapHost does not (line 117). |
| Comment by Mikaël Geljić [ 31/Oct/17 ] |
|
Can you explain how come it works at all? —so far, if I follow the chain of evaluation:
|
| Comment by Jan Haderka [ 31/Oct/17 ] |
|
hieu.nguyen Exactly. Before enforcing the check, if toURI was left empty, this virtual mapping would return results for cases where target for hosts was configured, and "null" for the fallback. The "null" result made "VirtualURIManager" skip the mapping in such case and continue with other mappings until if found some that works. |
| Comment by Hieu Nguyen Duc [ 01/Nov/17 ] |
|
mgeljic Do we expect it is VirtualUriMappingProvider? |
| Comment by Mikaël Geljić [ 01/Nov/17 ] |
|
Thanks, that explains it |
| Comment by Sang Ngo Huu [ 13/Nov/17 ] |
|
mgeljic There are 2 concerns I have when doing QA for this ticket
I think one of them is mandatory, If it is not configured in VirturalUriMapping it has to be configured in HostUriMapping and otherwise. toUri in HostUriMapping overrides the value in VirtualUriMapping. |
| Comment by Mikaël Geljić [ 13/Nov/17 ] |
Both of these would be worth improvement tickets; nothing critical though |