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

SLING-9174: Added the code for enhancement #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mittalxkunal
Copy link

@@ -166,8 +167,22 @@ private String getUrl(Configuration config, SlingHttpServletRequest request) {
private String buildUrl(Configuration config, SlingHttpServletRequest request) {
final Resource resource = request.getResource();

// The below code gets the path to the XF and then passes it to the buildUrl method
// so that the path to the component is replaced with path to the XF
ValueMap vm = resource.adaptTo(ValueMap.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would base the code on the resource type rather than the existence of a property.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check for resource type because its already checked before it comes to buildUrl method based on the configuration added in resource-types OSGi config

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - but this code is going to run for all configured resource types.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So within the config you can configure multiple resource types and then enable option to rewrite content path instead of component path. So this will be enabled for all configured resource types...

If there was a 1:1 mapping then we would verify for which resourcetype path rewrite is enabled... Does that make sense?

@sonarcloud
Copy link

sonarcloud bot commented Mar 7, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 3 Code Smells

45.2% 45.2% Coverage
0.0% 0.0% Duplication

@rombert
Copy link
Contributor

rombert commented Apr 7, 2020

@toniedzwiedz - would be great to have your input as well

@sonarcloud
Copy link

sonarcloud bot commented Aug 10, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 3 Code Smells

45.2% 45.2% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_252) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@sonarcloud
Copy link

sonarcloud bot commented Jan 16, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants