This is an automated email from the ASF dual-hosted git repository.
ilgrosso pushed a commit to branch 4_0_X
in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/4_0_X by this push:
new 59909ffeb7 [SYNCOPE-1919] Ensuring that JobStatusUpdater can only
update existing rows (#1199)
59909ffeb7 is described below
commit 59909ffeb7d0b783fd7b292902c43631d36f8d38
Author: Francesco Chicchiriccò <[email protected]>
AuthorDate: Wed Oct 1 10:33:24 2025 +0200
[SYNCOPE-1919] Ensuring that JobStatusUpdater can only update existing rows
(#1199)
---
.../syncope/core/logic/AbstractJobLogic.java | 3 +-
.../core/persistence/api/dao/JobStatusDAO.java | 10 ++-
.../core/persistence/jpa/dao/JPAJobStatusDAO.java | 86 ++++++++--------------
.../persistence/neo4j/dao/Neo4jJobStatusDAO.java | 64 +++++-----------
.../provisioning/java/ProvisioningContext.java | 4 +-
.../provisioning/java/job/JobStatusUpdater.java | 20 +----
.../java/job/JobStatusUpdaterTest.java | 19 +++--
.../core/src/main/resources/core-oracle.properties | 2 -
.../docker-compose/docker-compose-oracle.yml | 4 +-
9 files changed, 76 insertions(+), 136 deletions(-)
diff --git
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AbstractJobLogic.java
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AbstractJobLogic.java
index e32b9b0c43..7b3b33c1dd 100644
---
a/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AbstractJobLogic.java
+++
b/core/idrepo/logic/src/main/java/org/apache/syncope/core/logic/AbstractJobLogic.java
@@ -28,7 +28,6 @@ import org.apache.syncope.common.lib.to.JobTO;
import org.apache.syncope.common.lib.types.JobAction;
import org.apache.syncope.common.lib.types.JobType;
import org.apache.syncope.core.persistence.api.dao.JobStatusDAO;
-import org.apache.syncope.core.persistence.api.entity.JobStatus;
import org.apache.syncope.core.provisioning.api.job.JobManager;
import org.apache.syncope.core.provisioning.java.job.SyncopeTaskScheduler;
import org.apache.syncope.core.provisioning.java.job.SystemLoadReporterJob;
@@ -91,7 +90,7 @@ abstract class AbstractJobLogic<T extends EntityTO> extends
AbstractTransactiona
jobTO.setRunning(jobManager.isRunning(jobName));
-
jobTO.setStatus(jobStatusDAO.findById(jobName).map(JobStatus::getStatus).orElse("UNKNOWN"));
+ jobTO.setStatus(jobStatusDAO.get(jobName));
}
return Optional.ofNullable(jobTO);
diff --git
a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/JobStatusDAO.java
b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/JobStatusDAO.java
index 40114748d7..0fa09e7aa8 100644
---
a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/JobStatusDAO.java
+++
b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/dao/JobStatusDAO.java
@@ -18,13 +18,17 @@
*/
package org.apache.syncope.core.persistence.api.dao;
-import org.apache.syncope.core.persistence.api.entity.JobStatus;
-
-public interface JobStatusDAO extends DAO<JobStatus> {
+public interface JobStatusDAO {
String JOB_FIRED_STATUS = "JOB_FIRED";
+ String UNKNOWN_STATUS = "UNKNOWN";
+
boolean lock(String key);
void unlock(String key);
+
+ void set(String key, String status);
+
+ String get(String key);
}
diff --git
a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAJobStatusDAO.java
b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAJobStatusDAO.java
index 761347e301..520808b525 100644
---
a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAJobStatusDAO.java
+++
b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/JPAJobStatusDAO.java
@@ -20,11 +20,8 @@ package org.apache.syncope.core.persistence.jpa.dao;
import jakarta.persistence.EntityManager;
import jakarta.persistence.Query;
-import jakarta.persistence.TypedQuery;
import java.util.List;
-import java.util.Optional;
import org.apache.syncope.core.persistence.api.dao.JobStatusDAO;
-import org.apache.syncope.core.persistence.api.entity.JobStatus;
import org.apache.syncope.core.persistence.jpa.entity.JPAJobStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -41,64 +38,21 @@ public class JPAJobStatusDAO implements JobStatusDAO {
this.entityManager = entityManager;
}
- @Transactional(readOnly = true)
- @Override
- public boolean existsById(final String key) {
- Query query = entityManager.createQuery(
- "SELECT COUNT(e) FROM " + JPAJobStatus.class.getSimpleName() +
" e WHERE e.id = :key");
- query.setParameter("key", key);
- return ((Number) query.getSingleResult()).longValue() > 0;
- }
-
- @Transactional(readOnly = true)
- @Override
- public Optional<? extends JobStatus> findById(final String key) {
- return Optional.ofNullable(entityManager.find(JPAJobStatus.class,
key));
- }
-
- @Transactional(readOnly = true)
- @Override
- public long count() {
- Query query = entityManager.createQuery(
- "SELECT COUNT(e) FROM " + JPAJobStatus.class.getSimpleName() +
" e");
- return ((Number) query.getSingleResult()).longValue();
- }
-
- @Transactional(readOnly = true)
- @Override
- public List<? extends JobStatus> findAll() {
- TypedQuery<JobStatus> query = entityManager.createQuery(
- "SELECT e FROM " + JPAJobStatus.class.getSimpleName() + " e",
JobStatus.class);
- return query.getResultList();
- }
-
- @Override
- public <S extends JobStatus> S save(final S jobStatus) {
- return entityManager.merge(jobStatus);
- }
-
- @Override
- public void delete(final JobStatus jobStatus) {
- entityManager.remove(jobStatus);
- }
-
- @Override
- public void deleteById(final String key) {
- findById(key).ifPresent(this::delete);
- }
-
@Override
public boolean lock(final String key) {
- if (existsById(key)) {
+ if (!UNKNOWN_STATUS.equals(get(key))) {
+ LOG.debug("Job {} already locked", key);
return false;
}
try {
- JobStatus jobStatus = new JPAJobStatus();
- jobStatus.setKey(key);
- jobStatus.setStatus(JOB_FIRED_STATUS);
- save(jobStatus);
+ Query query = entityManager.createNativeQuery(
+ "INSERT INTO " + JPAJobStatus.TABLE + "(id, jobStatus)
VALUES (?,?)");
+ query.setParameter(1, key);
+ query.setParameter(2, JOB_FIRED_STATUS);
+ query.executeUpdate();
+ LOG.debug("Job {} locked", key);
return true;
} catch (Exception e) {
LOG.debug("Could not lock job {}", key, e);
@@ -108,6 +62,28 @@ public class JPAJobStatusDAO implements JobStatusDAO {
@Override
public void unlock(final String key) {
- deleteById(key);
+ Query query = entityManager.createNativeQuery("DELETE FROM " +
JPAJobStatus.TABLE + " WHERE id=?");
+ query.setParameter(1, key);
+ query.executeUpdate();
+ LOG.debug("Job {} unlocked", key);
+ }
+
+ @Override
+ public void set(final String key, final String status) {
+ Query query = entityManager.createNativeQuery("UPDATE " +
JPAJobStatus.TABLE + " SET jobStatus=? WHERE id=?");
+ query.setParameter(1, UNKNOWN_STATUS.equals(status) ? "Status " +
UNKNOWN_STATUS : status);
+ query.setParameter(2, key);
+ query.executeUpdate();
+ }
+
+ @Transactional(readOnly = true)
+ @Override
+ public String get(final String key) {
+ Query query = entityManager.createNativeQuery("SELECT jobStatus FROM "
+ JPAJobStatus.TABLE + " WHERE id=?");
+ query.setParameter(1, key);
+
+ @SuppressWarnings("unchecked")
+ List<Object> result = query.getResultList();
+ return result.isEmpty() ? UNKNOWN_STATUS :
result.getFirst().toString();
}
}
diff --git
a/core/persistence-neo4j/src/main/java/org/apache/syncope/core/persistence/neo4j/dao/Neo4jJobStatusDAO.java
b/core/persistence-neo4j/src/main/java/org/apache/syncope/core/persistence/neo4j/dao/Neo4jJobStatusDAO.java
index 62d7948183..f283de6529 100644
---
a/core/persistence-neo4j/src/main/java/org/apache/syncope/core/persistence/neo4j/dao/Neo4jJobStatusDAO.java
+++
b/core/persistence-neo4j/src/main/java/org/apache/syncope/core/persistence/neo4j/dao/Neo4jJobStatusDAO.java
@@ -18,8 +18,6 @@
*/
package org.apache.syncope.core.persistence.neo4j.dao;
-import java.util.List;
-import java.util.Optional;
import org.apache.syncope.core.persistence.api.dao.JobStatusDAO;
import org.apache.syncope.core.persistence.api.entity.JobStatus;
import org.apache.syncope.core.persistence.neo4j.entity.Neo4jJobStatus;
@@ -43,48 +41,10 @@ public class Neo4jJobStatusDAO implements JobStatusDAO {
this.nodeValidator = nodeValidator;
}
- @Transactional(readOnly = true)
- @Override
- public boolean existsById(final String key) {
- return neo4jTemplate.existsById(key, Neo4jJobStatus.class);
- }
-
- @Transactional(readOnly = true)
- @Override
- public Optional<? extends JobStatus> findById(final String key) {
- return neo4jTemplate.findById(key, Neo4jJobStatus.class);
- }
-
- @Transactional(readOnly = true)
- @Override
- public long count() {
- return neo4jTemplate.count(Neo4jJobStatus.class);
- }
-
- @Transactional(readOnly = true)
- @Override
- public List<? extends JobStatus> findAll() {
- return neo4jTemplate.findAll(Neo4jJobStatus.class);
- }
-
- @Override
- public <S extends JobStatus> S save(final S jobStatus) {
- return neo4jTemplate.save(nodeValidator.validate(jobStatus));
- }
-
- @Override
- public void delete(final JobStatus jobStatus) {
- neo4jTemplate.deleteById(jobStatus.getKey(), Neo4jJobStatus.class);
- }
-
- @Override
- public void deleteById(final String key) {
- findById(key).ifPresent(this::delete);
- }
-
@Override
public boolean lock(final String key) {
- if (existsById(key)) {
+ if (neo4jTemplate.existsById(key, Neo4jJobStatus.class)) {
+ LOG.debug("Job {} already locked", key);
return false;
}
@@ -92,7 +52,7 @@ public class Neo4jJobStatusDAO implements JobStatusDAO {
JobStatus jobStatus = new Neo4jJobStatus();
jobStatus.setKey(key);
jobStatus.setStatus(JOB_FIRED_STATUS);
- save(jobStatus);
+ neo4jTemplate.save(nodeValidator.validate(jobStatus));
return true;
} catch (Exception e) {
@@ -103,6 +63,22 @@ public class Neo4jJobStatusDAO implements JobStatusDAO {
@Override
public void unlock(final String key) {
- deleteById(key);
+ neo4jTemplate.deleteById(key, Neo4jJobStatus.class);
+ }
+
+ @Override
+ public void set(final String key, final String status) {
+ if (neo4jTemplate.existsById(key, Neo4jJobStatus.class)) {
+ JobStatus jobStatus = new Neo4jJobStatus();
+ jobStatus.setKey(key);
+ jobStatus.setStatus(status);
+ neo4jTemplate.save(nodeValidator.validate(jobStatus));
+ }
+ }
+
+ @Transactional(readOnly = true)
+ @Override
+ public String get(final String key) {
+ return neo4jTemplate.findById(key,
Neo4jJobStatus.class).map(JobStatus::getStatus).orElse(UNKNOWN_STATUS);
}
}
diff --git
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java
index 88a3d2fcad..d16b7d349b 100644
---
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java
+++
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/ProvisioningContext.java
@@ -291,8 +291,8 @@ public class ProvisioningContext {
@ConditionalOnMissingBean
@Bean
- public JobStatusUpdater jobStatusUpdater(final JobStatusDAO jobStatusDAO,
final EntityFactory entityFactory) {
- return new JobStatusUpdater(jobStatusDAO, entityFactory);
+ public JobStatusUpdater jobStatusUpdater(final JobStatusDAO jobStatusDAO) {
+ return new JobStatusUpdater(jobStatusDAO);
}
@ConditionalOnMissingBean
diff --git
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdater.java
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdater.java
index e1668fe935..24e81d1f66 100644
---
a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdater.java
+++
b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdater.java
@@ -19,8 +19,6 @@
package org.apache.syncope.core.provisioning.java.job;
import org.apache.syncope.core.persistence.api.dao.JobStatusDAO;
-import org.apache.syncope.core.persistence.api.entity.EntityFactory;
-import org.apache.syncope.core.persistence.api.entity.JobStatus;
import org.apache.syncope.core.provisioning.api.event.JobStatusEvent;
import org.apache.syncope.core.spring.security.AuthContextUtils;
import org.slf4j.Logger;
@@ -34,13 +32,10 @@ public class JobStatusUpdater {
protected final JobStatusDAO jobStatusDAO;
- protected final EntityFactory entityFactory;
-
protected boolean initCompleted = false;
- public JobStatusUpdater(final JobStatusDAO jobStatusDAO, final
EntityFactory entityFactory) {
+ public JobStatusUpdater(final JobStatusDAO jobStatusDAO) {
this.jobStatusDAO = jobStatusDAO;
- this.entityFactory = entityFactory;
}
public void initComplete() {
@@ -69,16 +64,9 @@ public class JobStatusUpdater {
LOG.debug("Updating job '{}#{}' with status '{}'",
event.getDomain(), event.getJobName(),
event.getJobStatus());
- AuthContextUtils.runAsAdmin(event.getDomain(), () -> {
- JobStatus jobStatus =
jobStatusDAO.findById(event.getJobName()).orElse(null);
- if (jobStatus == null) {
- jobStatus = entityFactory.newEntity(JobStatus.class);
- jobStatus.setKey(event.getJobName());
- }
-
- jobStatus.setStatus(event.getJobStatus());
- jobStatusDAO.save(jobStatus);
- });
+ AuthContextUtils.runAsAdmin(
+ event.getDomain(),
+ () -> jobStatusDAO.set(event.getJobName(),
event.getJobStatus()));
}
}
}
diff --git
a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdaterTest.java
b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdaterTest.java
index bdf7078d55..807ad300c7 100644
---
a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdaterTest.java
+++
b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/job/JobStatusUpdaterTest.java
@@ -18,11 +18,10 @@
*/
package org.apache.syncope.core.provisioning.java.job;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import org.apache.syncope.common.lib.SyncopeConstants;
import org.apache.syncope.core.persistence.api.dao.JobStatusDAO;
-import org.apache.syncope.core.persistence.api.entity.EntityFactory;
import org.apache.syncope.core.provisioning.api.event.JobStatusEvent;
import org.apache.syncope.core.provisioning.java.AbstractTest;
import org.apache.syncope.core.spring.security.SecureRandomUtils;
@@ -33,9 +32,6 @@ import
org.springframework.transaction.annotation.Transactional;
@Transactional
public class JobStatusUpdaterTest extends AbstractTest {
- @Autowired
- private EntityFactory entityFactory;
-
@Autowired
private JobStatusDAO jobStatusDAO;
@@ -43,14 +39,17 @@ public class JobStatusUpdaterTest extends AbstractTest {
public void verifyUpdate() {
String jobName = "job-" + SecureRandomUtils.generateRandomNumber();
- JobStatusUpdater jobStatusUpdater = new JobStatusUpdater(jobStatusDAO,
entityFactory);
+ JobStatusUpdater jobStatusUpdater = new JobStatusUpdater(jobStatusDAO);
jobStatusUpdater.initComplete();
jobStatusUpdater.update(new JobStatusEvent(this,
SyncopeConstants.MASTER_DOMAIN, jobName, "Started"));
- assertTrue(jobStatusDAO.findById(jobName).isPresent());
+ assertEquals(JobStatusDAO.UNKNOWN_STATUS, jobStatusDAO.get(jobName));
+
+ jobStatusDAO.lock(jobName);
+ jobStatusUpdater.update(new JobStatusEvent(this,
SyncopeConstants.MASTER_DOMAIN, jobName, "Started"));
+ assertEquals("Started", jobStatusDAO.get(jobName));
- jobStatusUpdater.update(new JobStatusEvent(this,
SyncopeConstants.MASTER_DOMAIN, jobName, null));
- // no change is expected
- assertTrue(jobStatusDAO.findById(jobName).isPresent());
+ jobStatusDAO.unlock(jobName);
+ assertEquals(JobStatusDAO.UNKNOWN_STATUS, jobStatusDAO.get(jobName));
}
}
diff --git a/docker/core/src/main/resources/core-oracle.properties
b/docker/core/src/main/resources/core-oracle.properties
index b92ee09975..1df1c6e56e 100644
--- a/docker/core/src/main/resources/core-oracle.properties
+++ b/docker/core/src/main/resources/core-oracle.properties
@@ -28,5 +28,3 @@
persistence.domain[0].databasePlatform=org.apache.openjpa.jdbc.sql.OracleDiction
persistence.domain[0].orm=META-INF/oracle/spring-orm.xml
persistence.domain[0].poolMaxActive=${DB_POOL_MAX}
persistence.domain[0].poolMinIdle=${DB_POOL_MIN}
-
-persistence.indexesXML=classpath:oracle_indexes.xml
diff --git a/docker/src/main/resources/docker-compose/docker-compose-oracle.yml
b/docker/src/main/resources/docker-compose/docker-compose-oracle.yml
index 5954cc31c8..cf67fecd90 100644
--- a/docker/src/main/resources/docker-compose/docker-compose-oracle.yml
+++ b/docker/src/main/resources/docker-compose/docker-compose-oracle.yml
@@ -29,14 +29,14 @@ services:
syncope:
depends_on:
- db
- command: ["wait-for-it", "db:3306", "-t", "60", "--",
"/opt/syncope/bin/startup.sh"]
+ command: ["wait-for-it", "db:1521", "-t", "60", "--",
"/opt/syncope/bin/startup.sh"]
image: apache/syncope:${SYNCOPE_VERSION}
ports:
- "18080:8080"
restart: always
environment:
SPRING_PROFILES_ACTIVE: docker,oracle,saml2
- DB_URL: jdbc:oracle:thin:@db:1521/XEPDB1
+ DB_URL: jdbc:oracle:thin:@db:1521/FREEPDB1
DB_SCHEMA: SYNCOPE
DB_USER: syncope
DB_PASSWORD: syncope
