Skip to content

Commit

Permalink
Merge pull request #4024 from LinuxForHealth/issue-4023
Browse files Browse the repository at this point in the history
issue #4023 - use FHIRPersistenceTransaction instead of ITransactionProvider
  • Loading branch information
lmsurpre authored Oct 21, 2022
2 parents 770fd13 + 0b5c2fb commit 6d6c459
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 25 deletions.
1 change: 1 addition & 0 deletions .github/workflows/audit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Integration - Audit
on:
pull_request:
paths-ignore:
- '**.md'
- 'docs/**'
- 'demo/**'
- '.github/workflows/site.yml'
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Build and Test
on:
pull_request:
paths-ignore:
- '**.md'
- 'docs/**'
- 'demo/**'
- '.github/workflows/site.yml'
Expand Down Expand Up @@ -79,6 +80,7 @@ jobs:
)
jdbc_paths=(
fhir-persistence-jdbc
fhir-persistence-params
fhir-search/src/main
fhir-persistence/src/main
fhir-persistence-schema/src/main
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Integration
on:
pull_request:
paths-ignore:
- '**.md'
- 'docs/**'
- 'demo/**'
- '.github/workflows/site.yml'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/migration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:
- 'fhir-database-utils/**'
- 'fhir-persistence-schema/**'
- '!fhir-persistence-schema/docs/**'
- '!**/README.md'
- '!**.md'
# allows the workflow to be manually executed any time
workflow_dispatch:
inputs:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/notification.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Integration - Notifications
on:
pull_request:
paths-ignore:
- '**.md'
- 'docs/**'
- 'demo/**'
- '.github/workflows/site.yml'
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/persistence.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Integration - Persistence
on:
pull_request:
paths-ignore:
- '**.md'
- 'docs/**'
- 'demo/**'
- '.github/workflows/site.yml'
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/reindex.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Reindex - Integration Tests
on:
pull_request:
paths-ignore:
- '**.md'
- 'docs/**'
- 'demo/**'
- '.github/workflows/site.yml'
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Integration - Windows
on:
pull_request:
paths-ignore:
- '**.md'
- 'docs/**'
- 'demo/**'
- '.github/workflows/site.yml'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@

package org.linuxforhealth.fhir.path;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -339,8 +337,10 @@ public enum FHIRPathType {

private static final Map<java.lang.String, FHIRPathType> TYPE_NAME_MAP = buildTypeNameMap();
private static final Map<Class<?>, FHIRPathType> TYPE_MAP = buildTypeMap();
private static final Set<FHIRPathType> SYSTEM_TYPES = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(SYSTEM_BOOLEAN, SYSTEM_STRING, SYSTEM_INTEGER, SYSTEM_DECIMAL, SYSTEM_DATE, SYSTEM_DATE_TIME, SYSTEM_QUANTITY, SYSTEM_TIME)));
private static final Set<FHIRPathType> METAMODEL_TYPES = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(SYSTEM_TYPE_INFO, SYSTEM_CLASS_INFO, SYSTEM_TUPLE_TYPE_INFO, SYSTEM_LIST_TYPE_INFO, SYSTEM_SIMPLE_TYPE_INFO)));
private static final Set<FHIRPathType> SYSTEM_TYPES =
Set.of(SYSTEM_BOOLEAN, SYSTEM_STRING, SYSTEM_INTEGER, SYSTEM_DECIMAL, SYSTEM_DATE, SYSTEM_DATE_TIME, SYSTEM_QUANTITY, SYSTEM_TIME);
private static final Set<FHIRPathType> METAMODEL_TYPES =
Set.of(SYSTEM_TYPE_INFO, SYSTEM_CLASS_INFO, SYSTEM_TUPLE_TYPE_INFO, SYSTEM_LIST_TYPE_INFO, SYSTEM_SIMPLE_TYPE_INFO);
private static final Map<Class<?>, FHIRPathType> METAMODEL_TYPE_MAP = buildMetamodelTypeMap();
private static final Map<Class<?>, FHIRPathType> JAVA_TYPE_MAP = buildJavaTypeMap();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2020
* (C) Copyright IBM Corp. 2020, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -19,32 +19,32 @@

/**
* Hides the logic behind obtaining a JDBC {@link Connection} from the DAO code.
*
*
* Use by unit tests or other scenarios where connections are obtained using an
* IConnectionProvider implementation, outside the scope of a JEE container.
* Transactions are managed with the help of the SimpleTransactionProvider and
* wrapped by this class, meaning we have a uniform interface for handling
* transactions across JEE and unit-test scenarios.
*
*
*/
public class FHIRTestTransactionAdapter implements FHIRPersistenceTransaction {
private static final Logger log = Logger.getLogger(FHIRDbTestConnectionStrategy.class.getName());

// Provides connections when outside of a container
private final IConnectionProvider connectionProvider;

// If given, invoke this callback just prior to commit
private final IFHIRTransactionAdapterCallback beforeCommitCallback;

// Support transactions for the persistence unit tests
private final SimpleTransactionProvider transactionProvider;

// Just in case we have nesting issues, use ThreadLocal to track the current tx
private final ThreadLocal<ITransaction> currentTransaction = new ThreadLocal<>();

// Was this instance responsible for starting the transaction
private boolean startedByThis;

// support nesting by tracking the number of begin/end requests
private int startCount;

Expand All @@ -68,7 +68,7 @@ public void begin() throws FHIRPersistenceException {
this.currentTransaction.set(this.transactionProvider.getTransaction());
this.startedByThis = true;
}

// add to the start request counter every time. We only close when this
// counter reaches 0.
startCount++;
Expand All @@ -79,14 +79,15 @@ public void end() throws FHIRPersistenceException {
if (currentTransaction.get() == null) {
throw new FHIRPersistenceDataAccessException("Transaction not started");
}

try {
// only end it if we started it
if (startedByThis && --startCount == 0) {
if (this.beforeCommitCallback != null) {
this.beforeCommitCallback.beforeCommit();
try (ITransaction tx = this.currentTransaction.get()) {
if (this.beforeCommitCallback != null) {
this.beforeCommitCallback.beforeCommit();
}
}
this.currentTransaction.get().close();
}
} catch (Throwable x) {
// translate to a FHIRPersistenceException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.linuxforhealth.fhir.model.spec.test.IExampleProcessor;
import org.linuxforhealth.fhir.model.visitor.ResourceFingerprintVisitor;
import org.linuxforhealth.fhir.persistence.FHIRPersistence;
import org.linuxforhealth.fhir.persistence.FHIRPersistenceTransaction;
import org.linuxforhealth.fhir.persistence.context.FHIRHistoryContext;
import org.linuxforhealth.fhir.persistence.context.FHIRPersistenceContext;
import org.linuxforhealth.fhir.persistence.context.FHIRPersistenceContextFactory;
Expand Down Expand Up @@ -187,11 +188,20 @@ public void process(String jsonFile, Resource resource) throws Exception {
context.setOriginalFingerprint(v.getSaltAndHash());

if (this.persistence == null) {
try (ITransaction tx = this.transactionProvider.getTransaction()) {
// ITestResourceOperation#process throws Exception, which precludes the
// use of forEach here...so going old-school keeps it simpler
for (ITestResourceOperation op: operations) {

// ITestResourceOperation#process throws Exception, which precludes the
// use of forEach here...so going old-school keeps it simpler
for (ITestResourceOperation op: operations) {
FHIRPersistenceTransaction tx = context.getPersistence().getTransaction();
tx.begin();
try {
op.process(context);
} catch (Throwable t) {
tx.setRollbackOnly();
throw t;
} finally {
// will do a commit, or rollback if tx.setRollbackOnly() has been set
tx.end();
}
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void pushBatch() throws FHIRPersistenceException {
try {
dao.pushBatch();
} catch (SQLException x) {
throw new FHIRPersistenceException("pushBatch failed for '" + resourceType + "'");
throw new FHIRPersistenceException("pushBatch failed for '" + resourceType + "'", x);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,12 @@ private void validate(FHIRPathResourceNode resourceNode) {
constraints.addAll(ProfileSupport.getConstraints(profiles, resourceType));
}

// path (with array indices) -> Extension instance (which never have a url version suffix)
Map<String, Extension> pathToExtension = gatherInstanceExtensions(resourceNode);
// path (with no array indices) -> Extension URLs (with no version suffix)
Map<String, Set<String>> alreadyGeneratedExtensionConstraints = gatherExtensionConstraints(constraints);

// path (with array indices) -> Extension instance (which never have a url version suffix)
Map<String, Extension> pathToExtension = gatherInstanceExtensions(resourceNode);

// for each extension in the resource instance
for (Entry<String, Extension> entry : pathToExtension.entrySet()) {
String path = entry.getKey();
Expand Down

0 comments on commit 6d6c459

Please sign in to comment.