Skip to content

Commit

Permalink
DT-937 - Remove nested threadpool (#1852)
Browse files Browse the repository at this point in the history 
* Remove nested threadpool

* Only remove thread pool in the one, nested pool instance

* Update TableDirectoryDaoTest to Junit5

And clean up

* Add unit test
  • Loading branch information
@snf2ye
snf2ye authored Nov 13, 2024
1 parent d5d2c3c commit e8e8f19
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 24 deletions.
13 changes: 12 additions & 1 deletion src/main/java/bio/terra/service/filedata/azure/tables/TableDirectoryDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ public void addEntriesToSnapshot(
// Store the batch of entries. This will override existing entries,
// but that is not the typical case and it is lower cost just overwrite
// rather than retrieve to avoid the write.
batchStoreDirectoryEntry(snapshotTableServiceClient, snapshotId, snapshotEntries);
storeDirectoryEntries(snapshotTableServiceClient, snapshotId, snapshotEntries);

return null;
}));
Expand Down Expand Up @@ -498,4 +498,15 @@ void batchStoreDirectoryEntry(
createEntityForPath(tableClient, snapshotId, tableName, snapshotEntry)))
.toList());
}

// Store the directory entries in the snapshot table sequentially, without a thread pool
void storeDirectoryEntries(
TableServiceClient snapshotTableServiceClient,
UUID snapshotId,
List<FireStoreDirectoryEntry> snapshotEntries) {
String tableName = StorageTableName.SNAPSHOT.toTableName(snapshotId);
TableClient tableClient = snapshotTableServiceClient.getTableClient(tableName);
snapshotEntries.forEach(
snapshotEntry -> createEntityForPath(tableClient, snapshotId, tableName, snapshotEntry));
}
}
59 changes: 36 additions & 23 deletions src/test/java/bio/terra/service/filedata/azure/tables/TableDirectoryDaoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.when;

import bio.terra.common.EmbeddedDatabaseTest;
Expand All @@ -16,6 +17,7 @@
import bio.terra.service.filedata.google.firestore.FireStoreDirectoryEntry;
import bio.terra.service.resourcemanagement.azure.AzureAuthService;
import com.azure.core.http.rest.PagedIterable;
import com.azure.core.http.rest.Response;
import com.azure.data.tables.TableClient;
import com.azure.data.tables.TableServiceClient;
import com.azure.data.tables.models.TableEntity;
Expand All @@ -25,28 +27,26 @@
import java.util.List;
import java.util.UUID;
import java.util.stream.Stream;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.junit4.SpringRunner;

@RunWith(SpringRunner.class)
@SpringBootTest
@AutoConfigureMockMvc
@ActiveProfiles({"google", "unittest"})
@Category(Unit.class)
@Tag(Unit.TAG)
@EmbeddedDatabaseTest
public class TableDirectoryDaoTest {
class TableDirectoryDaoTest {
private static final String FULL_PATH = "/directory/file.json";
private static final UUID DATASET_ID = UUID.randomUUID();
private static final UUID SNAPSHOT_ID = UUID.randomUUID();
private static final String PARTITION_KEY = DATASET_ID + " _dr_ directory";
private static final String ROW_KEY = " _dr_ directory file.json";
private static final String NONEXISTENT_PATH = "/directory/nonexistent.json";
Expand All @@ -60,8 +60,8 @@ public class TableDirectoryDaoTest {
@MockBean private TableClient tableClient;
@Autowired private TableDirectoryDao dao;

@Before
public void setUp() {
@BeforeEach
void setUp() {
dao = spy(dao);
when(authService.getTableServiceClient(any(), any(), any())).thenReturn(tableServiceClient);
when(tableServiceClient.getTableClient(any())).thenReturn(tableClient);
Expand Down Expand Up @@ -90,15 +90,15 @@ public void setUp() {
}

@Test
public void testRetrieveByPath() {
void testRetrieveByPath() {
when(tableClient.getEntity(PARTITION_KEY, ROW_KEY)).thenReturn(entity);
FireStoreDirectoryEntry response =
dao.retrieveByPath(
tableServiceClient,
DATASET_ID,
StorageTableName.DATASET.toTableName(DATASET_ID),
FULL_PATH);
assertEquals("The same entry is returned", directoryEntry, response);
assertThat("The same entry is returned", directoryEntry, equalTo(response));

when(tableClient.getEntity(PARTITION_KEY, NONEXISTENT_ROW_KEY))
.thenThrow(TableServiceException.class);
Expand All @@ -108,11 +108,11 @@ public void testRetrieveByPath() {
DATASET_ID,
StorageTableName.DATASET.toTableName(DATASET_ID),
NONEXISTENT_PATH);
assertNull("The entry does not exist", nonExistentEntry);
assertThat("The entry does not exist", nonExistentEntry, is(nullValue()));
}

@Test
public void testRetrieveByFileId() {
void testRetrieveByFileId() {
PagedIterable<TableEntity> mockPagedIterable = mock(PagedIterable.class);
Iterator<TableEntity> mockIterator = mock(Iterator.class);
when(mockIterator.hasNext()).thenReturn(true, false);
Expand All @@ -136,7 +136,7 @@ public void testRetrieveByFileId() {
}

@Test
public void testRetrieveByFileIdNotFound() {
void testRetrieveByFileIdNotFound() {
PagedIterable<TableEntity> mockPagedIterable = mock(PagedIterable.class);
Iterator<TableEntity> mockIterator = mock(Iterator.class);
when(mockIterator.hasNext()).thenReturn(false);
Expand All @@ -146,11 +146,11 @@ public void testRetrieveByFileIdNotFound() {
FireStoreDirectoryEntry response =
dao.retrieveById(
tableServiceClient, StorageTableName.DATASET.toTableName(DATASET_ID), "nonexistentId");
assertNull("The entry does not exist", response);
assertThat("The entry does not exist", response, is(nullValue()));
}

@Test
public void validateRefIdsFindsMissingRecords() {
void validateRefIdsFindsMissingRecords() {
PagedIterable<TableEntity> mockPagedIterable = mock(PagedIterable.class);
Iterator<TableEntity> mockIterator = mock(Iterator.class);
when(mockIterator.hasNext()).thenReturn(false);
Expand All @@ -160,19 +160,32 @@ public void validateRefIdsFindsMissingRecords() {
String missingId = UUID.randomUUID().toString();
List<String> refIds = List.of(missingId);
List<String> response = dao.validateRefIds(tableServiceClient, DATASET_ID, refIds);
assertEquals(response.get(0), missingId);
assertThat(response.get(0), equalTo(missingId));
}

@Test
public void testEnumerateDirectory() {
void testEnumerateDirectory() {
PagedIterable<TableEntity> mockPagedIterable = mock(PagedIterable.class);
Stream<TableEntity> mockStream = List.of(entity).stream();
Stream<TableEntity> mockStream = Stream.of(entity);
when(mockPagedIterable.stream()).thenReturn(mockStream);
when(tableClient.listEntities(any(), any(), any())).thenReturn(mockPagedIterable);

List<FireStoreDirectoryEntry> response =
dao.enumerateDirectory(
tableServiceClient, StorageTableName.DATASET.toTableName(DATASET_ID), FILE_ID);
assertEquals(response.get(0), directoryEntry);
assertThat(response.get(0), equalTo(directoryEntry));
}

@Test
void storeDirectoryEntries() {
FireStoreDirectoryEntry entry1 = new FireStoreDirectoryEntry().name("file1").path("path1");
FireStoreDirectoryEntry entry2 = new FireStoreDirectoryEntry().name("file2").path("path2");
Response<Void> responseMock = mock(Response.class);
when(responseMock.getStatusCode()).thenReturn(200);
when(tableClient.upsertEntityWithResponse(any(), any(), any(), any())).thenReturn(responseMock);

dao.storeDirectoryEntries(tableServiceClient, SNAPSHOT_ID, List.of(entry1, entry2));

Mockito.verify(tableClient, times(2)).upsertEntityWithResponse(any(), any(), any(), any());
}
}

0 comments on commit e8e8f19

Please sign in to comment.