Skip to content
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

Use inject for non singleton class #1617

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class ApplicationContext {
protected static final Map<String, String> TEST_ENV_VARS = new ConcurrentHashMap<>();
protected static final Set<Object> IMPLEMENTATIONS = new HashSet<>();

protected static boolean skipMissingImplementations = false;

protected ApplicationContext() {}

public static void register(Class<?> clazz, Object implementation) {
Expand All @@ -53,16 +55,39 @@ public static <T> Set<Class<? extends T>> getImplementors(Class<T> interfaze) {
}

public static void injectRegisteredImplementations() {
injectRegisteredImplementations(false);
doInjectRegisteredImplementations();
}

protected static void injectRegisteredImplementations(boolean skipMissingImplementations) {
protected static void doInjectRegisteredImplementations() {
var fields = Reflection.getFieldsAnnotatedWith(Inject.class);

fields.forEach(field -> injectIntoField(field, skipMissingImplementations));
fields.forEach(ApplicationContext::injectIntoField);
}

public static void injectIntoNonSingleton(Object instance) {
var fields = Reflection.getFieldsAnnotatedWithInstance(instance.getClass(), Inject.class);

fields.forEach(field -> injectIntoField(field, instance));
}

private static void injectIntoField(Field field, Object instance) {
var fieldType = field.getType();

Object fieldImplementation = getFieldImplementation(fieldType);
if (fieldImplementation == null) {
return;
}

field.trySetAccessible();
try {
field.set(instance, fieldImplementation);
} catch (IllegalAccessException | IllegalArgumentException exception) {
throw new IllegalArgumentException(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding specific exception handling for different types of injection failures. This could improve the robustness and security of the application by providing clearer error messages and handling specific cases differently. [important]

"unable to inject " + fieldType + " into " + instance.getClass(), exception);
}
}

private static void injectIntoField(Field field, boolean skipMissingImplementations) {
private static void injectIntoField(Field field) {
var fieldType = field.getType();
var declaringClass = field.getDeclaringClass();

Expand All @@ -76,13 +101,13 @@ private static void injectIntoField(Field field, boolean skipMissingImplementati
declaringClassesToTry.add(declaringClass);
declaringClassesToTry.addAll(Arrays.asList(declaringClass.getInterfaces()));

Object fieldImplementation = getFieldImplementation(fieldType, skipMissingImplementations);
Object fieldImplementation = getFieldImplementation(fieldType);
if (fieldImplementation == null) {
return;
}

Object declaringClassImplementation =
getDeclaringClassImplementation(declaringClassesToTry, skipMissingImplementations);
getDeclaringClassImplementation(declaringClassesToTry);
if (declaringClassImplementation == null) {
return;
}
Expand All @@ -97,8 +122,7 @@ private static void injectIntoField(Field field, boolean skipMissingImplementati
}
}

private static Object getFieldImplementation(
Class<?> fieldType, boolean skipMissingImplementations) {
private static Object getFieldImplementation(Class<?> fieldType) {
Object fieldImplementation;

try {
Expand All @@ -116,8 +140,7 @@ private static Object getFieldImplementation(
return fieldImplementation;
}

private static Object getDeclaringClassImplementation(
List<Class<?>> declaringClassesToTry, boolean skipMissingImplementations) {
private static Object getDeclaringClassImplementation(List<Class<?>> declaringClassesToTry) {
Object declaringClassImplementation =
declaringClassesToTry.stream()
.map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import static org.reflections.scanners.Scanners.FieldsAnnotated;
import static org.reflections.scanners.Scanners.SubTypes;

import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.Set;
import java.util.stream.Collectors;
import org.reflections.Reflections;

/**
Expand All @@ -27,4 +30,10 @@ public static <T> Set<Class<? extends T>> getImplementors(Class<T> interfaze) {
public static Set<Field> getFieldsAnnotatedWith(Class<?> annotation) {
return REFLECTIONS.get(FieldsAnnotated.with(annotation).as(Field.class));
}

public static Set<Field> getFieldsAnnotatedWithInstance(Class<?> clazz, Class<?> annotation) {
return Arrays.stream(clazz.getDeclaredFields())
.filter(field -> field.isAnnotationPresent(annotation.asSubclass(Annotation.class)))
.collect(Collectors.toSet());
}
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
package gov.hhs.cdc.trustedintermediary.external.hapi;

import gov.hhs.cdc.trustedintermediary.context.ApplicationContext;
import gov.hhs.cdc.trustedintermediary.wrappers.HealthData;
import gov.hhs.cdc.trustedintermediary.wrappers.Logger;
import javax.inject.Inject;
import org.hl7.fhir.instance.model.api.IBaseResource;

/** An implementation of {@link HealthData} to use as a wrapper around HAPI FHIR IBaseResource */
public class HapiFhirResource implements HealthData<IBaseResource> {

@Inject private Logger logger;
private final IBaseResource innerResource;

public HapiFhirResource(IBaseResource innerResource) {
ApplicationContext.injectIntoNonSingleton(this);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid potential issues with circular dependencies and incomplete construction, consider using a factory or builder pattern to manage object creation and dependency injection, rather than injecting dependencies directly in the constructor. [important]

this.innerResource = innerResource;
}

@Override
public IBaseResource getUnderlyingData() {
this.logger.logDebug("testing @Inject");
return innerResource;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,28 @@ import java.nio.file.Paths

class ApplicationContextTest extends Specification {

interface TestingInterface {
void test()
}

static class DogCow implements TestingInterface {

@Override
void test() {
print("test()")
}
}

static class DogCowTwo implements TestingInterface {

@Override
void test() {
print("testTwo()")
}
}
def DOGCOW = new DogCow()
def DOGCOWTWO = new DogCowTwo()

def setup() {
TestApplicationContext.reset()
}
Expand All @@ -21,6 +43,18 @@ class ApplicationContextTest extends Specification {
result == ApplicationContext.getImplementation(String.class)
}

def "implementors retrieval test"() {
setup:
def dogCow = DOGCOW
def dogCowTwo = DOGCOWTWO
def implementors = new HashSet()
implementors.add(DogCow)
implementors.add(DogCowTwo)

expect:
implementors == ApplicationContext.getImplementors(TestingInterface)
}

def "implementation injection test"() {
given:
def injectedValue = "DogCow"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package gov.hhs.cdc.trustedintermediary.external.hapi

import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext
import gov.hhs.cdc.trustedintermediary.wrappers.Logger
import org.hl7.fhir.r4.model.Bundle
import spock.lang.Specification

class HapiFhirResourceTest extends Specification {

def mockLogger = Mock(Logger)
def setup() {
TestApplicationContext.reset()
TestApplicationContext.init()
TestApplicationContext.register(Logger, mockLogger)
}

def "sample test run to see if logger is injected"() {
given:
def expectedBundle = new Bundle()
when:
def resource = new HapiFhirResource(expectedBundle)
def actualBundle = resource.getUnderlyingData()
then:
expectedBundle == actualBundle
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ class TestApplicationContext extends ApplicationContext {
TEST_ENV_VARS.clear()
}

def static injectRegisteredImplementations() {
injectRegisteredImplementations(true)
def static injectRegisteredImplementations(def skip = true) {
skipMissingImplementations = skip
doInjectRegisteredImplementations()
}

def static addEnvironmentVariable(String key, String value) {
Expand Down