chore: solver service byom security improvements - refactor to not in… by mswiderski · Pull Request #2695 · TimefoldAI/timefold-solver

mswiderski · 2026-09-25T07:46:59Z

…clude storages into models and intorduce single proxy component instead

rsynek · 2026-10-01T07:20:09Z

+ * The per data set lock is taken by the operations of the storage service, and the composite operations take it
+ * around several of them, so it has to be reentrant without ever letting a second thread in.
+ */
+class AbstractStorageServiceLockTest {


Is it worth having a test that checks internal locking?

rsynek · 2026-10-01T07:20:49Z

+            Field field = AbstractStorageService.class.getDeclaredField("locks");
+            field.setAccessible(true);


Especially if it needs to hack private fields?

rsynek · 2026-10-01T07:23:56Z

        Azure(AZURE_STORAGE),
        InMemory(INMEMORY_STORAGE),
-        FileSystem(FILESYSTEM_STORAGE);
+        FileSystem(FILESYSTEM_STORAGE),


Indeed, this is unusual naming for enum entries. Better to change all of them.

I removed this enum from solver service as most of the storages were removed as well. No point for keeping it here.

rsynek · 2026-10-01T07:30:01Z

+ * Produces StorageObjectMapper instances with storage-specific (de)serialization settings.
+ */
+@ApplicationScoped
+public class StorageObjectMapperProducer {


No beans in the definition module, please. It can silently alternate the behavior in any module that depends on it.

removed ApplicationScoped from the class.

rsynek · 2026-10-01T08:02:32Z

+    /**
+     * What was last read, replaced as a whole so that a reader never sees a half-written one.
+     */
+    private volatile Reading reading;


The SonarCloud comment is correct, but more importantly, what's the point of volatile here?

don't think it is correct, the Reading is immutable object so it won't change by different thread but the reference can be changed when reloaded and that's why volatile is used, to immediately propagate that to other threads.
Service account token is rotated every hour so it needs to be periodically reloaded.

rsynek · 2026-10-01T08:10:23Z

@@ -0,0 +1,109 @@
+package ai.timefold.solver.service.definition.internal.platform;


Is this class going to be imported by the platform?

it is used also by one component of the platform. That's why in the package, I could make a copy of it if you prefer that?

…clude storages into models and intorduce single proxy component instead

Copilot

Copilot review overview

🟡 Changes recommended

Cache correctness, metadata loss, compression detection, and mapper configuration defects must be resolved.

Review effort: Balanced
Findings: 3 · 2

Open (5)

Read both magic bytes before classifying gzip streams · New
Install updated immutable deserialization configuration · New
Preserve all metadata fields when listing runs · New
Avoid Optional for nullable result · New
Document new cache and retry configuration properties · New

What changed in this PR

Refactors solver-service storage into a model-agnostic byte-stream layer and adds authenticated service communication.

Changes:

Centralizes serialization, compression, caching, and retries in AbstractStorageService.
Moves in-memory storage into a dedicated module and simplifies Quarkus generation.
Adds Kubernetes service-account authentication for map-service calls.

File	Description
`service/worker/.../TestdataStorageService.java`	Adapts test storage service to the new API.
`service/worker/.../TestdataStorage.java`	Uses the relocated in-memory storage.
`service/worker/.../DefaultSolverWorkerFacadeTest.java`	Configures serialization for storage tests.
`service/worker/pom.xml`	Adds the in-memory test dependency.
`service/storage-inmemory/.../InMemoryStorage.java`	Implements model-agnostic in-memory storage.
`service/storage-inmemory/pom.xml`	Defines the new storage module.
`service/quarkus/runtime/pom.xml`	Includes default in-memory storage at runtime.
`service/quarkus/integration-tests/.../ModelsExtensionTest.java`	Updates storage injection expectations.
`service/quarkus/deployment/.../ModelsExtensionStorageGenerationTest.java`	Updates generated-storage tests.
`service/quarkus/deployment/.../TimefoldStorageProcessor.java`	Removes typed storage generation.
`service/quarkus/deployment/pom.xml`	Adds in-memory storage for tests.
`service/pom.xml`	Registers the new module.
`service/maps/service-client/.../DummyStorageService.java`	Implements the output-type hook.
`service/maps/service-client/.../ServiceAccountTokenHeadersFactoryTest.java`	Tests bearer-header behavior.
`service/maps/service-client/.../ServiceAccountTokenHeadersFactory.java`	Adds service-account authorization headers.
`service/maps/service-client/.../MapServiceClient.java`	Registers the header factory.
`service/facade/service-parent/pom.xml`	Removes direct enterprise storage dependencies.
`service/definition/.../AbstractStorageServiceRetryTest.java`	Tests retry behavior.
`service/definition/.../ServiceAccountTokenTest.java`	Tests token loading and refresh.
`service/definition/.../CompressionUtilsTest.java`	Tests compression utilities.
`service/definition/.../SupportedStorages.java`	Removes obsolete storage identifiers.
`service/definition/.../StorageItem.java`	Introduces listed-storage metadata.
`service/definition/.../StorageContent.java`	Introduces streamed storage content.
`service/definition/.../Storage.java`	Reworks storage around raw streams.
`service/definition/.../AbstractStorageService.java`	Adds serialization, caching, locking, and retries.
`service/definition/.../ServiceAccountToken.java`	Loads rotating Kubernetes credentials.
`service/definition/.../StorageObjectMapperWrapper.java`	Wraps the storage-specific mapper.
`service/definition/.../StorageObjectMapperProducer.java`	Produces the configured mapper.
`service/definition/.../inmemory/InMemoryStorage.java`	Removes the old typed implementation.
`service/definition/.../CompressionUtils.java`	Adds gzip stream utilities.
`build/bom/pom.xml`	Publishes the new storage module.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    public static InputStream decompressIfNeeded(InputStream source) throws IOException {
+        var pushback = new PushbackInputStream(source, GZIP_MAGIC_LENGTH);
+        var magic = new byte[GZIP_MAGIC_LENGTH];
+        int read = pushback.read(magic);


+         * 2. Enable case-insensitive property matching as some cloud storages are case-insensitive.
+         */
+        objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+        objectMapper.getDeserializationConfig().with(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES, true);


+        if (hasSolverStatus(item.attributes())) {
+            return mapper().convertValue(item.attributes(), Metadata.class);


+    /**
+     * The value of the {@value #AUTHORIZATION_HEADER} header to send, empty when there is no token to send.
+     */
+    public Optional<String> authorization() {


+    /**
+     * Property that turns the caching of the model output and the metadata on and off.
+     */
+    public static final String USE_CACHE_PROPERTY = "timefold.storage.use-cache";


mswiderski deployed to internal September 25, 2026 07:47 — with GitHub Actions Active

mswiderski force-pushed the byom-security-improvements branch from 27b6e96 to 5282daf Compare September 28, 2026 09:00

mswiderski deployed to internal September 28, 2026 09:00 — with GitHub Actions Active

mswiderski force-pushed the byom-security-improvements branch from 5282daf to d05a2d1 Compare October 1, 2026 06:08

mswiderski deployed to internal October 1, 2026 06:08 — with GitHub Actions Active

rsynek reviewed Oct 1, 2026

View reviewed changes

chore: solver service byom security improvements - refactor to not in…

b715230

…clude storages into models and intorduce single proxy component instead

mswiderski force-pushed the byom-security-improvements branch from d05a2d1 to b715230 Compare October 2, 2026 06:08

mswiderski deployed to internal October 2, 2026 06:09 — with GitHub Actions Active

mswiderski marked this pull request as ready for review October 2, 2026 07:40

mswiderski requested review from a team and a balanced review from Copilot October 2, 2026 07:40

mswiderski requested a review from triceo as a code owner October 2, 2026 07:40

Copilot started reviewing on behalf of mswiderski October 2, 2026 07:41 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Navigation Menu

chore: solver service byom security improvements - refactor to not in… - #2695

chore: solver service byom security improvements - refactor to not in…#2695
mswiderski wants to merge 1 commit into
TimefoldAI:mainfrom
mswiderski:byom-security-improvements

mswiderski commented Sep 25, 2026

Uh oh!

rsynek Oct 1, 2026

Uh oh!

mswiderski Oct 2, 2026

Uh oh!

rsynek Oct 1, 2026

Uh oh!

rsynek Oct 1, 2026

Uh oh!

mswiderski Oct 2, 2026

Uh oh!

rsynek Oct 1, 2026

Uh oh!

mswiderski Oct 2, 2026

Uh oh!

rsynek Oct 1, 2026

Uh oh!

mswiderski Oct 2, 2026

Uh oh!

rsynek Oct 1, 2026

Uh oh!

mswiderski Oct 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Field field = AbstractStorageService.class.getDeclaredField("locks");
		field.setAccessible(true);

		@@ -0,0 +1,109 @@
		package ai.timefold.solver.service.definition.internal.platform;

		if (hasSolverStatus(item.attributes())) {
		return mapper().convertValue(item.attributes(), Metadata.class);

Conversation

mswiderski commented Sep 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants