[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: PNG File failbackIsNotConfigured.png     PNG File listOfDomains.png     PNG File problemTab.png    
Issue Links:
Relates
relates to MAGNOLIA-7083 Backport sanity-checks against NPEs t... Closed
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 fixing

Given 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
It redirects to http://travel.de:8085/de/index.html

But the Problems tab shows "config is incomplete and mapping is disabled" (?!)

The same warning turns up with old HostBasedVirtualURIMapping

Reason

DefaultVirtualUriMapping#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:

  • VirtualUriFilter gets all definitions from the registry
  • AbstractRegistry#getAllDefinitions filters all invalid definitions
  • VirtualUriRegistry.VirtualUriMappingProvider#isValid checks if the mapping is valid
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.
Now you either configure the fallback, which you might not know at the time what it should be for when there's no host configured, or the mapping is disabled completely.
Simplest example of it not working is that if you try to login to corp web currently, you end up w/ blank page, instead of host mapping falling through and (on author) "default" mapping being resolved instead pointing you to login page.
the whole isValid() check is fine for simple mappings, but for more complex ones, they need the choice of optionally firing or "skipping".

Comment by Hieu Nguyen Duc [ 01/Nov/17 ]

mgeljic
AbstractRegistry#getAllDefintions is to filter out that invalid provider but the provider returned for host-based mapping is YamlDeefinitionProvider which calls AbstractDefinitionProviderWrapper#isValid and delegates to DefintiionProviderBuilder#isValid and returns true.

Do we expect it is VirtualUriMappingProvider?

Comment by Mikaël Geljić [ 01/Nov/17 ]

Thanks, that explains it . Yeah I was expecting the VirtualUriMappingProvider there too.
Either we can fix it, or alternatively we add the filter VirtualUriMapping::isValid to the eval in VirtualUriFilter.

Comment by Sang Ngo Huu [ 13/Nov/17 ]

mgeljic There are 2 concerns I have when doing QA for this ticket

  • I haven't got any warnings if I don't configure fallback toUri
  • I have usecase: I have a list of domains, and I want to map them with a resource fromUri & toUri, I have to configure domain by domain, or at least one domain have toUri (to make it valid) and fallback toUri ?

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 ]
  • Re: host-mapping missing toUri: this mapping is valid (i.e. can be evaluated) if at least one of the host-mappings is valid.
    • to output a message, we would need a way to log problems without disabling the URI mapping.
    • not so obvious where to do this, as the mapping is not aware of its provider (rather the opposite).
  • Re: multiple host-mappings pointing to the same toUri; either they all point to the same and no need for a host-based one;
  • or the only way is to have one entry per host, with repeated toUri throughout them.

Both of these would be worth improvement tickets; nothing critical though

Generated at Mon Feb 12 04:21:16 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.