-
Notifications
You must be signed in to change notification settings - Fork 0
[Feature] add integration test reproducing the race in competition #599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
antonpampukha
merged 19 commits into
dev
from
feature/add-integration-test-reproducing-the-race-in-competition
Sep 6, 2026
Merged
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
721f6fa
fix: tour can't be removed if stage has only one
antonpampukha 92b3a58
feat: add dependency for testcontainers to pom.xml
antonpampukha 544ba8d
feat: add basic class for integration test & application-test.yml
antonpampukha 9539987
chore: update application-test.yml
antonpampukha 5f3900e
feat(competition): add integration tests
antonpampukha 6a8bef7
feat: add timeout for lock (select for update) to lockCompetitionForU…
antonpampukha 6bf91cc
feat: update HierarchyValidatorTest
antonpampukha ee42a39
feat: add step to build job to main.yml
antonpampukha e71a666
feat: remove @Testcontainers from PostgresIntegrationTest
antonpampukha 24e6939
feat: add competition.hierarchy-lock-timeout-ms property to applicati…
antonpampukha e8be2f8
feat: set lock_timeout in lockCompetitionForUpdate() using entityManager
antonpampukha 0553e5d
Merge branch 'dev' into feature/add-integration-test-reproducing-the-…
antonpampukha 24ede73
fix(dorny): change hash to version & change path
antonpampukha c4b1ee3
Merge remote-tracking branch 'origin/feature/add-integration-test-rep…
antonpampukha a3e1d69
fix: sonar issues
antonpampukha 65b94d9
fix formatter
antonpampukha 142ad63
fix sonar issue
antonpampukha 799ea7a
feat: add comment about PostgresIntegrationTest
antonpampukha b60ab8e
feat: change version number to version (commit) hash to solve CWE-829
antonpampukha File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| spring: | ||
| datasource: | ||
| url: jdbc:postgresql://localhost:5432/test_dummy | ||
| username: test | ||
| password: test | ||
|
|
||
| liquibase: | ||
| enabled: true | ||
| change-log: classpath:db/changelog/db.changelog-master.xml | ||
| drop-first: true | ||
|
|
||
| jpa: | ||
| show-sql: true | ||
| hibernate: | ||
| ddl-auto: validate | ||
| properties: | ||
| jakarta: | ||
| persistence: | ||
| lock: | ||
| timeout: 500 | ||
|
|
||
| mail: | ||
| username: dummy@test.com | ||
| password: dummyPassword | ||
|
|
||
| security: | ||
| oauth2: | ||
| client: | ||
| registration: | ||
| google: | ||
| client-id: dummy-google-id | ||
| client-secret: dummy-google-secret | ||
| microsoft: | ||
| client-id: dummy-microsoft-id | ||
| client-secret: dummy-microsoft-secret | ||
|
|
||
| jwt: | ||
| encrypted-key: dummyEncryptedKey1234567890123456 | ||
| sign-key: dummySignKey123456789012345678901234567890 | ||
|
|
||
| graph: | ||
| tenant-id: dummy-tenant | ||
| client-id: dummy-client | ||
| client-secret: dummy-secret | ||
| drive-id: dummy-drive | ||
|
|
||
| web_client: | ||
| origin: http://localhost:3000 | ||
|
|
||
| competition: | ||
| hierarchy-lock-timeout-ms: ${LOCK_TIMEOUT:1000} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
22 changes: 22 additions & 0 deletions
22
src/test/java/com/itasocialacademy/oitassist/PostgresIntegrationTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package com.itasocialacademy.oitassist; | ||
|
|
||
| import org.junit.jupiter.api.Tag; | ||
| import org.springframework.boot.test.context.SpringBootTest; | ||
| import org.springframework.boot.testcontainers.service.connection.ServiceConnection; | ||
| import org.springframework.test.context.ActiveProfiles; | ||
| import org.testcontainers.containers.PostgreSQLContainer; | ||
| import org.testcontainers.utility.DockerImageName; | ||
|
|
||
| @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) | ||
| @ActiveProfiles("test") | ||
| @Tag("integration") | ||
| public abstract class PostgresIntegrationTest { | ||
|
|
||
| @ServiceConnection | ||
| static final PostgreSQLContainer<?> POSTGRES = | ||
| new PostgreSQLContainer<>(DockerImageName.parse("postgres:16-alpine")); | ||
|
|
||
| static { | ||
| POSTGRES.start(); | ||
| } | ||
| } |
124 changes: 124 additions & 0 deletions
124
...est/java/com/itasocialacademy/oitassist/competition/service/CompetitionLockTimeoutIT.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| package com.itasocialacademy.oitassist.competition.service; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import com.itasocialacademy.oitassist.PostgresIntegrationTest; | ||
| import com.itasocialacademy.oitassist.competition.dao.enums.CompetitionStatus; | ||
| import com.itasocialacademy.oitassist.competition.dao.enums.ExecutionStatus; | ||
| import com.itasocialacademy.oitassist.competition.dao.enums.StageScope; | ||
| import com.itasocialacademy.oitassist.competition.dao.enums.StageStatus; | ||
| import com.itasocialacademy.oitassist.competition.dao.model.Competition; | ||
| import com.itasocialacademy.oitassist.competition.dao.model.Stage; | ||
| import com.itasocialacademy.oitassist.competition.dao.model.Tour; | ||
| import com.itasocialacademy.oitassist.competition.dao.repository.CompetitionRepository; | ||
| import com.itasocialacademy.oitassist.competition.dao.repository.StageRepository; | ||
| import com.itasocialacademy.oitassist.competition.dao.repository.TourRepository; | ||
| import com.itasocialacademy.oitassist.competition.dto.request.ChangeCompetitionStatusRequest; | ||
| import com.itasocialacademy.oitassist.competition.service.interfaces.CompetitionService; | ||
| import jakarta.persistence.EntityManager; | ||
| import jakarta.persistence.LockModeType; | ||
| import jakarta.persistence.PersistenceContext; | ||
| import java.time.ZonedDateTime; | ||
| import java.util.concurrent.CountDownLatch; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.Future; | ||
| import java.util.concurrent.TimeUnit; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.dao.PessimisticLockingFailureException; | ||
| import org.springframework.orm.jpa.JpaTransactionManager; | ||
| import org.springframework.transaction.support.TransactionTemplate; | ||
|
|
||
| class CompetitionLockTimeoutIT extends PostgresIntegrationTest { | ||
|
|
||
| @PersistenceContext | ||
| private EntityManager entityManager; | ||
| @Autowired | ||
| private JpaTransactionManager transactionManager; | ||
| @Autowired | ||
| private CompetitionRepository competitionRepository; | ||
| @Autowired | ||
| private StageRepository stageRepository; | ||
| @Autowired | ||
| private TourRepository tourRepository; | ||
| @Autowired | ||
| private CompetitionService competitionService; | ||
|
|
||
| private ExecutorService executor; | ||
|
|
||
| @AfterEach | ||
| void tearDown() { | ||
| if (executor != null) { | ||
| executor.shutdownNow(); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void lockHeldLongerThanTimeout_shouldSurfaceAsPessimisticLockingFailure() throws Exception { | ||
| executor = Executors.newFixedThreadPool(1); | ||
|
|
||
| Competition competition = competitionRepository.save(Competition.builder() | ||
| .title("Timeout Test") | ||
| .dateStart(ZonedDateTime.now().plusDays(1)) | ||
| .dateFinish(ZonedDateTime.now().plusDays(10)) | ||
| .competitionStatus(CompetitionStatus.DRAFT) | ||
| .createdBy(1L) | ||
| .build()); | ||
|
|
||
| Stage stage = stageRepository.save(Stage.builder() | ||
| .competitionId(competition.getId()) | ||
| .title("Stage") | ||
| .dateStart(competition.getDateStart()) | ||
| .dateFinish(competition.getDateFinish()) | ||
| .sortPosition((short) 1) | ||
| .scope(StageScope.CITY) | ||
| .status(StageStatus.SCHEDULED) | ||
| .createdBy(1L) | ||
| .build()); | ||
|
|
||
| tourRepository.save(Tour.builder() | ||
| .stageId(stage.getId()) | ||
| .title("Tour") | ||
| .dateStart(competition.getDateStart()) | ||
| .dateFinish(competition.getDateFinish()) | ||
| .sortPosition((short) 1) | ||
| .executionStatus(ExecutionStatus.SCHEDULED) | ||
| .location("location") | ||
| .createdBy(1L) | ||
| .build()); | ||
|
|
||
| TransactionTemplate holderTx = new TransactionTemplate(transactionManager); | ||
| CountDownLatch lockAcquired = new CountDownLatch(1); | ||
| CountDownLatch releaseHolder = new CountDownLatch(1); | ||
|
|
||
| Future<?> holderFuture = executor.submit(() -> holderTx.executeWithoutResult(_ -> { | ||
| entityManager.find(Competition.class, competition.getId(), LockModeType.PESSIMISTIC_WRITE); | ||
|
|
||
| lockAcquired.countDown(); | ||
| try { | ||
| releaseHolder.await(); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| throw new RuntimeException(e); | ||
| } | ||
| })); | ||
|
|
||
| assertTrue(lockAcquired.await(5, TimeUnit.SECONDS), "Holder thread should acquire the lock"); | ||
|
|
||
| ChangeCompetitionStatusRequest request = | ||
| new ChangeCompetitionStatusRequest(CompetitionStatus.ENROLLMENT, competition.getVersion()); | ||
|
|
||
| Long id = competition.getId(); | ||
| assertNotNull(id); | ||
|
|
||
| assertThrows(PessimisticLockingFailureException.class, | ||
| () -> competitionService.changeStatus(id, request)); | ||
|
|
||
| releaseHolder.countDown(); | ||
| holderFuture.get(5, TimeUnit.SECONDS); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: ita-social-projects/oitAssist
Length of output: 10867
🌐 Web query:
dorny/test-reporter v1.9.1 required permissions checks write actions read contents read💡 Result:
The dorny/test-reporter action (including version 1.9.1) typically requires the following permissions in your workflow configuration to function correctly when creating Check Runs [1][2][3]: permissions: contents: read actions: read checks: write These permissions are necessary because the action needs to access repository contents, download test result artifacts (via actions: read), and create check runs (via checks: write) [2]. Important Considerations: 1. Forked Repository Security: Workflows triggered by pull requests from forked repositories are executed with a read-only token by default, which prevents the creation of Check Runs [1][4]. A common workaround is to use a two-workflow pattern: - A CI workflow that runs on the PR branch (with read-only tokens) to execute tests and upload results as build artifacts [1][2]. - A Test Report workflow that runs on the main branch (using workflow_run) with the necessary read/write permissions to download the artifacts and create the reports [1][2][3]. 2. Alternative Approaches: If you do not want to use the checks: write permission, you may be able to use the use-actions-summary: true option, which can generate a report summary in the workflow execution logs without requiring the write-level check permission [2][5]. 3. Token Permissions: If your repository settings are configured for default read-only tokens, you must explicitly define the permissions block in your YAML file as shown above to grant the action the required access [2][6]. Failure to do so will often result in an error like "Resource not accessible by integration" [7].
Citations:
🤖 get_repo_knowledge executed:
get_repo_knowledge ita-social-projects/oitAssist /tmp/coderabbit-repo-knowledge/ita-social-projects-oitassist-2ebd8cd8/conventionsLength of output: 5655
Security Misconfiguration (CWE-250)
Reachability: External · Exploitability: Difficult
Declare the
buildjob permissions explicitly.The
buildjob runsdorny/test-reporterwithout apermissionsblock. Grant only the scopes required by the reporter, such ascontents: read,actions: read, andchecks: write.Suggested permissions
jobs: build: runs-on: ubuntu-latest + permissions: + contents: read + actions: read + checks: write🧰 Tools
🪛 zizmor (1.29.0)
[warning] 1-172: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 17-49: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Source: Linters/SAST tools
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: ita-social-projects/oitAssist
Length of output: 2838
🤖 get_repo_knowledge executed:
get_repo_knowledge ita-social-projects/oitAssist /tmp/coderabbit-repo-knowledge/ita-social-projects-oitassist-2ebd8cd8/conventionsLength of output: 5655
🏁 Script executed:
Repository: ita-social-projects/oitAssist
Length of output: 350
Other (CWE-829): Inclusion of Functionality from Untrusted Control Sphere
Reachability: External · Exploitability: Difficult
Pin
dorny/test-reporterto commit31a54ee7ebcacc03a09ea97a7e5465a47b84aea5.v1.9.1is mutable. Use the full commit SHA and configure dependency automation for updates.🧰 Tools
🪛 zizmor (1.29.0)
[warning] 1-172: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[warning] 17-49: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents