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

Fixing concurrency and compilation issues. #385

Merged
merged 21 commits into from
Mar 17, 2021

Conversation

prashanthjos
Copy link
Member

This PR fixes below issues.

  1. Compilation issue in TypeFactoryTest class. Not sure how this compilation issue went in. Commenting the compilation issue on the test class for now. Respective developer can have a look at it.
  2. Concurrency issue that can be caused by the static instance variable in BaseJsonValidator class. While setting the attributes of ValidatorState's by two threads can result in a "lost update" issue.
  3. Updates to Walking. Adding a new WalkFlow Constant.
  4. Minor code comment updates.
  5. Test case updates.

Please let me know if you see any concerns with the above issues or updated files.

@codecov-io
Copy link

Codecov Report

Merging #385 (34b8cf7) into master (ca1847a) will increase coverage by 0.15%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #385      +/-   ##
============================================
+ Coverage     69.63%   69.78%   +0.15%     
- Complexity      707      713       +6     
============================================
  Files            81       81              
  Lines          2766     2777      +11     
  Branches        562      564       +2     
============================================
+ Hits           1926     1938      +12     
+ Misses          626      623       -3     
- Partials        214      216       +2     
Impacted Files Coverage Δ Complexity Δ
...n/java/com/networknt/schema/BaseJsonValidator.java 65.45% <ø> (-0.62%) 20.00 <0.00> (-1.00)
...in/java/com/networknt/schema/CollectorContext.java 83.87% <ø> (-3.23%) 12.00 <0.00> (ø)
...rc/main/java/com/networknt/schema/TypeFactory.java 73.33% <0.00%> (-3.42%) 18.00 <0.00> (ø)
...main/java/com/networknt/schema/walk/WalkEvent.java 81.25% <0.00%> (-2.63%) 5.00 <0.00> (ø)
...a/com/networknt/schema/SchemaValidatorsConfig.java 57.37% <33.33%> (+0.48%) 19.00 <1.00> (+2.00)
...etworknt/schema/AdditionalPropertiesValidator.java 87.23% <50.00%> (ø) 16.00 <0.00> (ø)
...java/com/networknt/schema/PropertiesValidator.java 88.13% <50.00%> (-0.58%) 14.00 <0.00> (-1.00)
...worknt/schema/walk/AbstractWalkListenerRunner.java 85.00% <57.14%> (-15.00%) 10.00 <4.00> (ø)
...main/java/com/networknt/schema/ValidatorState.java 94.11% <66.66%> (+23.52%) 8.00 <1.00> (+2.00)
...main/java/com/networknt/schema/OneOfValidator.java 61.34% <85.71%> (+0.32%) 12.00 <1.00> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46cc360...34b8cf7. Read the comment docs.

@prashanthjos
Copy link
Member Author

Pre checkin Build or CI/CD is no longer running and this might result in some compilation issues sneaking into master. Can you please take a look at the CI/CD flow also.

@prashanthjos
Copy link
Member Author

Hi @stevehu can you please have a look at this PR and can you please merge at your convenience.

@stevehu
Copy link
Contributor

stevehu commented Mar 16, 2021

@hkupty Could you please take a look at why the lossless test cases are failing after the change? Thanks.

@prashanthjos
Copy link
Member Author

@stevehu can you merge this PR if you are OK with the changes. I have commented the test cases causing compilation issues. Also can we have build hooks enabled, so that such issues can be caught early.

@prashanthjos
Copy link
Member Author

Also any idea why travis builds are not being triggered?

@stevehu
Copy link
Contributor

stevehu commented Mar 17, 2021

@prashanthjos I just realized that the Travis build failed. Check the log for the master branch and found the following errors. I am looking into it now. Thanks a lot for raising it.

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.1:testCompile (test-compile) on project json-schema-validator: Compilation failure: Compilation failure: 
[ERROR] /home/travis/build/networknt/json-schema-validator/src/test/java/com/networknt/schema/TypeFactoryTest.java:[57,24] cannot find symbol
[ERROR]   symbol:   variable validValue
[ERROR]   location: class com.networknt.schema.TypeFactoryTest
[ERROR] /home/travis/build/networknt/json-schema-validator/src/test/java/com/networknt/schema/TypeFactoryTest.java:[60,24] cannot find symbol
[ERROR]   symbol:   variable validValue
[ERROR]   location: class com.networknt.schema.TypeFactoryTest
[ERROR] /home/travis/build/networknt/json-schema-validator/src/test/java/com/networknt/schema/TypeFactoryTest.java:[67,20] cannot find symbol
[ERROR]   symbol:   variable validValue
[ERROR]   location: class com.networknt.schema.TypeFactoryTest
[ERROR] /home/travis/build/networknt/json-schema-validator/src/test/java/com/networknt/schema/TypeFactoryTest.java:[70,20] cannot find symbol
[ERROR]   symbol:   variable validValue
[ERROR]   location: class com.networknt.schema.TypeFactoryTest
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

@stevehu
Copy link
Contributor

stevehu commented Mar 17, 2021

@prashanthjos I have fixed the TypeFactoryTest issues and it should be working now in the master branch. Could you please sync from master? It was my mistake when merging the previous PR. Thanks.

@prashanthjos
Copy link
Member Author

@stevehu I fixed the merge conflict. Can you please restore Travis CI from next builds.

Can you please merge the PR.

@stevehu stevehu merged commit 4fa1ab4 into networknt:master Mar 17, 2021
@hkupty
Copy link
Contributor

hkupty commented Mar 18, 2021

Sorry I missed the notification. What was the issue? Do you still want me to have a look at it?

@prashanthjos
Copy link
Member Author

@hkupty I think @stevehu already fixed the issue. @stevehu can you please release latest version with these changes.

prashanthjos added a commit to prashanthjos/json-schema-validator that referenced this pull request Mar 18, 2021
Fixing concurrency and compilation issues. (networknt#385)
@stevehu
Copy link
Contributor

stevehu commented Mar 18, 2021

A new release 1.0.50 is out. Thanks.

https://github.com/networknt/json-schema-validator/releases/tag/1.0.50

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

Successfully merging this pull request may close these issues.

5 participants