Issue/247 Duplicate Resources

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/authorization/StructureDefinitionAuthorizationRule.java

@@ -73,6 +73,10 @@ private Optional<String> newResourceOk(Connection connection, StructureDefinitio
 		{
 			errors.add("StructureDefinition.version not defined");
 		}
+		if (!newResource.hasBaseDefinition())
+		{
+			errors.add("StructureDefinition.baseDefinition not defined");
+		}
 
 		if (!hasValidReadAccessTag(connection, newResource))
 		{

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/AbstractCommandList.java

@@ -16,7 +16,7 @@
 import jakarta.ws.rs.core.Response.Status;
 import jakarta.ws.rs.core.Response.Status.Family;
 
-class AbstractCommandList
+abstract class AbstractCommandList
 {
 	private static final Logger audit = LoggerFactory.getLogger("dsf-audit-logger");
 
@@ -119,7 +119,7 @@ protected BundleEntryComponent toEntry(Exception exception)
 		if (!(exception instanceof WebApplicationException w)
 				|| !(w.getResponse().getEntity() instanceof OperationOutcome))
 		{
-			exception = exceptionHandler.internalServerErrorBundleBatch(exception);
+			exception = internalServerError(exception);
 		}
 
 		Response httpResponse = ((WebApplicationException) exception).getResponse();
@@ -129,4 +129,6 @@ protected BundleEntryComponent toEntry(Exception exception)
 
 		return entry;
 	}
+
+	protected abstract Exception internalServerError(Exception exception);
 }

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/AuthorizationHelper.java

@@ -6,19 +6,23 @@
 import org.hl7.fhir.r4.model.Resource;
 
 import dev.dsf.common.auth.conf.Identity;
+import jakarta.ws.rs.WebApplicationException;
 
 public interface AuthorizationHelper
 {
-	void checkCreateAllowed(int index, Connection connection, Identity identity, Resource newResource);
+	void checkCreateAllowed(int index, Connection connection, Identity identity, Resource newResource)
+			throws WebApplicationException;
 
-	void checkReadAllowed(int index, Connection connection, Identity identity, Resource existingResource);
+	void checkReadAllowed(int index, Connection connection, Identity identity, Resource existingResource)
+			throws WebApplicationException;
 
 	void checkUpdateAllowed(int index, Connection connection, Identity identity, Resource oldResource,
-			Resource newResource);
+			Resource newResource) throws WebApplicationException;
 
-	void checkDeleteAllowed(int index, Connection connection, Identity identity, Resource oldResource);
+	void checkDeleteAllowed(int index, Connection connection, Identity identity, Resource oldResource)
+			throws WebApplicationException;
 
-	void checkSearchAllowed(int index, Identity identity, String resourceTypeName);
+	void checkSearchAllowed(int index, Identity identity, String resourceTypeName) throws WebApplicationException;
 
 	void filterIncludeResults(int index, Connection connection, Identity identity, Bundle multipleResult);
 }

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/AuthorizationHelperImpl.java

@@ -54,6 +54,7 @@ private WebApplicationException forbidden(String operation, Identity identity) t
 
 	@Override
 	public void checkCreateAllowed(int index, Connection connection, Identity identity, Resource newResource)
+			throws WebApplicationException
 	{
 		final String resourceTypeName = getResourceTypeName(newResource);
 
@@ -77,6 +78,7 @@ private String getResourceTypeName(Resource resource)
 
 	@Override
 	public void checkReadAllowed(int index, Connection connection, Identity identity, Resource existingResource)
+			throws WebApplicationException
 	{
 		final String resourceTypeName = getResourceTypeName(existingResource);
 		final String resourceId = existingResource.getIdElement().getIdPart();
@@ -98,7 +100,7 @@ public void checkReadAllowed(int index, Connection connection, Identity identity
 
 	@Override
 	public void checkUpdateAllowed(int index, Connection connection, Identity identity, Resource oldResource,
-			Resource newResource)
+			Resource newResource) throws WebApplicationException
 	{
 		final String resourceTypeName = getResourceTypeName(oldResource);
 		final String resourceId = oldResource.getIdElement().getIdPart();
@@ -121,6 +123,7 @@ public void checkUpdateAllowed(int index, Connection connection, Identity identi
 
 	@Override
 	public void checkDeleteAllowed(int index, Connection connection, Identity identity, Resource oldResource)
+			throws WebApplicationException
 	{
 		final String resourceTypeName = getResourceTypeName(oldResource);
 		final String resourceId = oldResource.getIdElement().getIdPart();
@@ -140,7 +143,7 @@ public void checkDeleteAllowed(int index, Connection connection, Identity identi
 	}
 
 	@Override
-	public void checkSearchAllowed(int index, Identity identity, String resourceTypeName)
+	public void checkSearchAllowed(int index, Identity identity, String resourceTypeName) throws WebApplicationException
 	{
 		Optional<AuthorizationRule<Resource>> optRule = getAuthorizationRule(resourceTypeName);
 		optRule.flatMap(rule -> rule.reasonSearchAllowed(identity)).ifPresentOrElse(reason ->

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/BatchCommandList.java

@@ -16,11 +16,14 @@
 import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent;
 import org.hl7.fhir.r4.model.Bundle.BundleType;
 import org.hl7.fhir.r4.model.IdType;
+import org.postgresql.util.PSQLException;
+import org.postgresql.util.PSQLState;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import dev.dsf.fhir.event.EventHandler;
 import dev.dsf.fhir.help.ExceptionHandler;
+import dev.dsf.fhir.help.ResponseGenerator;
 import dev.dsf.fhir.validation.SnapshotGenerator;
 import jakarta.ws.rs.WebApplicationException;
 
@@ -31,15 +34,18 @@ public class BatchCommandList extends AbstractCommandList implements CommandList
 	private final ValidationHelper validationHelper;
 	private final SnapshotGenerator snapshotGenerator;
 	private final EventHandler eventHandler;
+	private final ResponseGenerator responseGenerator;
 
 	public BatchCommandList(DataSource dataSource, ExceptionHandler exceptionHandler, List<? extends Command> commands,
-			ValidationHelper validationHelper, SnapshotGenerator snapshotGenerator, EventHandler eventHandler)
+			ValidationHelper validationHelper, SnapshotGenerator snapshotGenerator, EventHandler eventHandler,
+			ResponseGenerator responseGenerator)
 	{
 		super(dataSource, exceptionHandler, commands);
 
 		this.validationHelper = validationHelper;
 		this.snapshotGenerator = snapshotGenerator;
 		this.eventHandler = eventHandler;
+		this.responseGenerator = responseGenerator;
 	}
 
 	@Override
@@ -50,23 +56,15 @@ public Bundle execute() throws WebApplicationException
 			boolean initialReadOnly = connection.isReadOnly();
 			boolean initialAutoCommit = connection.getAutoCommit();
 			int initialTransactionIsolationLevel = connection.getTransactionIsolation();
-			logger.debug(
-					"Running batch with DB connection setting: read-only {}, auto-commit {}, transaction-isolation-level {}",
-					initialReadOnly, initialAutoCommit,
-					getTransactionIsolationLevelString(initialTransactionIsolationLevel));
 
 			Map<Integer, Exception> caughtExceptions = new HashMap<>((int) (commands.size() / 0.75) + 1);
 			Map<String, IdType> idTranslationTable = new HashMap<>();
 
 			if (hasModifyingCommands)
 			{
-				logger.debug(
-						"Elevating DB connection setting to: read-only {}, auto-commit {}, transaction-isolation-level {}",
-						false, false, getTransactionIsolationLevelString(Connection.TRANSACTION_REPEATABLE_READ));
-
 				connection.setReadOnly(false);
 				connection.setAutoCommit(false);
-				connection.setTransactionIsolation(Connection.TRANSACTION_REPEATABLE_READ);
+				connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
 			}
 
 			commands.forEach(preExecute(idTranslationTable, connection, caughtExceptions));
@@ -75,11 +73,6 @@ public Bundle execute() throws WebApplicationException
 
 			if (hasModifyingCommands)
 			{
-				logger.debug(
-						"Reseting DB connection setting to: read-only {}, auto-commit {}, transaction-isolation-level {}",
-						initialReadOnly, initialAutoCommit,
-						getTransactionIsolationLevelString(initialTransactionIsolationLevel));
-
 				connection.setReadOnly(initialReadOnly);
 				connection.setAutoCommit(initialAutoCommit);
 				connection.setTransactionIsolation(initialTransactionIsolationLevel);
@@ -110,20 +103,6 @@ public Bundle execute() throws WebApplicationException
 		}
 	}
 
-	private String getTransactionIsolationLevelString(int level)
-	{
-		return switch (level)
-		{
-			case Connection.TRANSACTION_NONE -> "NONE";
-			case Connection.TRANSACTION_READ_UNCOMMITTED -> "READ_UNCOMMITTED";
-			case Connection.TRANSACTION_READ_COMMITTED -> "READ_COMMITTED";
-			case Connection.TRANSACTION_REPEATABLE_READ -> "REPEATABLE_READ";
-			case Connection.TRANSACTION_SERIALIZABLE -> "SERIALIZABLE";
-
-			default -> "?";
-		};
-	}
-
 	private Consumer<Command> preExecute(Map<String, IdType> idTranslationTable, Connection connection,
 			Map<Integer, Exception> caughtExceptions)
 	{
@@ -188,7 +167,12 @@ private Consumer<Command> execute(Map<String, IdType> idTranslationTable, Connec
 				logger.warn("Error while executing command {}, rolling back transaction for entry at index {}: {} - {}",
 						command.getClass().getName(), command.getIndex(), e.getClass().getName(), e.getMessage());
 
-				caughtExceptions.put(command.getIndex(), e);
+				if (e instanceof PSQLException s && PSQLState.UNIQUE_VIOLATION.getState().equals(s.getSQLState()))
+					caughtExceptions.put(command.getIndex(),
+							new WebApplicationException(responseGenerator.dupicateResourceExists()));
+				else
+					caughtExceptions.put(command.getIndex(), e);
+
 
 				try
 				{
@@ -244,4 +228,10 @@ private Consumer<Command> postExecute(Connection connection, Map<Integer, Except
 			}
 		};
 	}
+
+	@Override
+	protected Exception internalServerError(Exception exception)
+	{
+		return exceptionHandler.internalServerErrorBundleBatch(exception);
+	}
 }

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/Command.java

@@ -23,7 +23,6 @@ public interface Command
 
 	default void preExecute(Map<String, IdType> idTranslationTable, Connection connection,
 			ValidationHelper validationHelper, SnapshotGenerator snapshotGenerator)
-
 	{
 	}
 

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/CommandFactoryImpl.java

@@ -210,10 +210,10 @@ public CommandList createCommands(Bundle bundle, Identity identity, PreferReturn
 			return switch (bundle.getType())
 			{
 				case BATCH -> new BatchCommandList(dataSource, exceptionHandler, commands, validationHelper,
-						snapshotGenerator, eventHandler);
+						snapshotGenerator, eventHandler, responseGenerator);
 
-				case TRANSACTION ->
-					new TransactionCommandList(dataSource, exceptionHandler, commands, transactionResourcesFactory);
+				case TRANSACTION -> new TransactionCommandList(dataSource, exceptionHandler, commands,
+						transactionResourcesFactory, responseGenerator);
 
 				default -> throw new BadBundleException("Unsupported bundle type " + bundle.getType());
 			};

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/ReadCommand.java

@@ -82,12 +82,6 @@ public ReadCommand(int index, Identity identity, PreferReturnType returnType, Bu
 		this.handlingType = handlingType;
 	}
 
-	@Override
-	public void preExecute(Map<String, IdType> idTranslationTable, Connection connection,
-			ValidationHelper validationHelper, SnapshotGenerator snapshotGenerator)
-	{
-	}
-
 	@Override
 	public void execute(Map<String, IdType> idTranslationTable, Connection connection,
 			ValidationHelper validationHelper, SnapshotGenerator snapshotGenerator)

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/command/TransactionCommandList.java

@@ -1,6 +1,7 @@
 package dev.dsf.fhir.dao.command;
 
 import java.sql.Connection;
+import java.sql.SQLException;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
@@ -16,10 +17,13 @@
 import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent;
 import org.hl7.fhir.r4.model.Bundle.BundleType;
 import org.hl7.fhir.r4.model.IdType;
+import org.postgresql.util.PSQLException;
+import org.postgresql.util.PSQLState;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import dev.dsf.fhir.help.ExceptionHandler;
+import dev.dsf.fhir.help.ResponseGenerator;
 import dev.dsf.fhir.validation.SnapshotGenerator;
 import jakarta.ws.rs.WebApplicationException;
 import jakarta.ws.rs.core.Response;
@@ -30,13 +34,16 @@ public class TransactionCommandList extends AbstractCommandList implements Comma
 	private static final Logger logger = LoggerFactory.getLogger(TransactionCommandList.class);
 
 	private final Function<Connection, TransactionResources> transactionResourceFactory;
+	private final ResponseGenerator responseGenerator;
 
 	public TransactionCommandList(DataSource dataSource, ExceptionHandler exceptionHandler,
-			List<? extends Command> commands, Function<Connection, TransactionResources> transactionResourceFactory)
+			List<? extends Command> commands, Function<Connection, TransactionResources> transactionResourceFactory,
+			ResponseGenerator responseGenerator)
 	{
 		super(dataSource, exceptionHandler, commands);
 
 		this.transactionResourceFactory = transactionResourceFactory;
+		this.responseGenerator = responseGenerator;
 
 		Collections.sort(this.commands,
 				Comparator.comparing(Command::getTransactionPriority).thenComparing(Command::getIndex));
@@ -55,7 +62,7 @@ public Bundle execute() throws WebApplicationException
 				{
 					connection.setReadOnly(false);
 					connection.setAutoCommit(false);
-					connection.setTransactionIsolation(Connection.TRANSACTION_REPEATABLE_READ);
+					connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
 				}
 
 				TransactionResources transactionResources = transactionResourceFactory.apply(connection);
@@ -131,7 +138,11 @@ public Bundle execute() throws WebApplicationException
 									e1.getMessage());
 						}
 
-						throw e;
+						if (e instanceof PSQLException s
+								&& PSQLState.UNIQUE_VIOLATION.getState().equals(s.getSQLState()))
+							throw new WebApplicationException(responseGenerator.dupicateResourceExists());
+						else
+							throw e;
 					}
 				}
 
@@ -177,8 +188,20 @@ public Bundle execute() throws WebApplicationException
 
 				if (hasModifyingCommands)
 				{
-					logger.debug("Committing DB transaction");
-					connection.commit();
+					try
+					{
+						logger.debug("Committing DB transaction");
+						connection.commit();
+					}
+					catch (SQLException e)
+					{
+						connection.rollback();
+
+						if (PSQLState.UNIQUE_VIOLATION.getState().equals(e.getSQLState()))
+							throw new WebApplicationException(responseGenerator.dupicateResourceExists());
+						else
+							throw e;
+					}
 				}
 			}
 
@@ -228,4 +251,10 @@ public Bundle execute() throws WebApplicationException
 			throw exceptionHandler.internalServerErrorBundleTransaction(e);
 		}
 	}
+
+	@Override
+	protected Exception internalServerError(Exception exception)
+	{
+		return exceptionHandler.internalServerErrorBundleTransaction(exception);
+	}
 }

dsf-fhir/dsf-fhir-server/src/main/java/dev/dsf/fhir/dao/jdbc/AbstractResourceDaoJdbc.java

@@ -252,7 +252,7 @@ public Connection newReadWriteTransaction() throws SQLException
 	{
 		Connection connection = dataSource.getConnection();
 		connection.setReadOnly(false);
-		connection.setTransactionIsolation(Connection.TRANSACTION_REPEATABLE_READ);
+		connection.setTransactionIsolation(Connection.TRANSACTION_READ_COMMITTED);
 		connection.setAutoCommit(false);
 
 		return connection;
@@ -565,12 +565,8 @@ public final R update(R resource, Long expectedVersion)
 		Objects.requireNonNull(resource, "resource");
 		// expectedVersion may be null
 
-		try (Connection connection = dataSource.getConnection())
+		try (Connection connection = newReadWriteTransaction())
 		{
-			connection.setReadOnly(false);
-			connection.setTransactionIsolation(Connection.TRANSACTION_REPEATABLE_READ);
-			connection.setAutoCommit(false);
-
 			try
 			{
 				R updatedResource = updateWithTransaction(connection, resource, expectedVersion);
@@ -596,9 +592,8 @@ public R updateWithTransaction(Connection connection, R resource, Long expectedV
 		// expectedVersion may be null
 		if (connection.isReadOnly())
 			throw new IllegalArgumentException("Connection is read-only");
-		if (connection.getTransactionIsolation() != Connection.TRANSACTION_REPEATABLE_READ
-				&& connection.getTransactionIsolation() != Connection.TRANSACTION_SERIALIZABLE)
-			throw new IllegalArgumentException("Connection transaction isolation not REPEATABLE_READ or SERIALIZABLE");
+		if (connection.getTransactionIsolation() != Connection.TRANSACTION_READ_COMMITTED)
+			throw new IllegalArgumentException("Connection transaction isolation not READ_COMMITTED");
 		if (connection.getAutoCommit())
 			throw new IllegalArgumentException("Connection transaction is in auto commit mode");