GROOVY-11578: provide default FastStringService in case of service loader failure

[royteeuwen]

No description provided.

[royteeuwen]

@paulk-asert could we already go for this workaround for the linked ticket, it should not break something and will at least fix the groovy-json bundle which is the only one specifically with this kind of logic where it searches for a service loader or else returns null

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.0074%. Comparing base (d0540bc) to head (ac2ce2a).
⚠️ Report is 53 commits behind head on master.

Files with missing lines	Patch %	Lines
...g/apache/groovy/json/internal/FastStringUtils.java	0.0000%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2330        +/-   ##
==================================================
+ Coverage     67.0045%   67.0074%   +0.0028%     
- Complexity      29330      29357        +27     
==================================================
  Files            1382       1382                
  Lines          116613     116623        +10     
  Branches        20432      20445        +13     
==================================================
+ Hits            78136      78146        +10     
- Misses          32038      32041         +3     
+ Partials         6439       6436         -3     

Files with missing lines	Coverage Δ
...g/apache/groovy/json/internal/FastStringUtils.java	55.5556% <0.0000%> (-5.5555%)	⬇️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[royteeuwen]

@eric-milles would you be able to review this :?

[royteeuwen]

@eric-milles can you recheck with my smaller list of changes :)?

[eric-milles]

You can probably remove the null check on lines 55-57. Is there any hope for a test case?

[eric-milles]

If the DefaultFastStringService comes from the groovy-json module, is it supposed to be available via service loading? Sorry, I'm a bit behind in the discussion.

[royteeuwen]

@eric-milles yes, it should become available through Service Loading but there are two issues:

• Now that groovy-json is set as a Fragment, it does not work anymore, only Bundles in OSGi will be loaded through service loading, hence the change of removing the Fragment-Host property that was initially part of this PR
• There can be a race condition, where the Bundle is still starting up and registering it's service loaders (which happens sort of async), but the groovy-json class FastStringUtils is already called, meaning that it doesn't find the service loader at that moment. So the code how it's written now is never going to work in that case because it only allows you to get the string service instance once. I hope this will be resolved in [POC] OSGi Core R9 - Java SPI Support felix-dev#455 and Enhance the specification for OSGi Core SPI support osgi/osgi#882

I will check if I find some time to add test cases :) I expect it will be a simple test hehe.

[royteeuwen]

@eric-milles test case added :)