LPS-132150 Fix error registering GraphQLDTOContributors because Facet GraphQLObjectType was not registered at that moment #1061

javierdearcos · 2021-05-14T06:03:43Z

The bug was caused by a race condition because types coming from annotations are loaded asynchronously and when the type was requested by the GraphQLDTODefinition to be used as part of the Page definition, it was not available yet. To solve it I try to check if the type is available and if not, register it manually.

This solution causes a conflict when the Facet class GraphQL annotations were processed and try to register the type again, so to avoid this conflict I choose to remove the annotations and always register the Facet type at the beginning to be available for GraphQL annotations processor and for the GraphQLDTOContributor registry.

I think it could be a good approach as we mentioned several times that we wanted to remove the GraphQL annotations lib.

I left both commits so you can check the process but we can squash them before sending the PR

… GraphQLObjectType was not registered The problem was caused by a race condition, because types coming from annotations are loaded asynchronously and when the type was requested by the GraphQLDTODefinition to be used as part of the Page definition, it was not available yet. To solve it I check if the type is available and if not, register it manually

…the Facet GraphQL annotations are processed. So change the approach removing the GraphQL annotations and registering the type at the beginning to be available for the graphql-annotations library and for the GraphQLDTOContributor registry

liferay-continuous-integration · 2021-05-14T06:03:47Z

CI is automatically triggering the following test suites:

ci:test:relevant
ci:test:sf

liferay-continuous-integration · 2021-05-14T06:09:48Z

✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: b2df07e393f9f46c83b605e0a50e6f03c699cdf4

Sender Branch:

Branch Name: LPS-132150
Branch GIT ID: 7aab5d50ab1512a6a363a90566d205594ba1847a

1 out of 1jobs PASSED

1 Successful Jobs:

test-portal-source-format

For more details click here.

liferay-continuous-integration · 2021-05-14T06:12:05Z

Jenkins Build:test-portal-source-format#5881

Jenkins Report:jenkins-report.html

Jenkins Suite:sf

Pull Request:liferay-frontend#1061

Testray Routine:EE Pull Request

Testray Build:[master] ci:test:sf - javierdearcos > liferay-frontend - PR#1061 - 2021-05-13[23:06:34]

Testray Importer:publish-testray-report#1011

liferay-continuous-integration · 2021-05-14T08:20:17Z

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 21 out of 22 jobs passed in 1 hour 38 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: b2df07e393f9f46c83b605e0a50e6f03c699cdf4

Upstream Comparison:

Branch GIT ID: 6e25ad1e3cdcd4c6657caad6adef34ec3fbd64c6
Jenkins Build URL: Acceptance Upstream DXP (master) #1863

ci:test:stable - 9 out of 9 jobs PASSED

9 Successful Jobs:

ci:test:relevant - 20 out of 22 jobs PASSED

2 Failed Jobs:

20 Successful Jobs:

For more details click here.

This pull contains no unique failures.

Failures in common with acceptance upstream results at 6e25ad1:

test-portal-acceptance-pullrequest-batch(master)/modules-semantic-versioning-jdk8/0

Job Results:

2 Tests Passed.
1 Test Failed.

AXIS_VARIABLE=0,label_exp=!master #336641

com.liferay.semantic.versioning.SemanticVersioningTest.testSemanticVersioning[/apps/portal-vulcan/portal-vulcan-api]

java.lang.AssertionError:   PACKAGE_NAME                                       DELTA      CUR_VER    BASE_VER   REC_VER    WARNINGS  
= ================================================== ========== ========== ========== ========== ==========
* com.liferay.portal.vulcan.aggregation              CHANGED    1.2.0      1.2.0      1.2.1      VERSION INCREASE REQUIRED
	~   class      com.liferay.portal.vulcan.aggregation.Facet
		~   field      facetCriteria
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
		~   field      facetValues
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
		-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLName
			-   property   value='Facet'
	~   class      com.liferay.portal.vulcan.aggregation.Facet$FacetValue
		~   field      numberOfOccurrences
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
		~   field      term
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
			-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLField
		-   annotated  com.liferay.portal.vulcan.graphql.annotation.GraphQLName
			-   property   value='FacetValue'
at org.junit.Assert.fail(Assert.java:88)
at org.junit....</code></pre></div></li></ol></div></li></ol></div></li></ol></details></details></html><h5>Test bundle downloads:</h5><ul><li><a href="https://files.liferay.com/private/bundles/test-1-9/jobs/test-portal-acceptance-pullrequest(master)/builds/4790/liferay-portal-bundle-tomcat.tar.gz">liferay-portal-bundle-tomcat.tar.gz</a> (mirrors: <a href="http://mirrors.dlc.liferay.com/files.liferay.com/private/bundles/test-1-9/jobs/test-portal-acceptance-pullrequest(master)/builds/4790/liferay-portal-bundle-tomcat.tar.gz">dlc</a>, <a href="http://mirrors.lax.liferay.com/files.liferay.com/private/bundles/test-1-9/jobs/test-portal-acceptance-pullrequest(master)/builds/4790/liferay-portal-bundle-tomcat.tar.gz">lax</a>)</li></ul>

liferay-continuous-integration · 2021-05-14T08:24:20Z

Jenkins Build:test-portal-acceptance-pullrequest(master)#4790

Jenkins Report:jenkins-report.html

Jenkins Suite:relevant

Pull Request:liferay-frontend#1061

Testray Routine:EE Pull Request

Testray Build:[master] ci:test:relevant - javierdearcos > liferay-frontend - PR#1061 - 2021-05-13[23:40:50]

Testray Importer:publish-testray-report#765

javierdearcos · 2021-05-14T08:26:23Z

Seems there is a missing baseline in the Failures in common with acceptance upstream results at 6e25ad1. Adding it

javiergamarra · 2021-05-15T15:48:15Z

The bug was caused by a race condition because types coming from annotations are loaded asynchronously

Are you sure that's the case?

javiergamarra

Code is Ok. But I'm worried about the bug... can you double check if 1) you can reproduce it several times (I can't) 2) check if the annotations are really processed asynchronously. Thanks!

javiergamarra · 2021-05-15T15:58:18Z

...src/main/java/com/liferay/portal/vulcan/internal/graphql/servlet/GraphQLServletExtender.java

@@ -1237,6 +1256,11 @@ private Servlet _createServlet() throws Exception {
 			Map<Class<?>, Set<Class<?>>> classesMap =
 				processingElementsContainer.getExtensionsTypeRegistry();

+			processingElementsContainer.setTypeRegistry(


Why here? looks like more of a field related thing that should be before _collectObjectFields

javierdearcos · 2021-05-17T15:28:43Z

I've tried to reproduce it all afternoon without success 😞

I'm pretty sure that the other day the classes were registered in an asynchronous way because as it executes the register... methods, the registered classes increase by hundreds (when there weren't so many registered) and the Facet class wasn't present, although it should be there just after the collect... methods.

Maybe I was wrong the other day and it is better to close the bug until it happens again if it does

javiergamarra · 2021-05-17T19:50:04Z

I've checked the code internally and I don't see where it could be asynchronous... the error makes sense if there were no ServletData registered but having APIs in the server it does not makes sense.

Closing for now, if it happens again, we'll reopen.

javierdearcos · 2021-05-18T05:36:07Z

Ok, I'll close the bug as no longer reproducible too

Javier de Arcos added 2 commits May 14, 2021 07:40

javiergamarra added the s-headless Pull requests to be reviewed by the headless squad label May 14, 2021

liferay-continuous-integration added ci:test:sf - pending ci:test:sf - success and removed ci:test:sf - pending labels May 14, 2021

liferay-continuous-integration added ci:test:relevant - pending ci:test:stable - pending labels May 14, 2021

javierdearcos requested a review from javiergamarra May 14, 2021 07:19

liferay-continuous-integration added ci:test:relevant - success ci:test:stable - success and removed ci:test:relevant - pending ci:test:stable - pending labels May 14, 2021

LPS-132150 Baseline

a8b470d

javiergamarra reviewed May 15, 2021

View reviewed changes

javiergamarra closed this May 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LPS-132150 Fix error registering GraphQLDTOContributors because Facet GraphQLObjectType was not registered at that moment #1061

LPS-132150 Fix error registering GraphQLDTOContributors because Facet GraphQLObjectType was not registered at that moment #1061

javierdearcos commented May 14, 2021

liferay-continuous-integration commented May 14, 2021

liferay-continuous-integration commented May 14, 2021

Base Branch:

Sender Branch:

For more details click here.

liferay-continuous-integration commented May 14, 2021

liferay-continuous-integration commented May 14, 2021

Base Branch:

Upstream Comparison:

2 Failed Jobs:

For more details click here.

This pull contains no unique failures.

test-portal-acceptance-pullrequest-batch(master)/modules-semantic-versioning-jdk8/0

Job Results:

liferay-continuous-integration commented May 14, 2021

javierdearcos commented May 14, 2021

javiergamarra commented May 15, 2021

javiergamarra left a comment

javiergamarra May 15, 2021

javierdearcos commented May 17, 2021

javiergamarra commented May 17, 2021

javierdearcos commented May 18, 2021

LPS-132150 Fix error registering GraphQLDTOContributors because Facet GraphQLObjectType was not registered at that moment #1061

LPS-132150 Fix error registering GraphQLDTOContributors because Facet GraphQLObjectType was not registered at that moment #1061

Conversation

javierdearcos commented May 14, 2021

liferay-continuous-integration commented May 14, 2021

liferay-continuous-integration commented May 14, 2021

✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes

Base Branch:

Sender Branch:

For more details click here.

liferay-continuous-integration commented May 14, 2021

liferay-continuous-integration commented May 14, 2021

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 21 out of 22 jobs passed in 1 hour 38 minutes

Base Branch:

Upstream Comparison:

2 Failed Jobs:

For more details click here.

This pull contains no unique failures.

test-portal-acceptance-pullrequest-batch(master)/modules-semantic-versioning-jdk8/0

Job Results:

liferay-continuous-integration commented May 14, 2021

javierdearcos commented May 14, 2021

javiergamarra commented May 15, 2021

javiergamarra left a comment

Choose a reason for hiding this comment

javiergamarra May 15, 2021

Choose a reason for hiding this comment

javierdearcos commented May 17, 2021

javiergamarra commented May 17, 2021

javierdearcos commented May 18, 2021