Add S3/MinIO support for application settings

[muuki88]

This PR adds support for an additional storage system to retrieve

• application settings
• stored impressions
• stored requests
• stored responses

Any S3 compatible system can be used.

Usage

It work's bascially the same as the file system settings, but the files are on S3. What we found, works really well, is using the ad unit path of GAM for the stored impression ID as this maps nicely to paths.

Credits

Credits goe to @0xlee for the intitial idea and @rmattis for the actual implementation.

[muuki88]

@And1sS I hope I resolved all the issues. Do you mind taking a look 🧐☺️

[SerhiiNahornyi]

@muuki88 Java Ci failed, due to checkstyle

[SerhiiNahornyi]

@muuki88, According to Java CI, a few failed tests; Please fix

[muuki88]

@SerhiiNahornyi thanks for the ping. I fixed it. I removed the Optional implementation too radically

[And1sS]

Hi @muuki88, since PBS 3.0 is finally released, we can come back to this pr! Can you, please, merge latest master into your branch?

[muuki88]

:tada: ... I'll try to do this weekend. I'll be off for the next two weeks, but I'm super hyped to put some work into this.

[SerhiiNahornyi]

Hi @muuki88,

The Java CI is failing because of the S3PeriodicRefreshServiceTest. Could you take a look and fix it when you get a chance?

[muuki88]

Hi @SerhiiNahornyi

Thanks for the ping. I haven't found the time yet, to continue 🙈🙈 Try to this the next week.

[SerhiiNahornyi]

@muuki88 We have migrated to JUnit 5 in our latest master commits. Could you please merge the master branch into your branch to verify that it works after these changes?

[muuki88]

Hi @SerhiiNahornyi

I added the region configuration setting, but I'm totally lost with the Junit5 migration. I haven't used Junit in years and I looked at the DatabaseSettings refactorings, but they include not enough information to figure out how to migrate the vertx test setup. Can you give any pointers?

[SerhiiNahornyi]

Java CI failed because of checkstyle

Error:  /home/runner/work/prebid-server-java/prebid-server-java/src/main/java/org/prebid/server/settings/service/S3PeriodicRefreshService.java:16:8: Unused import - software.amazon.awssdk.core.BytesWrapper. [UnusedImports]
Error:  /home/runner/work/prebid-server-java/prebid-server-java/src/main/java/org/prebid/server/settings/service/S3PeriodicRefreshService.java:25:8: Unused import - java.util.ArrayList. [UnusedImports]

[SerhiiNahornyi]

Again, checkstyle :)

Usually, to avoid this, before pushing, we run the command mvn clean package -f extra in the root of the project. It performs the same checks that Java CI does.

Error:  /home/runner/work/prebid-server-java/prebid-server-java/src/test/java/org/prebid/server/settings/service/S3PeriodicRefreshServiceTest.java:32:15: Unused import - java.util.Collections.emptyList. [UnusedImports]
Error:  /home/runner/work/prebid-server-java/prebid-server-java/src/test/java/org/prebid/server/settings/service/S3PeriodicRefreshServiceTest.java:33:15: Unused import - java.util.Collections.emptyMap. [UnusedImports]
Error:  /home/runner/work/prebid-server-java/prebid-server-java/src/test/java/org/prebid/server/settings/service/S3PeriodicRefreshServiceTest.java:34:15: Unused import - java.util.Collections.singletonList. [UnusedImports]

[SerhiiNahornyi]

Some tests inside Java CI failed

Errors: 
Error:    S3PeriodicRefreshServiceTest.initializeShouldMakeOneInitialRequestAndTwoScheduledRequestsWithParam:95->createAndInitService:165 » NullPointer Cannot invoke "io.vertx.core.impl.ContextInternal.promise()" because "context" is null
Error:    S3PeriodicRefreshServiceTest.initializeShouldMakeOnlyOneInitialRequestIfRefreshPeriodIsNegative:105->createAndInitService:165 » NullPointer Cannot invoke "io.vertx.core.impl.ContextInternal.promise()" because "context" is null
Error:    S3PeriodicRefreshServiceTest.shouldCallSaveWithExpectedParameters:82->createAndInitService:165 » NullPointer Cannot invoke "io.vertx.core.impl.ContextInternal.promise()" because "context" is null
Error:    S3PeriodicRefreshServiceTest.shouldUpdateTimerAndErrorMetric:129->createAndInitService:165 » NullPointer Cannot invoke "io.vertx.core.impl.ContextInternal.promise()" because "context" is null
Error:    S3PeriodicRefreshServiceTest.shouldUpdateTimerMetric:115->createAndInitService:165 » NullPointer Cannot invoke "io.vertx.core.impl.ContextInternal.promise()" because "context" is null

[muuki88]

The tests fail due to the requested vertx.getOrCreateContext refactoring. I couldn't find any other usage in the codebase of this method call and thus no example on how to mock this. Also searching the internet I wasn't able to figure out how to stub this method call.

java.lang.NullPointerException: Cannot invoke "io.vertx.core.impl.ContextInternal.promise()" because "context" is null

	at io.vertx.core.Future.fromCompletionStage(Future.java:623)
	at org.prebid.server.settings.service.S3PeriodicRefreshService.listFiles(S3PeriodicRefreshService.java:141)
	at org.prebid.server.settings.service.S3PeriodicRefreshService.getFileContentsForDirectory(S3PeriodicRefreshService.java:125)
	at org.prebid.server.settings.service.S3PeriodicRefreshService.fetchStoredDataResult(S3PeriodicRefreshService.java:95)
	at org.prebid.server.settings.service.S3PeriodicRefreshService.initialize(S3PeriodicRefreshService.java:82)
	at org.prebid.server.settings.service.S3PeriodicRefreshServiceTest.createAndInitService(S3PeriodicRefreshServiceTest.java:165)
	at org.prebid.server.settings.service.S3PeriodicRefreshServiceTest.initializeShouldMakeOnlyOneInitialRequestIfRefreshPeriodIsNegative(S3PeriodicRefreshServiceTest.java:105)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

[CTMBNara]

This works, but it's kinda strange, because it just swallows the exception.

The point of this initializePromise is to check if that service was started properly. If it can't start, then there's no point in starting up prebid server at all, because all configuration is missing.

While this "fixes" the issue, it allows prebid server to start up in a useless state.

WDYT?

Yeah, you are right. Thanks that you accidentally found it :) Bug-fix: https://github.com/prebid/prebid-server-java/pull/3385

Regarding your question about "how to mock vertx.getOrCreateContext() call": see CircuitBreakerSecuredGeoLocationServiceTest.class on how to mock vertx environment for tests. Also, given that we have fixed the underlying issues, we need to revert this commit.

[muuki88]

Regarding your question about "how to mock vertx.getOrCreateContext() call": see CircuitBreakerSecuredGeoLocationServiceTest.class on how to mock vertx environment for tests. Also, given that we have fixed the underlying issues, we need to revert this commit.

I did it a little bit differently to keep the assertions being made

[muuki88]

Sorry for my confusion. Should I use vertx.getOrCreateContext() or not 😅?

I guess this is the only thing left. And the adjustments to tests, depending on the decision.

[CTMBNara]

Sorry for my confusion. Should I use vertx.getOrCreateContext() or not 😅?

I guess this is the only thing left. And the adjustments to tests, depending on the decision.

Better to use

[Compile-Ninja]

@muuki88, Thank you so much for your hard work and contributions on PR #2733. Your efforts laid a strong foundation, and we genuinely appreciate the time and energy you invested in this.

Since we needed to move forward and complete the work, we created a new PR (PR #3418) based on your original changes. The new PR is now finished, and we’re just waiting for your approval to proceed.

We’re truly grateful for your contributions, and your work was instrumental in getting us to this point. We would greatly appreciate it if you could take a moment to review and approve the new PR when you have a chance.

Thanks again!

[muuki88]

Hi @Compile-Ninja

That means a lot ❤️ thank you. I'll try to review it as soon as possible. Currently I'm on vacation

[Compile-Ninja]

Duplication of https://github.com/prebid/prebid-server-java/pull/3418