LPP-26493 #361

ericyanLr · 2017-08-24T02:22:08Z

Main Issue

Currently, canonical URLs do not account for localization. This means that every page, regardless of which locale you are viewing it from, currently uses the same canonical URL.

For example, for the URL: http://localhost:8080
The canonical URL is http://localhost:8080

For a localized example, for the URL: http://localhost:8080/fr/
The canonical URL is currently also http://localhost:8080

This is an issue, since http://localhost:8080 and http://localhost:8080/fr/ should each have it's own canonical url.

The expected canonical URL for this case should be: http://localhost:8080/fr/

Note: Below will be a section with some example scenarios to further describe expected results related to this issue

Second Issue

Currently, alternate URLs are not being generated properly in several cases.
There was an attempt to resolve this for certain cases in LPS-72431.

Even then, the changes introduced did not work properly at times.
This can be seen in a recent ticket: LPS-74272

The changes introduced in this pr should address generating the proper alternate URLs for the two tickets mentioned above as well.

In addition, this pr will also handle providing the proper url for the x-default hreflang tag.

Note: Below will be a section with some example scenarios to further describe expected results related to this issue

The fixes for this pr also accounts for other factors, such as:

The portal property locale.prepend.friendly.url.style=2
- The examples provided mainly focus on the overall changes.
portal.proxy.path

Please let me know if you have any questions.
Thanks!

A Few Example Scenarios - Main Issue

To better describe the goals of the fix regarding canonical URLs, here are expected results from some scenarios.

Default Site (using default locale - en_US)

URL: http://localhost:8080
Expected Canonical URL: http://localhost:8080

URL: http://localhost:8080/en_US/
Expected Canonical URL: http://localhost:8080

URL: http://localhost:8080/web/guest/home
Expected Canonical URL: http://localhost:8080

Default Site (using non-default locale - fr)

URL: http://localhost:8080/fr/
Expected Canonical URL: http://localhost:8080/fr/

URL: http://localhost:8080/fr/web/guest/home
Expected Canonical URL: http://localhost:8080/fr/

Created site called testsite with second created public page testpage2 (using default locale - en_US)

URL: http://localhost:8080/web/testsite/testpage2
Expected Canonical URL: http://localhost:8080/web/testsite/testpage2

URL: http://localhost:8080/en_US/web/testsite/testpage2
Expected Canonical URL: http://localhost:8080/web/testsite/testpage2

Created site called testsite with second created public page testpage2 (using non-default locale - fr)

URL: http://localhost:8080/fr/web/testsite/testpage2
Expected Canonical URL: http://localhost:8080/fr/web/testsite/testpage2

A Few Example Scenarios - Second Issue

To better describe the goals of the fix regarding alternate URLs, here are expected results from some scenarios. For these examples, we will only include a snippet of the hreflang tags to reduce the amount of space used.

Default Site (using default locale - en_US)

URL: http://localhost:8080/web/guest/home

<link data-senna-track="temporary" href="https://localhost:8080/fr/" hreflang="fr-CA" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080" hreflang="x-default" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080" hreflang="en-US" rel="alternate">

Default Site (using non-default locale - fr)

URL: http://localhost:8080/fr/web/guest/home

<link data-senna-track="temporary" href="https://localhost:8080/fr/" hreflang="fr-CA" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080" hreflang="x-default" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080" hreflang="en-US" rel="alternate">

Created site called testsite with second created public page testpage2 (using default locale - en_US)

URL: http://localhost:8080/web/testsite/testpage2

<link data-senna-track="temporary" href="https://localhost:8080/fr/web/testsite/testpage2" hreflang="fr-CA" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080/web/testsite/testpage2" hreflang="x-default" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080/web/testsite/testpage2" hreflang="en-US" rel="alternate">

URL: http://localhost:8080/en_US/web/testsite/testpage2

<link data-senna-track="temporary" href="https://localhost:8080/fr/web/testsite/testpage2" hreflang="fr-CA" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080/web/testsite/testpage2" hreflang="x-default" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080/web/testsite/testpage2" hreflang="en-US" rel="alternate">

Created site called testsite with second created public page testpage2 (using non-default locale - fr)

URL: http://localhost:8080/fr/web/testsite/testpage2

<link data-senna-track="temporary" href="https://localhost:8080/fr/web/testsite/testpage2" hreflang="fr-CA" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080/web/testsite/testpage2" hreflang="x-default" rel="alternate">
<link data-senna-track="temporary" href="https://localhost:8080/web/testsite/testpage2" hreflang="en-US" rel="alternate">

…lt locale when generating alternate urls for the root site

… included; localization will eventually be re-added if needed, as the next lines generate alternate urls with localization

…same as the canonical url

liferay-continuous-integration · 2017-08-24T02:22:10Z

Pull request test invoked at http://test-1-21.liferay.com/job/test-portal-acceptance-pullrequest(master).

liferay-continuous-integration · 2017-08-24T04:59:45Z

The pull request tester is still running.

Please wait until you get the final report before running 'ci:retest'.

See this link to check on the status of your test:

test-portal-acceptance-pullrequest(master)

@ericyanLr

However, the pull request was closed.

The pull request was closed because the following critical batches had failed:

source-format-jdk8

For information as to why we automatically close out certain pull requests see this article.

*This pull will no longer automatically close if this comment is available. If you believe this is a mistake please re-open this pull by entering the following command as a comment.

ci:reopen

Critical Failure Details:

test-portal-acceptance-pullrequest-batch(master)/source-format-jdk8

Job Results:

0 Tests Passed.
1 Test Failed.

AXIS_VARIABLE=0,label_exp=!master #131494

     [java] 	@Override
     [java] 	public [List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(groupId);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(
     [java] 		long groupId, int start, int end,
     [java] 		OrderByComparator&lt;AssetDisplayTemplate> orderByComparator) {
     [java] 
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(
     [java] 			groupId, start, end, orderByComparator);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public int getAssetDisplayTemplatesCount(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.countByGroupId(groupId]);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	p...> but was:&lt;...
     [java] 	@Override
     [java] 	public [int getAssetDisplayTemplatesCount(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.countByGroupId(groupId);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(groupId);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(
     [java] 		long groupId, int start, int end,
     [java] 		OrderByComparator&lt;AssetDisplayTemplate> orderByComparator) {
     [java] 
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(
     [java] 			groupId, start, end, orderByComparator]);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	p...>
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor.processFormattedFile(BaseSourceProcessor.java:351)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor._format(BaseSourceProcessor.java:526)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor.access$000(BaseSourceProcessor.java:67)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor$1.call(BaseSourceProcessor.java:103)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor$1.call(BaseSourceProcessor.java:98)

liferay-continuous-integration · 2017-08-24T06:21:19Z

Some tests FAILED.

Build Time: 3 hours 20 minutes 10 seconds 993 ms

Base Branch:

Branch Name: master
Branch GIT ID: 9ce9abdd38de489d3d0746f5becf58dc0ef04cf3

Job Summary:

For more details click here.

Failed Jobs:

test-portal-acceptance-pullrequest(master)
Job Results:
130 Jobs Passed.
4 Jobs Failed.
```
Downstream jobs FAILED.
```
test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/11
Job Results:
7 Tests Passed.
1 Test Failed.
1. AXIS_VARIABLE=0,label_exp=!master #139252
  1. CPCommerceOptions#AddContributorCheckboxOptionsEntryValue - Poshi Report - Poshi Summary - Console Output
test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/15
Job Results:
14 Tests Passed.
2 Tests Failed.
1. AXIS_VARIABLE=3,label_exp=!master #131509
  1. CPCommerceOptions#AddCheckboxOptionsEntry - Poshi Report - Poshi Summary - Console Output
  2. CPCommerceOptions#AddCheckboxOptionsEntryValue - Poshi Report - Poshi Summary - Console Output

test-portal-acceptance-pullrequest-batch(master)/source-format-jdk8

Job Results:

0 Tests Passed.
1 Test Failed.

AXIS_VARIABLE=0,label_exp=!master #131494

     [java] 	@Override
     [java] 	public [List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(groupId);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(
     [java] 		long groupId, int start, int end,
     [java] 		OrderByComparator&lt;AssetDisplayTemplate> orderByComparator) {
     [java] 
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(
     [java] 			groupId, start, end, orderByComparator);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public int getAssetDisplayTemplatesCount(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.countByGroupId(groupId]);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	p...> but was:&lt;...
     [java] 	@Override
     [java] 	public [int getAssetDisplayTemplatesCount(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.countByGroupId(groupId);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(long groupId) {
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(groupId);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	public List&lt;AssetDisplayTemplate> getAssetDisplayTemplates(
     [java] 		long groupId, int start, int end,
     [java] 		OrderByComparator&lt;AssetDisplayTemplate> orderByComparator) {
     [java] 
     [java] 		return assetDisplayTemplatePersistence.findByGroupId(
     [java] 			groupId, start, end, orderByComparator]);
     [java] 	}
     [java] 
     [java] 	@Override
     [java] 	p...>
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor.processFormattedFile(BaseSourceProcessor.java:351)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor._format(BaseSourceProcessor.java:526)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor.access$000(BaseSourceProcessor.java:67)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor$1.call(BaseSourceProcessor.java:103)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor$1.call(BaseSourceProcessor.java:98)

For upstream results, click here.

SamZiemer · 2017-08-24T16:22:50Z

ci:reopen

SamZiemer · 2017-08-24T16:22:58Z

ci:retest

liferay-continuous-integration · 2017-08-24T16:23:01Z

Pull request test invoked at http://test-1-6.liferay.com/job/test-portal-acceptance-pullrequest(master).

ericyanLr · 2017-08-24T18:29:13Z

Hi @SamZiemer,

I just received a comment from a related ticket: LPS-74272#comment-1097963
It looks like it would be best to include integration tests with this fix.

I'll be working on adding the integration tests, should I close this pr?
Thanks!

SamZiemer · 2017-08-24T18:31:06Z

Hi Eric, yeah go ahead and close this PR and re assign the tickets to yourself while you work on the integration tests, thanks for letting me know!

SamZiemer · 2017-08-24T18:31:16Z

ci:stop

liferay-continuous-integration · 2017-08-24T18:38:52Z

Some tests FAILED.

Build Time: 2 hours 8 minutes 38 seconds 585 ms

Base Branch:

Branch Name: master
Branch GIT ID: a9e93f94a9af6c4dc919ec308cb190b315cda1a5

Job Summary:

For more details click here.

Failed Jobs:

test-portal-acceptance-pullrequest(master)

Job Results:

128 Jobs Passed.
5 Jobs Failed.

[beanshell] 
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" completed at https://test-1-11.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/145775/. SUCCESS
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" started at https://test-1-11.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/145777/.
[beanshell] 
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" completed at https://test-1-11.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/145777/. SUCCESS
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" started at https://test-1-15.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/94228/.
[beanshell] 
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" completed at https://test-1-15.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/94228/. SUCCESS
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" started at https://test-1-11.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/145776/.
[beanshell] 
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" completed at https://test-1-11.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/145776/. SUCCESS
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" started at https://test-1-18.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/155131/.
[beanshell] 
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" completed at https://test-1-18.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/155131/. SUCCESS
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" started at https://test-1-23.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/8265/.
[beanshell] 
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" completed at https://test-1-23.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/8265/. FAILURE
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" started at https://test-1-4.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/153971/.
[beanshell] 
[beanshell] Build "test-portal-acceptance-pullrequest-batch(master)" completed at https://test-1-4.liferay.com/job/test-portal-acceptance-pullrequest-batch(master)/153971/. SUCCESS
[beanshell] Build model created in 7 seconds 294 ms

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/11
Job Results:
7 Tests Passed.
1 Test Failed.
1. AXIS_VARIABLE=0,label_exp=!master #129747
  1. CPCommerceOptions#AddContributorCheckboxOptionsEntryValue - Poshi Report - Poshi Summary - Console Output
test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/15
Job Results:
14 Tests Passed.
2 Tests Failed.
1. AXIS_VARIABLE=3,label_exp=!master #144441
  1. CPCommerceOptions#AddCheckboxOptionsEntry - Poshi Report - Poshi Summary - Console Output
  2. CPCommerceOptions#AddCheckboxOptionsEntryValue - Poshi Report - Poshi Summary - Console Output
test-portal-acceptance-pullrequest-batch(master)/lpkg-startup-jdk8
Job Results:
9 Tests Passed.
1 Test Failed.
1. AXIS_VARIABLE=0,label_exp=!master #155128
  1. PortalSmoke#Smoke - Poshi Report - Poshi Summary - Console Output

Failures in common with upstream(`9ce9abd`):

test-portal-acceptance-pullrequest-batch(master)/source-format-jdk8

Job Results:

0 Tests Passed.
1 Test Failed.

AXIS_VARIABLE=0,label_exp=!master #8265

     [java] SLF4J: Class path contains multiple SLF4J bindings.
     [java] SLF4J: Found binding in [file:/opt/dev/projects/github/liferay-portal/util-slf4j/classes/org/slf4j/impl/StaticLoggerBinder.class]
     [java] SLF4J: Found binding in [jar:file:/opt/dev/projects/github/liferay-portal/lib/development/slf4j-simple.jar!/org/slf4j/impl/StaticLoggerBinder.class]
     [java] SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
     [java] SLF4J: Actual binding is of type [com.liferay.util.sl4fj.LiferayLoggerFactory]
     [java] /opt/dev/projects/github/liferay-portal/portal-impl/src/com/liferay/portal/util/PortalImpl.java
     [java] Exception in thread "main" com.liferay.source.formatter.SourceMismatchException: ./portal-impl/src/com/liferay/portal/util/PortalImpl.java expected:&lt;...canonicalURLSuffix =[ canonicalURLSuffix.substring(
     [java] 					]i18nPath.length());
     [java] ...> but was:&lt;...canonicalURLSuffix =[
     [java] 					canonicalURLSuffix.substring(]i18nPath.length());
     [java] ...>
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor.processFormattedFile(BaseSourceProcessor.java:351)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor._format(BaseSourceProcessor.java:526)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor.access$000(BaseSourceProcessor.java:67)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor$1.call(BaseSourceProcessor.java:103)
     [java] 	at com.liferay.source.formatter.BaseSourceProcessor$1.call(BaseSourceProcessor.java:98)

For upstream results, click here.

ericyanLr added 4 commits August 23, 2017 11:49

LPS-74031 Account for localization when generating canonical urls

45226cb

LPS-74031 Account for LOCALE_PREPEND_FRIENDLY_URL_STYLE and the defau…

8b71319

…lt locale when generating alternate urls for the root site

LPS-74031 Exclude localization from the canonical url suffix if it is…

52869cd

… included; localization will eventually be re-added if needed, as the next lines generate alternate urls with localization

LPS-74031 Account for the default hreflang url; it is not always the …

938467b

…same as the canonical url

liferay-continuous-integration closed this Aug 24, 2017

liferay-continuous-integration reopened this Aug 24, 2017

SamZiemer closed this Aug 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LPP-26493 #361

LPP-26493 #361

ericyanLr commented Aug 24, 2017

liferay-continuous-integration commented Aug 24, 2017

liferay-continuous-integration commented Aug 24, 2017

test-portal-acceptance-pullrequest-batch(master)/source-format-jdk8

Job Results:

liferay-continuous-integration commented Aug 24, 2017

test-portal-acceptance-pullrequest(master)

Job Results:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/11

Job Results:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/15

Job Results:

test-portal-acceptance-pullrequest-batch(master)/source-format-jdk8

Job Results:

SamZiemer commented Aug 24, 2017

SamZiemer commented Aug 24, 2017

liferay-continuous-integration commented Aug 24, 2017

ericyanLr commented Aug 24, 2017

SamZiemer commented Aug 24, 2017

SamZiemer commented Aug 24, 2017

liferay-continuous-integration commented Aug 24, 2017

test-portal-acceptance-pullrequest(master)

Job Results:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/11

Job Results:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat80-mysql56-jdk8/15

Job Results:

test-portal-acceptance-pullrequest-batch(master)/lpkg-startup-jdk8

Job Results:

test-portal-acceptance-pullrequest-batch(master)/source-format-jdk8

Job Results:

LPP-26493 #361

LPP-26493 #361

Conversation

ericyanLr commented Aug 24, 2017

Main Issue

Second Issue

A Few Example Scenarios - Main Issue

Default Site (using default locale - en_US)

Default Site (using non-default locale - fr)

Created site called testsite with second created public page testpage2 (using default locale - en_US)

Created site called testsite with second created public page testpage2 (using non-default locale - fr)

A Few Example Scenarios - Second Issue

Default Site (using default locale - en_US)

Default Site (using non-default locale - fr)

Created site called testsite with second created public page testpage2 (using default locale - en_US)

Created site called testsite with second created public page testpage2 (using non-default locale - fr)

liferay-continuous-integration commented Aug 24, 2017

liferay-continuous-integration commented Aug 24, 2017

The pull request tester is still running.

However, the pull request was closed.

Critical Failure Details:

Job Results:

liferay-continuous-integration commented Aug 24, 2017

Some tests FAILED.

Base Branch:

Job Summary:

For more details click here.

Failed Jobs:

Job Results:

Job Results:

Job Results:

Job Results:

For upstream results, click here.

SamZiemer commented Aug 24, 2017

SamZiemer commented Aug 24, 2017

liferay-continuous-integration commented Aug 24, 2017

ericyanLr commented Aug 24, 2017

SamZiemer commented Aug 24, 2017

SamZiemer commented Aug 24, 2017

liferay-continuous-integration commented Aug 24, 2017

Some tests FAILED.

Base Branch:

Job Summary:

For more details click here.

Failed Jobs:

Job Results:

Job Results:

Job Results:

Job Results:

Failures in common with upstream(9ce9abd):

Job Results:

For upstream results, click here.

Failures in common with upstream(`9ce9abd`):