Skip to content

Commit

Permalink
AJ-1591: no-db solution for instance-to-workspace association (#492)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidangb authored Feb 5, 2024
1 parent 0eacd2f commit e73b3c5
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@
import static org.databiosphere.workspacedataservice.service.RecordUtils.validateVersion;

import java.util.List;
import java.util.Objects;
import java.util.UUID;
import org.databiosphere.workspacedataservice.activitylog.ActivityLogger;
import org.databiosphere.workspacedataservice.dao.InstanceDao;
import org.databiosphere.workspacedataservice.sam.SamDao;
import org.databiosphere.workspacedataservice.service.model.exception.AuthorizationException;
import org.databiosphere.workspacedataservice.service.model.exception.MissingObjectException;
import org.databiosphere.workspacedataservice.shared.model.InstanceId;
import org.databiosphere.workspacedataservice.shared.model.WorkspaceId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
import org.springframework.web.server.ResponseStatusException;
Expand All @@ -22,12 +26,31 @@ public class InstanceService {
private final SamDao samDao;
private final ActivityLogger activityLogger;

private final WorkspaceId workspaceId;

private static final Logger LOGGER = LoggerFactory.getLogger(InstanceService.class);

public InstanceService(InstanceDao instanceDao, SamDao samDao, ActivityLogger activityLogger) {
public InstanceService(
InstanceDao instanceDao,
SamDao samDao,
ActivityLogger activityLogger,
@Value("${twds.instance.workspace-id:}") String workspaceIdProperty) {
this.instanceDao = instanceDao;
this.samDao = samDao;
this.activityLogger = activityLogger;
// jump through a few hoops to initialize workspaceId to ensure validity.
// workspaceId will be null if the WORKSPACE_ID env var is unset/not a valid UUID.
UUID workspaceUuid = null;
try {
workspaceUuid = UUID.fromString(workspaceIdProperty);
} catch (Exception e) {
LOGGER.warn("WORKSPACE_ID could not be parsed into a UUID: {}", workspaceIdProperty);
}
if (workspaceUuid == null) {
workspaceId = null;
} else {
workspaceId = WorkspaceId.of(workspaceUuid);
}
}

public List<UUID> listInstances(String version) {
Expand Down Expand Up @@ -90,4 +113,22 @@ public void validateInstance(UUID instanceId) {
throw new MissingObjectException("Instance");
}
}

/**
* Return the workspace that contains the specified instance. The logic for this method is:
*
* <p>- if the WORKSPACE_ID env var is set, return its value. This will be true for all data plane
* WDSes, as long as WDS is deployed per-workspace.
*
* <p>- else, return the InstanceId value as the WorkspaceId. This perpetuates the assumption that
* instance and workspace ids are the same. But, this will be true in the GCP control plane as
* long as Rawls continues to manage data tables, since "instance" is a WDS concept and does not
* exist in Rawls.
*
* @param instanceId the instance for which to look up the workspace
* @return the workspace containing the given instance.
*/
public WorkspaceId getWorkspaceId(InstanceId instanceId) {
return Objects.requireNonNullElseGet(workspaceId, () -> WorkspaceId.of(instanceId.id()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ public static InstanceId of(UUID id) {
return new InstanceId(id);
}

/**
* Create a new InstanceId using the given id
*
* @param id the instance's id
* @return new InstanceId
*/
public static InstanceId fromString(String id) {
return InstanceId.of(UUID.fromString(id));
}

@Override
public String toString() {
return this.id().toString();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.databiosphere.workspacedataservice.shared.model;

import java.util.UUID;

/**
* Model to represent a workspace id, which currently is a UUID. Since we use UUIDs throughout our
* code for multiple use cases, this wrapper class exists to help disambiguate between those UUIDs.
*
* @param id the workspace's id
*/
public record WorkspaceId(UUID id) {

// disallow nulls
public WorkspaceId {
if (id == null) {
throw new IllegalArgumentException("Id cannot be null");
}
}

/**
* Create a new WorkspaceId using the given id
*
* @param id the workspace's id
* @return new WorkspaceId
*/
public static WorkspaceId of(UUID id) {
return new WorkspaceId(id);
}

/**
* Create a new WorkspaceId using the given id
*
* @par public static WorkspaceId fromString(String id) { return
* WorkspaceId.of(UUID.fromString(id)); }am id the workspace's id
* @return new WorkspaceId
*/
@Override
public String toString() {
return this.id().toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,9 @@
import java.util.UUID;
import org.broadinstitute.dsde.workbench.client.sam.ApiException;
import org.broadinstitute.dsde.workbench.client.sam.api.ResourcesApi;
import org.databiosphere.workspacedataservice.activitylog.ActivityLogger;
import org.databiosphere.workspacedataservice.activitylog.ActivityLoggerConfig;
import org.databiosphere.workspacedataservice.dao.InstanceDao;
import org.databiosphere.workspacedataservice.dao.MockInstanceDaoConfig;
import org.databiosphere.workspacedataservice.retry.RestClientRetry;
import org.databiosphere.workspacedataservice.sam.SamClientFactory;
import org.databiosphere.workspacedataservice.sam.SamConfig;
import org.databiosphere.workspacedataservice.sam.SamDao;
import org.databiosphere.workspacedataservice.service.model.exception.AuthorizationException;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.mockito.Mockito;
Expand All @@ -30,21 +23,13 @@

@ActiveProfiles(profiles = "mock-instance-dao")
@DirtiesContext
@SpringBootTest(
classes = {
MockInstanceDaoConfig.class,
SamConfig.class,
ActivityLoggerConfig.class,
RestClientRetry.class
})
@SpringBootTest
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class InstanceServiceNoPermissionSamTest {

private InstanceService instanceService;
@Autowired private InstanceService instanceService;

@Autowired private InstanceDao instanceDao;
@Autowired private SamDao samDao;
@Autowired private ActivityLogger activityLogger;

// mock for the SamClientFactory; since this is a Spring bean we can use @MockBean
@MockBean SamClientFactory mockSamClientFactory;
Expand All @@ -53,11 +38,6 @@ class InstanceServiceNoPermissionSamTest {
// to mock it manually
final ResourcesApi mockResourcesApi = Mockito.mock(ResourcesApi.class);

@BeforeEach
void setUp() {
instanceService = new InstanceService(instanceDao, samDao, activityLogger);
}

@Test
void testCreateInstanceNoPermission() throws ApiException {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package org.databiosphere.workspacedataservice.service;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.UUID;
import org.databiosphere.workspacedataservice.shared.model.InstanceId;
import org.databiosphere.workspacedataservice.shared.model.WorkspaceId;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.TestPropertySource;

@DirtiesContext
@SpringBootTest
@TestPropertySource(properties = {"twds.instance.workspace-id="})
class InstanceServiceNoWorkspaceTest {

@Autowired private InstanceService instanceService;

@Value("${twds.instance.workspace-id:}")
private String workspaceIdProperty;

@Test
void assumptions() {
// ensure the test is set up correctly, with an empty twds.instance.workspace-id property
assertThat(workspaceIdProperty).isEmpty();
}

// when twds.instance.workspace-id is empty, instanceService.getWorkspaceId will echo the
// instanceid back as the workspace id
@Test
void getWorkspaceId() {
InstanceId instanceId = InstanceId.of(UUID.randomUUID());
assertEquals(WorkspaceId.of(instanceId.id()), instanceService.getWorkspaceId(instanceId));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,8 @@
import org.broadinstitute.dsde.workbench.client.sam.ApiException;
import org.broadinstitute.dsde.workbench.client.sam.api.ResourcesApi;
import org.databiosphere.workspacedataservice.activitylog.ActivityLogger;
import org.databiosphere.workspacedataservice.activitylog.ActivityLoggerConfig;
import org.databiosphere.workspacedataservice.dao.InstanceDao;
import org.databiosphere.workspacedataservice.dao.MockInstanceDaoConfig;
import org.databiosphere.workspacedataservice.retry.RestClientRetry;
import org.databiosphere.workspacedataservice.sam.SamClientFactory;
import org.databiosphere.workspacedataservice.sam.SamConfig;
import org.databiosphere.workspacedataservice.sam.SamDao;
import org.databiosphere.workspacedataservice.service.model.exception.AuthenticationException;
import org.databiosphere.workspacedataservice.service.model.exception.AuthorizationException;
Expand Down Expand Up @@ -55,21 +51,15 @@
*/
@ActiveProfiles(profiles = "mock-instance-dao")
@DirtiesContext
@SpringBootTest(
classes = {
MockInstanceDaoConfig.class,
SamConfig.class,
ActivityLoggerConfig.class,
RestClientRetry.class
})
@SpringBootTest
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@TestPropertySource(
properties = {
"twds.instance.workspace-id=123e4567-e89b-12d3-a456-426614174000"
}) // example uuid from https://en.wikipedia.org/wiki/Universally_unique_identifier
class InstanceServiceSamExceptionTest {

private InstanceService instanceService;
@Autowired private InstanceService instanceService;

@Autowired private InstanceDao instanceDao;
@Autowired private SamDao samDao;
Expand All @@ -87,8 +77,6 @@ class InstanceServiceSamExceptionTest {

@BeforeEach
void setUp() {
instanceService = new InstanceService(instanceDao, samDao, activityLogger);

// return the mock ResourcesApi from the mock SamClientFactory
given(mockSamClientFactory.getResourcesApi(null)).willReturn(mockResourcesApi);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,8 @@
import org.broadinstitute.dsde.workbench.client.sam.api.ResourcesApi;
import org.broadinstitute.dsde.workbench.client.sam.model.CreateResourceRequestV2;
import org.databiosphere.workspacedataservice.activitylog.ActivityLogger;
import org.databiosphere.workspacedataservice.activitylog.ActivityLoggerConfig;
import org.databiosphere.workspacedataservice.dao.InstanceDao;
import org.databiosphere.workspacedataservice.dao.MockInstanceDaoConfig;
import org.databiosphere.workspacedataservice.retry.RestClientRetry;
import org.databiosphere.workspacedataservice.sam.SamClientFactory;
import org.databiosphere.workspacedataservice.sam.SamConfig;
import org.databiosphere.workspacedataservice.sam.SamDao;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -31,21 +27,15 @@

@ActiveProfiles(profiles = "mock-instance-dao")
@DirtiesContext
@SpringBootTest(
classes = {
MockInstanceDaoConfig.class,
SamConfig.class,
ActivityLoggerConfig.class,
RestClientRetry.class
})
@SpringBootTest
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@TestPropertySource(
properties = {
"twds.instance.workspace-id=123e4567-e89b-12d3-a456-426614174000"
}) // example uuid from https://en.wikipedia.org/wiki/Universally_unique_identifier
class InstanceServiceSamTest {

private InstanceService instanceService;
@Autowired private InstanceService instanceService;

@Autowired private InstanceDao instanceDao;
@Autowired private SamDao samDao;
Expand All @@ -63,8 +53,6 @@ class InstanceServiceSamTest {

@BeforeEach
void setUp() throws ApiException {
instanceService = new InstanceService(instanceDao, samDao, activityLogger);

// return the mock ResourcesApi from the mock SamClientFactory
given(mockSamClientFactory.getResourcesApi(null)).willReturn(mockResourcesApi);
// Sam permission check will always return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,18 @@

import java.util.List;
import java.util.UUID;
import org.databiosphere.workspacedataservice.activitylog.ActivityLogger;
import org.databiosphere.workspacedataservice.activitylog.ActivityLoggerConfig;
import org.databiosphere.workspacedataservice.dao.InstanceDao;
import org.databiosphere.workspacedataservice.dao.MockInstanceDaoConfig;
import org.databiosphere.workspacedataservice.retry.RestClientRetry;
import org.databiosphere.workspacedataservice.sam.MockSamClientFactoryConfig;
import org.databiosphere.workspacedataservice.sam.SamConfig;
import org.databiosphere.workspacedataservice.sam.SamDao;
import org.databiosphere.workspacedataservice.service.model.exception.MissingObjectException;
import org.databiosphere.workspacedataservice.shared.model.InstanceId;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ActiveProfiles;
Expand All @@ -27,6 +26,7 @@
@DirtiesContext
@SpringBootTest(
classes = {
InstanceService.class,
MockInstanceDaoConfig.class,
SamConfig.class,
MockSamClientFactoryConfig.class,
Expand All @@ -35,17 +35,13 @@
})
class InstanceServiceTest {

private InstanceService instanceService;
@Autowired private InstanceService instanceService;
@Autowired private InstanceDao instanceDao;
@Autowired private SamDao samDao;
@Autowired private ActivityLogger activityLogger;

private static final UUID INSTANCE = UUID.fromString("111e9999-e89b-12d3-a456-426614174000");
@Value("${twds.instance.workspace-id:}")
private String workspaceIdProperty;

@BeforeEach
void setUp() {
instanceService = new InstanceService(instanceDao, samDao, activityLogger);
}
private static final UUID INSTANCE = UUID.fromString("111e9999-e89b-12d3-a456-426614174000");

@AfterEach
void tearDown() {
Expand Down Expand Up @@ -96,4 +92,11 @@ void deleteInstance() {
() -> instanceService.validateInstance(INSTANCE),
"validateInstance should have thrown an error");
}

@Test
void getWorkspaceId() {
assertEquals(
workspaceIdProperty,
instanceService.getWorkspaceId(InstanceId.of(UUID.randomUUID())).toString());
}
}

0 comments on commit e73b3c5

Please sign in to comment.