Skip to content

Removed eager fetch in list - #846

Draft
ivo-ritense wants to merge 4 commits into
next-minorfrom
bugfix/collection-fetch
Draft

Removed eager fetch in list#846
ivo-ritense wants to merge 4 commits into
next-minorfrom
bugfix/collection-fetch

Conversation

@ivo-ritense

Copy link
Copy Markdown
Collaborator

@ivo-ritense
ivo-ritense requested review from a team as code owners July 23, 2026 09:13
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d9ecea2-231f-4e7f-a528-0cc58980ec66

📥 Commits

Reviewing files that changed from the base of the PR and between 53aa0d2 and 1e60546.

📒 Files selected for processing (2)
  • backend/case/src/test/kotlin/com/ritense/document/service/CaseTagServiceIntTest.kt
  • backend/gradle/test.gradle

📝 Walkthrough

Summary by CodeRabbit

  • Enhancements

    • Case lists and document searches now load only the current page of results, improving performance for large result sets.
    • Case tags are reliably included when viewing documents or displaying search and list results.
    • Document reindexing now handles pagination more reliably and continues processing when individual documents cannot be converted.
  • Bug Fixes

    • Fixed missing case-tag data when responses are serialized after database sessions close.

Walkthrough

The change initializes lazily loaded document case tags within transactional retrieval and search methods, adds REST integration coverage for document and search responses, and changes the entity mapping to batched lazy loading. OpenSearch reindexing now converts entities within read-only transactions and advances using the last fetched row. Release notes document page-by-page loading for case lists and searches.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PostgreSQL and MySQL test heap-size changes are unrelated to removing eager collection fetching from paginated list operations. Move the integration-test heap-size changes to a separate PR unless they are required to validate this change.
✅ Passed checks (1 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR removes eager collection fetching, configures lazy batched loading, and initializes caseTags within transactions for paginated list operations.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bugfix/collection-fetch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Klaas-Ritense Klaas-Ritense added the test env Create a remote test environment for this PR in the Ritense product development cluster label Jul 24, 2026
@valtimo-platform

valtimo-platform Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Will create a test environment. This comment will be updated once it is available. This usually takes a few minutes.
Closing or merging this PR will automatically delete the test environment. Pushing commits to this PR will update the test environment.
Progress:

  • Created test environment
  • Images tagged testEnv.pr846_commit.b04845ba26_contents.d1aa8bf15c_build.605 (frontend) / testEnv.pr846_commit.1e6054685f_contents.f2a552c2a3_build.1954 (backend) available
  • Started test environment
  • Test environment is running at https://pr846.product-development.test.k8s.ritense.com

Test environment metadata:

  • URL: https://pr846.product-development.test.k8s.ritense.com
  • Commit: 1e60546
  • Frontend contents: d1aa8bf15c
  • Backend contents: f2a552c2a3
  • Frontend image: ghcr.io/valtimo-platform/valtimo/gzac-frontend:testEnv.pr846_commit.b04845ba26_contents.d1aa8bf15c_build.605
  • Backend image: ghcr.io/valtimo-platform/valtimo/gzac-backend:testEnv.pr846_commit.1e6054685f_contents.f2a552c2a3_build.1954

Observability:

@Klaas-Ritense

Copy link
Copy Markdown
Member

Integration test gives an error

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
backend/case/src/test/java/com/ritense/document/web/rest/JsonSchemaDocumentCaseTagsSerializationIntegrationTest.java (1)

155-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the two document-list paths as well.

This regression test exercises single-document GET and search, but not getAllByDocumentDefinitionName or getAll—the two list methods changed in JsonSchemaDocumentService. Add list-response coverage here, or confirm equivalent existing tests cover lazy caseTags serialization for both methods.

backend/case-opensearch/src/main/kotlin/com/ritense/document/opensearch/service/DocumentOpenSearchReindexService.kt (2)

109-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update the stale "short transaction" doc comment.

The class-level comment on reindex (Lines 109-113) still describes each page as read in a "short read-only transaction," but conversion now happens inside that same transaction, including batch-lazy-loading of caseTags for every document in the batch. This can meaningfully extend how long the transaction (and its DB snapshot/connection) is held, especially for large page sizes. Worth updating the doc to reflect the new duration characteristics, and considering whether page size needs re-tuning now that conversion work is inside the transaction boundary.

Also applies to: 125-155


129-149: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add coverage for partial-batch conversion failures.

DocumentOpenSearchReindexServiceTest.kt only covers lock acquisition/cancellation; add a case where a batch converter throws partway through so the cursor advances past failed documents and the skipped count is recorded.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a58300b7-5b26-4cea-a8f4-335c8e7c7233

📥 Commits

Reviewing files that changed from the base of the PR and between b04845b and 53aa0d2.

📒 Files selected for processing (6)
  • backend/case-opensearch/src/main/kotlin/com/ritense/document/opensearch/service/DocumentOpenSearchReindexService.kt
  • backend/case/src/main/java/com/ritense/document/domain/impl/JsonSchemaDocument.java
  • backend/case/src/main/java/com/ritense/document/service/impl/JsonSchemaDocumentSearchService.java
  • backend/case/src/main/java/com/ritense/document/service/impl/JsonSchemaDocumentService.java
  • backend/case/src/test/java/com/ritense/document/web/rest/JsonSchemaDocumentCaseTagsSerializationIntegrationTest.java
  • documentation/release-notes/13.x.x/13.38.0/README.md

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

13.38.0 has been release already

// serializes lazy associations (e.g. the caseTags @ManyToMany) that would otherwise throw a
// LazyInitializationException once the persistence context is cleared and the transaction closes.
// Only the OpenSearch bulk write below happens outside the transaction.
val page = txTemplate.execute {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not the right PR but lgtm

@Klaas-Ritense

Copy link
Copy Markdown
Member

Test findings:

  • No issues in UI. All tags are still visible
  • Error still appears in logs
2026-08-06T10:08:48.197Z  WARN 1 --- GZAC backend[nio-8080-exec-5] org.hibernate.orm.query                  : HHH90003004: firstResult/maxResults specified with collection fetch; applying in memory valtimoCorrelationId=f17fdb5f-30ce-4c17-81ee-d7e2251b6ed2, valtimoUserId=klaas.schuijtemaker@ritense.com
2026-08-06T10:08:33.442Z  WARN 1 --- GZAC backend[io-8080-exec-39] org.hibernate.orm.query                  : HHH90003004: firstResult/maxResults specified with collection fetch; applying in memory com.ritense.document.domain.impl.JsonSchemaDocument=8f552a7d-e490-44a5-982e-ab1590cb8cc1, valtimoCorrelationId=352d09d0-a41d-4eab-b79c-2c1c0a1282c9, valtimoUserId=klaas.schuijtemaker@ritense.com
2026-08-06T10:07:25.142Z  WARN 1 --- GZAC backend[nio-8080-exec-3] org.hibernate.orm.query                  : HHH90003004: firstResult/maxResults specified with collection fetch; applying in memory com.ritense.document.domain.impl.JsonSchemaDocument=8f552a7d-e490-44a5-982e-ab1590cb8cc1, valtimoCorrelationId=7eb3401a-7b78-473b-b831-ac5ab5c4fc5f, valtimoUserId=klaas.schuijtemaker@ritense.com
2026-08-06T10:07:22.434Z  WARN 1 --- GZAC backend[io-8080-exec-32] org.hibernate.orm.query                  : HHH90003004: firstResult/maxResults specified with collection fetch; applying in memory valtimoCorrelationId=ed349512-9827-42c1-ad4d-248463efeb7f, valtimoUserId=klaas.schuijtemaker@ritense.com
2026-08-06T10:05:42.169Z  WARN 1 --- GZAC backend[nio-8080-exec-9] org.hibernate.orm.query                  : HHH90003004: firstResult/maxResults specified with collection fetch; applying in memory com.ritense.document.domain.impl.JsonSchemaDocument=8f552a7d-e490-44a5-982e-ab1590cb8cc1, valtimoCorrelationId=8d289e2c-e8f5-446a-ba3e-b6a60ef14112, valtimoUserId=klaas.schuijtemaker@ritense.com

@ivo-ritense
ivo-ritense marked this pull request as draft August 11, 2026 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feedback test env Create a remote test environment for this PR in the Ritense product development cluster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collection fetch used in memory

2 participants