Skip to content

Commit

Permalink
Cleanupo
Browse files Browse the repository at this point in the history
  • Loading branch information
@jamesagnew
jamesagnew committed Nov 24, 2024
1 parent 72ff253 commit 4713eee
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 106 deletions.
8 changes: 5 additions & 3 deletions hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/JpaConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,15 @@ public class JpaConfig {
@Autowired
private FhirContext myFhirContext;

@Autowired
private ValidationSupportChain.CacheConfiguration myCacheConfiguration;
@Bean
public ValidationSupportChain.CacheConfiguration validationSupportChainCacheConfiguration() {
return ValidationSupportChain.CacheConfiguration.defaultValues();
}

@Bean(name = JpaConfig.JPA_VALIDATION_SUPPORT_CHAIN)
@Primary
public IValidationSupport jpaValidationSupportChain() {
return new JpaValidationSupportChain(myFhirContext, myCacheConfiguration);
return new JpaValidationSupportChain(myFhirContext, validationSupportChainCacheConfiguration());
}

@Bean("myDaoRegistry")
Expand Down
6 changes: 0 additions & 6 deletions hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/config/ValidationSupportConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import ca.uhn.fhir.jpa.validation.ValidatorResourceFetcher;
import ca.uhn.fhir.validation.IInstanceValidatorModule;
import org.hl7.fhir.common.hapi.validation.support.InMemoryTerminologyServerValidationSupport;
import org.hl7.fhir.common.hapi.validation.support.ValidationSupportChain;
import org.hl7.fhir.common.hapi.validation.validator.FhirInstanceValidator;
import org.hl7.fhir.r5.utils.validation.constants.BestPracticeWarningLevel;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -57,11 +56,6 @@ public InMemoryTerminologyServerValidationSupport inMemoryTerminologyServerValid
return retVal;
}

@Bean
public ValidationSupportChain.CacheConfiguration validationSupportChainCacheConfiguration() {
return ValidationSupportChain.CacheConfiguration.defaultValues();
}

@Bean(name = JpaConfig.JPA_VALIDATION_SUPPORT)
public IValidationSupport jpaValidationSupport(FhirContext theFhirContext) {
return new JpaPersistedResourceValidationSupport(theFhirContext);
Expand Down
2 changes: 0 additions & 2 deletions ...-test-r4/src/test/java/ca/uhn/fhir/jpa/interceptor/RemoteTerminologyServiceJpaR4Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ public void testValidateSimpleCode() {
// Verify 1
Assertions.assertEquals(2, myCaptureQueriesListener.countGetConnections());
assertThat(ourValueSetProvider.mySearchUrls).asList().containsExactlyInAnyOrder(
"http://hl7.org/fhir/ValueSet/administrative-gender",
"http://hl7.org/fhir/ValueSet/administrative-gender",
"http://hl7.org/fhir/ValueSet/administrative-gender"
);
Expand Down Expand Up @@ -300,7 +299,6 @@ public void testValidateMultipleCodings() {
// Verify 1
Assertions.assertEquals(2, myCaptureQueriesListener.countGetConnections());
assertThat(ourValueSetProvider.mySearchUrls).asList().containsExactlyInAnyOrder(
"http://hl7.org/fhir/ValueSet/identifier-type",
"http://hl7.org/fhir/ValueSet/identifier-type",
"http://hl7.org/fhir/ValueSet/identifier-type"
);
Expand Down
35 changes: 29 additions & 6 deletions ...ion/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.hl7.fhir.common.hapi.validation.support;

import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.FhirVersionEnum;
Expand All @@ -21,11 +22,11 @@
import jakarta.annotation.PreDestroy;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.concurrent.BasicThreadFactory;
import org.apache.commons.lang3.time.DateUtils;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import org.slf4j.Logger;

import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -657,8 +658,30 @@ public IBaseResource fetchValueSet(String theUrl) {
return fetchResource(key, invoker, theUrl);
}

@SuppressWarnings("unchecked")
@Override
public <T extends IBaseResource> T fetchResource(Class<T> theClass, String theUri) {

/*
* If we're looking for a common type with a dedicated fetch method, use that
* so that we can use a common cache location for lookups wanting a given
* URL on both methods (the validator will call both paths when looking for a
* specific URL so this improves cache efficiency).
*/
if (theClass != null) {
BaseRuntimeElementDefinition<?> elementDefinition = getFhirContext().getElementDefinition(theClass);
if (elementDefinition != null) {
switch (elementDefinition.getName()) {
case "ValueSet":
return (T) fetchValueSet(theUri);
case "CodeSystem":
return (T) fetchCodeSystem(theUri);
case "StructureDefinition":
return (T) fetchStructureDefinition(theUri);
}
}
}

Function<IValidationSupport, T> invoker = v -> v.fetchResource(theClass, theUri);
TypedResourceByUrlKey<T> key = new TypedResourceByUrlKey<>(theClass, theUri);
return fetchResource(key, invoker, theUri);
Expand Down Expand Up @@ -956,9 +979,9 @@ public long getCacheTimeout() {
return myCacheTimeout;
}

public CacheConfiguration setCacheTimeout(long theCacheTimeout) {
Validate.isTrue(theCacheTimeout >= 0, "Cache timeout must not be negative");
myCacheTimeout = theCacheTimeout;
public CacheConfiguration setCacheTimeout(Duration theCacheTimeout) {
Validate.isTrue(theCacheTimeout.toMillis() >= 0, "Cache timeout must not be negative");
myCacheTimeout = theCacheTimeout.toMillis();
return this;
}

Expand All @@ -978,12 +1001,12 @@ public CacheConfiguration setCacheSize(int theCacheSize) {
*/
public static CacheConfiguration defaultValues() {
return new CacheConfiguration()
.setCacheTimeout(10 * DateUtils.MILLIS_PER_MINUTE)
.setCacheTimeout(Duration.ofMinutes(10))
.setCacheSize(5000);
}

public static CacheConfiguration disabled() {
return new CacheConfiguration().setCacheSize(0).setCacheTimeout(0);
return new CacheConfiguration().setCacheSize(0).setCacheTimeout(Duration.ofMillis(0));
}
}

Expand Down
131 changes: 42 additions & 89 deletions ...src/test/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChainTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,13 @@
import ca.uhn.fhir.test.BaseTest;
import ca.uhn.fhir.util.TestUtil;
import com.google.common.collect.Lists;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.metrics.LongCounter;
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.api.metrics.ObservableLongMeasurement;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.testing.LibraryTestRunner;
import io.opentelemetry.sdk.metrics.data.Data;
import io.opentelemetry.sdk.metrics.data.LongPointData;
import io.opentelemetry.sdk.metrics.data.MetricData;
import jakarta.annotation.Nonnull;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.r4.model.ListResource;
import org.hl7.fhir.r4.model.SearchParameter;
import org.hl7.fhir.r4.model.StructureDefinition;
import org.hl7.fhir.r4.model.ValueSet;
Expand All @@ -45,17 +34,14 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.swing.filechooser.FileSystemView;
import java.io.File;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

import static io.opentelemetry.api.common.AttributeKey.stringKey;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNotSame;
Expand Down Expand Up @@ -399,7 +385,7 @@ public void testFetchAllNonBaseStructureDefinitions() {

final ValidationSupportChain.CacheConfiguration cacheTimeouts = ValidationSupportChain.CacheConfiguration
.defaultValues()
.setCacheTimeout(500);
.setCacheTimeout(Duration.ofMillis(500));
ValidationSupportChain chain = new ValidationSupportChain(cacheTimeouts, myValidationSupport0);

// First call should return the full list
Expand All @@ -415,8 +401,7 @@ public void testFetchAllNonBaseStructureDefinitions() {
assertEquals(3, chain.fetchAllNonBaseStructureDefinitions().size());

// Eventually we should refresh
await().until(() -> chain.fetchAllNonBaseStructureDefinitions().size(), equalTo(2));

await().until(() -> chain.fetchAllNonBaseStructureDefinitions().size(), t -> t == 2);
}

@ParameterizedTest
Expand Down Expand Up @@ -540,24 +525,58 @@ public void testFetchValueSet(boolean theUseCache) {

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testResource(boolean theUseCache) {
public void testResource_ValueSet(boolean theUseCache) {
// Setup
prepareMock(myValidationSupport0, myValidationSupport1, myValidationSupport2);
ValidationSupportChain chain = new ValidationSupportChain(newCacheConfiguration(theUseCache), myValidationSupport0, myValidationSupport1, myValidationSupport2);

when(myValidationSupport0.fetchResource(any(), any())).thenReturn(null);
when(myValidationSupport1.fetchResource(any(), any())).thenAnswer(t -> new ValueSet());
when(myValidationSupport0.fetchValueSet(any())).thenReturn(null);
when(myValidationSupport1.fetchValueSet(any())).thenAnswer(t -> new ValueSet());

// Test
IBaseResource result = chain.fetchResource(ValueSet.class, VALUE_SET_URL_0);

// Verify
verify(myValidationSupport0, times(1)).fetchValueSet( any());
verify(myValidationSupport1, times(1)).fetchValueSet( any());
verify(myValidationSupport2, times(0)).fetchValueSet(any());

// Test again (should use cache)
IBaseResource result2 = chain.fetchResource(ValueSet.class, VALUE_SET_URL_0);

// Verify
if (theUseCache) {
assertSame(result, result2);
verify(myValidationSupport0, times(1)).fetchValueSet( any());
verify(myValidationSupport1, times(1)).fetchValueSet( any());
verify(myValidationSupport2, times(0)).fetchValueSet( any());
} else {
verify(myValidationSupport0, times(2)).fetchValueSet( any());
verify(myValidationSupport1, times(2)).fetchValueSet( any());
verify(myValidationSupport2, times(0)).fetchValueSet(any());
}
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testResource_Arbitrary(boolean theUseCache) {
// Setup
prepareMock(myValidationSupport0, myValidationSupport1, myValidationSupport2);
ValidationSupportChain chain = new ValidationSupportChain(newCacheConfiguration(theUseCache), myValidationSupport0, myValidationSupport1, myValidationSupport2);

when(myValidationSupport0.fetchResource(any(), any())).thenReturn(null);
when(myValidationSupport1.fetchResource(any(), any())).thenAnswer(t -> new ListResource());

// Test
IBaseResource result = chain.fetchResource(ListResource.class, "http://foo");

// Verify
verify(myValidationSupport0, times(1)).fetchResource(any(), any());
verify(myValidationSupport1, times(1)).fetchResource(any(), any());
verify(myValidationSupport2, times(0)).fetchResource(any(), any());

// Test again (should use cache)
IBaseResource result2 = chain.fetchResource(ValueSet.class, VALUE_SET_URL_0);
IBaseResource result2 = chain.fetchResource(ListResource.class, "http://foo");

// Verify
if (theUseCache) {
Expand Down Expand Up @@ -704,70 +723,4 @@ private static void createMockValidationSupportWithSingleBinary(IValidationSuppo
when(theValidationSupport.fetchBinary(theExpectedBinaryKey)).thenReturn(theExpectedBinaryContent);
}

// FIXME: remove
public static final class LongCounterExample {

public static final String INSTRUMENTATION_SCOPE = "io.opentelemetry.example.metrics";
private static final File homeDirectory = FileSystemView.getFileSystemView().getHomeDirectory();

private static final AttributeKey<String> ROOT_DIRECTORY_KEY = stringKey("root directory");
// statically allocate the attributes, since they are known at init time.
private static final Attributes HOME_DIRECTORY_ATTRIBUTES =
Attributes.of(ROOT_DIRECTORY_KEY, homeDirectory.getName());
private final Meter meter;
private final Tracer tracer;

public LongCounterExample(Tracer tracer, Meter meter) {
this.tracer = tracer;
this.meter = meter;
}

void run() {
Span span = tracer.spanBuilder("workflow").setSpanKind(SpanKind.INTERNAL).startSpan();
try (Scope scope = span.makeCurrent()) {
LongCounter directoryCounter = findFile("file_to_find.txt", homeDirectory);
directoryCounter.add(1, HOME_DIRECTORY_ATTRIBUTES); // count root directory
} catch (Exception e) {
span.setStatus(StatusCode.ERROR, "Error while finding file");
} finally {
span.end();
}
}

/**
* Uses the Meter instance to create a LongCounter with the given name, description, and units.
* This is the counter that is used to count directories during filesystem traversal.
*/
LongCounter createCounter() {
return meter
.counterBuilder("directories_search_count")
.setDescription("Counts directories accessed while searching for files.")
.setUnit("unit")
.build();
}

LongCounter findFile(String name, File directory) {
LongCounter directoryCounter = createCounter();
directoryCounter.add(1, HOME_DIRECTORY_ATTRIBUTES);
return directoryCounter;
}

public static void main(String[] args) {
OpenTelemetry sdk = GlobalOpenTelemetry.get();

Tracer tracer = sdk.getTracer(INSTRUMENTATION_SCOPE, "0.13.1");

Meter meterBuilder = sdk.meterBuilder("AAAAAA").setInstrumentationVersion("1").build();
ObservableLongMeasurement bbbb = meterBuilder.counterBuilder("BBBB").buildObserver();
bbbb.record(222L);

meterBuilder.counterBuilder("BBBB").buildWithCallback(t -> t.record(123L));


Meter meter = sdk.getMeter(INSTRUMENTATION_SCOPE);
LongCounterExample example = new LongCounterExample(tracer, meter);
example.run();
}
}

}

0 comments on commit 4713eee

Please sign in to comment.