IQSS / dataverse

Open source research data repository software
http://dataverse.org
Other
876 stars 484 forks source link

Suggestion: replace constructor/method params for beans with CDI call #10774

Open qqmyers opened 1 month ago

qqmyers commented 1 month ago

Overview of the Suggestion We have several places in the code (JSONParser, OREMap, ...) where we include persistent beans as constructor or method parameters (because these are POJO classes rather than beans so they can't just use @EJB to reference them. However, I recently ran across a couple places where jakarta.enterprise.inject.spi.CDI.current().select(*Bean.class).get() is used to get an instance of one of our services. It seems like this is a cleaner approach, especially for cases where we're passing the service bean through multiple calls.

Are you thinking about creating a pull request for this issue? Not in the near term - raising this as a possibility for others who might want to clean up a bit and to raise awareness that this might be the preferred approach for new code.

pdurbin commented 3 weeks ago

I like this idea and I gave it a try in 22453dd for DatasetTypeServiceBean but when I run mvn test I get failures:

[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   SchemaDotOrgExporterTest.testExportDataset:88->createExportFromJson:178 » IllegalState Unable to locate CDIProvider
[ERROR]   SchemaDotOrgExporterTest.testExportDescriptionTruncation:170->createExportFromJson:178 » IllegalState Unable to locate CDIProvider
[ERROR]   FeedbackUtilTest.setUpClass:149 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[ERROR]   JsonParserTest.setUp:134 » IllegalState Unable to locate CDIProvider
[INFO] 
[ERROR] Tests run: 1613, Failures: 0, Errors: 29, Skipped: 36
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  21.078 s
[INFO] Finished at: 2024-08-23T11:03:07-04:00
[INFO] ------------------------------------------------------------------------

I tried the suggestion at https://stackoverflow.com/questions/60694299/unit-tests-which-need-cdi-container-result-in-java-lang-illegalstateexception-u/60698771#60698771 but it didn't help. 🤷