Skip to main content
Tweeted twitter.com/StackSoftEng/status/1289078353685893120
Added new information
Source Link
totsubo
  • 129
  • 2

I used the above class in other parts of my code like this which feelsnicefeels nice and tidy:

public ProcessorInternalPublisher() {
    internalDataStorageClient = new InternalDataStorageClient();
}
  1. Is the use of the static method createDynamoDBMapper dependency injection?
  2. How can I improve my class implementation, what are some of the standard patterns/strategies I should be using?
  3. Is the problem my unit test and not the class implementation?
  4. In the Update below I write how this refactoring now breaks another unit test which I think is proof that there is an issue with my design
public ProcessorInternalPublisher() {
    String databaseTableName = System.getenv("TableName");
    DynamoDBMapper mapper = InternalDataStorageClient.createDynamoDBMapper(databaseTableName);
    internalDataStorageClient = new InternalDataStorageClient(mapper);
}
public class InternalDataStorageClientTest {
    private InternalDataStorageClient internalDataStorageClient;
    private DynamoDBMapper dynamoDBMapperMock;

    @Before
    public void setup() {
        dynamoDBMapperMock = Mockito.mock(DynamoDBMapper.class);
        when(dynamoDBMapperMock.batchSave()).thenReturn(new ArrayList<>());
        internalDataStorageClient = new InternalDataStorageClient(dynamoDBMapperMock);
    }

    @Test
    public void WHEN_SingleSuccessRequestResult_THEN_CorrectItemBatch() throws Exception {
        Request correctSingleRequest = SQSTestEvents.createCorrectSingleTrainingRequest();
        internalDataStorageClient.storeResults(correctSingleRequest.getEventList(), Collections.emptyList());

        DBItem item = internalDataStorageClient.createDBItem(correctSingleRequest.getEventList().get(0), true);
        assertThat(internalDataStorageClient.getItemBatch(), containsInAnyOrder(item));

        verify(dynamoDBMapperMock, times(1)).batchSave(internalDataStorageClient.getItemBatch());
    }
}

Update:

Though the solution above of using a static method allowed me to write my unit test, it broke the unit tests for my InternalPublisher class! The InternalPublisher unit tests create a InternalPublisher object and the construction fails because it is trying to use the external DB dependency.

public class InternalPublisherClientTest {

    private InternalPublisher internalPublisher;
    private ExternalPublisherServiceClient externalPublisher;
    private InternalDataStorageClient internalDataStorageClient;

    @Before
    public void setup() {
        internalPublisher = new InternalPublisher();
        externalPublisher = Mockito.mock(ExternalPublisherServiceClient.class, RETURNS_DEEP_STUBS);
        internalDataStorageClient = Mockito.mock(InternalDataStorageClient.class, RETURNS_DEEP_STUBS);
    }

    //Test that we do the right thing on external failure
    @Test
    public void WHEN_InvalidEvent_THEN_Failed_List_With_ID() {
        Request request = createCorrectSingleTrainingRequest();
        List<Event> failures = new ArrayList<>(request.getEventList());

        //Make external dependency call fail
        when(externalPublisher.newPublishServiceNotificationCall()).thenThrow(new PublisherException());

        //Confirm our internal client has collected all the failures
        internalPublisher.publishToPTNS(Collections.singletonList(request), externalPublisher);
        assertThat(internalPublisher.getFailedRequests(), is(failures));
    }
}

I used the above class in other parts of my code like this which feelsnice and tidy:

public Processor() {
    internalDataStorageClient = new InternalDataStorageClient();
}
  1. Is the use of the static method createDynamoDBMapper dependency injection?
  2. How can I improve my class implementation, what are some of the standard patterns/strategies I should be using?
  3. Is the problem my unit test and not the class implementation?
public Processor() {
    String databaseTableName = System.getenv("TableName");
    DynamoDBMapper mapper = InternalDataStorageClient.createDynamoDBMapper(databaseTableName);
    internalDataStorageClient = new InternalDataStorageClient(mapper);
}
public class InternalDataStorageClientTest {
    private InternalDataStorageClient internalDataStorageClient;
    private DynamoDBMapper dynamoDBMapperMock;

    @Before
    public void setup() {
        dynamoDBMapperMock = Mockito.mock(DynamoDBMapper.class);
        when(dynamoDBMapperMock.batchSave()).thenReturn(new ArrayList<>());
        internalDataStorageClient = new InternalDataStorageClient(dynamoDBMapperMock);
    }

    @Test
    public void WHEN_SingleSuccessRequestResult_THEN_CorrectItemBatch() throws Exception {
        Request correctSingleRequest = SQSTestEvents.createCorrectSingleTrainingRequest();
        internalDataStorageClient.storeResults(correctSingleRequest.getEventList(), Collections.emptyList());

        DBItem item = internalDataStorageClient.createDBItem(correctSingleRequest.getEventList().get(0), true);
        assertThat(internalDataStorageClient.getItemBatch(), containsInAnyOrder(item));

        verify(dynamoDBMapperMock, times(1)).batchSave(internalDataStorageClient.getItemBatch());
    }
}

I used the above class in other parts of my code like this which feels nice and tidy:

public InternalPublisher() {
    internalDataStorageClient = new InternalDataStorageClient();
}
  1. Is the use of the static method createDynamoDBMapper dependency injection?
  2. How can I improve my class implementation, what are some of the standard patterns/strategies I should be using?
  3. Is the problem my unit test and not the class implementation?
  4. In the Update below I write how this refactoring now breaks another unit test which I think is proof that there is an issue with my design
public InternalPublisher() {
    String databaseTableName = System.getenv("TableName");
    DynamoDBMapper mapper = InternalDataStorageClient.createDynamoDBMapper(databaseTableName);
    internalDataStorageClient = new InternalDataStorageClient(mapper);
}
public class InternalDataStorageClientTest {
    private InternalDataStorageClient internalDataStorageClient;
    private DynamoDBMapper dynamoDBMapperMock;

    @Before
    public void setup() {
        dynamoDBMapperMock = Mockito.mock(DynamoDBMapper.class);
        when(dynamoDBMapperMock.batchSave()).thenReturn(new ArrayList<>());
        internalDataStorageClient = new InternalDataStorageClient(dynamoDBMapperMock);
    }

    @Test
    public void WHEN_SingleSuccessRequestResult_THEN_CorrectItemBatch() throws Exception {
        Request correctSingleRequest = SQSTestEvents.createCorrectSingleTrainingRequest();
        internalDataStorageClient.storeResults(correctSingleRequest.getEventList(), Collections.emptyList());

        DBItem item = internalDataStorageClient.createDBItem(correctSingleRequest.getEventList().get(0), true);
        assertThat(internalDataStorageClient.getItemBatch(), containsInAnyOrder(item));

        verify(dynamoDBMapperMock, times(1)).batchSave(internalDataStorageClient.getItemBatch());
    }
}

Update:

Though the solution above of using a static method allowed me to write my unit test, it broke the unit tests for my InternalPublisher class! The InternalPublisher unit tests create a InternalPublisher object and the construction fails because it is trying to use the external DB dependency.

public class InternalPublisherClientTest {

    private InternalPublisher internalPublisher;
    private ExternalPublisherServiceClient externalPublisher;
    private InternalDataStorageClient internalDataStorageClient;

    @Before
    public void setup() {
        internalPublisher = new InternalPublisher();
        externalPublisher = Mockito.mock(ExternalPublisherServiceClient.class, RETURNS_DEEP_STUBS);
        internalDataStorageClient = Mockito.mock(InternalDataStorageClient.class, RETURNS_DEEP_STUBS);
    }

    //Test that we do the right thing on external failure
    @Test
    public void WHEN_InvalidEvent_THEN_Failed_List_With_ID() {
        Request request = createCorrectSingleTrainingRequest();
        List<Event> failures = new ArrayList<>(request.getEventList());

        //Make external dependency call fail
        when(externalPublisher.newPublishServiceNotificationCall()).thenThrow(new PublisherException());

        //Confirm our internal client has collected all the failures
        internalPublisher.publishToPTNS(Collections.singletonList(request), externalPublisher);
        assertThat(internalPublisher.getFailedRequests(), is(failures));
    }
}
Tried to improve the title
Link
Doc Brown
  • 220.6k
  • 35
  • 410
  • 625

Best practices for re-working Changing a class with static dependency injection to allow for unit testing

added 1145 characters in body
Source Link
totsubo
  • 129
  • 2

I wrote a class but when came time to write a unit test I ran into issueissues because the class has a dependency on an external AWS DynamoDB. I re-wrote the class and implemented a sort of dependency injection and would like advice on my approach and also on the unit test.

The objectThis was was my original implementation:

     public InternalDataStorageClient() {
         tableName = System.getenv("TableName");
         DynamoDBMapperConfig mapperConfig = new DynamoDBMapperConfig.Builder().withTableNameOverride(DynamoDBMapperConfig.TableNameOverride.withTableNameReplacement(tableName))
                 .build();
         ddbMapper = new DynamoDBMapper(client, mapperConfig);
         itemBatch = new ArrayList<>();
     }

I used the above class in other parts of my test iscode like this which feelsnice and tidy:

public Processor() {
    internalDataStorageClient = new InternalDataStorageClient();
}

When came time to write the test thatI couldn't because creating an instance of storeResults()InternalDataStorageClient will have written the correct objectwould throw an exception since you can't create a DynamoDBMapper without a connection to the databaseAWS.

I've writtenI rewrote the class implementationas below and one unit test. I feel that the class implementation is okay but that there is probablyit feels like a better way. Ihack, especially feel that the static method createDynamoDBMapper() is probably not the best waycreateDynamoDBMapper() as a work-around to achieve dependency injectionallow for unit testing.

  1. Are there other way to solveIs the externaluse of the static method createDynamoDBMapper dependency issueinjection?
  2. How can I improve my dependency-injection-like solutionclass implementation, what are some of the standard patterns/strategies I should be using?
  3. Is there a better way to write the problem my unit test and not the class implementation?

Now my other classes use the above class like this, which doesn't feel quite as clean as before:

public Processor() {
    String databaseTableName = System.getenv("TableName");
    DynamoDBMapper mapper = InternalDataStorageClient.createDynamoDBMapper(databaseTableName);
    internalDataStorageClient = new InternalDataStorageClient(mapper);
}

Here is the JUnit test:

I wrote a class but when came time to write a unit test I ran into issue because the class has a dependency on an external AWS DynamoDB. I re-wrote the class and implemented a sort of dependency injection and would like advice on my approach and also on the unit test.

The object of my test is to test that storeResults() will have written the correct object to the database.

I've written the class implementation below and one unit test. I feel that the class implementation is okay but that there is probably a better way. I especially feel that the static method createDynamoDBMapper() is probably not the best way to achieve dependency injection.

  1. Are there other way to solve the external dependency issue?
  2. How can I improve my dependency-injection-like solution?
  3. Is there a better way to write the unit test

Here is the JUnit test

I wrote a class but when came time to unit test I ran into issues because the class has a dependency on an external AWS DynamoDB. I re-wrote the class and implemented a sort of dependency injection and would like advice on my approach and also on the unit test.

This was was my original implementation:

     public InternalDataStorageClient() {
         tableName = System.getenv("TableName");
         DynamoDBMapperConfig mapperConfig = new DynamoDBMapperConfig.Builder().withTableNameOverride(DynamoDBMapperConfig.TableNameOverride.withTableNameReplacement(tableName))
                 .build();
         ddbMapper = new DynamoDBMapper(client, mapperConfig);
         itemBatch = new ArrayList<>();
     }

I used the above class in other parts of my code like this which feelsnice and tidy:

public Processor() {
    internalDataStorageClient = new InternalDataStorageClient();
}

When came time to write the test I couldn't because creating an instance of InternalDataStorageClient would throw an exception since you can't create a DynamoDBMapper without a connection to AWS.

I rewrote the class as below. I feel that the class implementation is okay but it feels like a hack, especially feel that the static method createDynamoDBMapper() as a work-around to allow for unit testing.

  1. Is the use of the static method createDynamoDBMapper dependency injection?
  2. How can I improve my class implementation, what are some of the standard patterns/strategies I should be using?
  3. Is the problem my unit test and not the class implementation?

Now my other classes use the above class like this, which doesn't feel quite as clean as before:

public Processor() {
    String databaseTableName = System.getenv("TableName");
    DynamoDBMapper mapper = InternalDataStorageClient.createDynamoDBMapper(databaseTableName);
    internalDataStorageClient = new InternalDataStorageClient(mapper);
}

Here is the JUnit test:

Source Link
totsubo
  • 129
  • 2
Loading