aem-core-wcm-components icon indicating copy to clipboard operation
aem-core-wcm-components copied to clipboard

[LinkHandler] Refactor / improve `LinkHandler`

Open ky940819 opened this issue 4 years ago • 2 comments

Feature Request

Is your feature request related to a problem? Please describe. This issue is not a bug fix - just optimizations / improvements:

  1. The current LinkHandler model does not implement an interface. #1818 requests that an interface be made public; however, until it is made public a non-exported interface should exist so that when it is time to make the interface public it does not require a major refactoring to accomplish.
  2. The link handler model implementation contains some unnecessary code duplication that can be eliminated.
  3. The link handler model implementation - in some cases - computes fall-back values before knowing that they are required, thus incurring the overhead even when they are not needed.
  4. The link handler model implementation returns the raw type Link where that generic type should be parameterized as Link<Page>. The ambiguity in the API does not provide any benefit and the weakness of the interface (once exposed) will lead to confusion by clients trying to make use of it, and to possible breaking-changes that cannot be expressed via the API.
  5. The test coverage for the LinkHandlerImpl is incomplete - it does not test linking to redirect pages / chains of redirect pages with/without shadowing.
  6. A silent bug exists in the LinkHandlerImpl tests that results in the incorrect resource being injected into the test models - this bug only surfaces when testing testing link redirect shadowing.

Describe the solution you'd like

  1. Create an interface LinkHandler and rename the current link handler model implementation to LinkHandlerImpl. The new interface should not (at this time) be exported from the bundle (see #1818
  2. Refactor LinkHandlerImpl to reduce code duplication
  3. Refactor LinkHanlderImpl to only compute fall-back value if needed.
  4. Change the LinkHandler to return Link<Page> instead of the raw type Link - do not update any exported model interfaces that already use the raw type so that this change is non-breaking.
  5. Improve test coverage prior to any refactoring of the link handler implementation so as to be able to reliably assert no break in functionality due to the refactoring / rewriting.
  6. Correct the testing bug and ensure that the resource injected into the link model is the link resource, and not the page content resource.

Are there alternatives? No

Documentation N/A - no functional change

ky940819 avatar Oct 10 '21 06:10 ky940819

@adobe export issue to Jira project SITES

gabrielwalt avatar Mar 23 '22 10:03 gabrielwalt

:white_check_mark: Jira issue SITES-5541 is successfully created for this GitHub issue.

github-jira-sync-bot avatar Mar 23 '22 10:03 github-jira-sync-bot