Skip to content

Commit

Permalink
InstanceConfiguration API fixes: Default Workspace selection and remo…
Browse files Browse the repository at this point in the history
…val of workspaceId from `/setup` call (#8617)
  • Loading branch information
pmossman committed Aug 31, 2023
1 parent eea536a commit 6b4546f
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 99 deletions.
4 changes: 0 additions & 4 deletions airbyte-api/src/main/openapi/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7553,15 +7553,11 @@ components:
InstanceConfigurationSetupRequestBody:
type: object
required:
- workspaceId
- email
- anonymousDataCollection
- initialSetupComplete
- displaySetupWizard
properties:
workspaceId:
type: string
format: uuid
email:
type: string
anonymousDataCollection:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
import io.airbyte.config.StandardWorkspace;
import io.airbyte.config.User;
import io.airbyte.config.persistence.ConfigNotFoundException;
import io.airbyte.config.persistence.ConfigRepository;
import io.airbyte.config.persistence.OrganizationPersistence;
import io.airbyte.config.persistence.UserPersistence;
import io.airbyte.config.persistence.WorkspacePersistence;
import io.airbyte.validation.json.JsonValidationException;
import io.micronaut.context.annotation.Value;
import jakarta.inject.Singleton;
Expand All @@ -42,7 +42,7 @@ public class InstanceConfigurationHandler {
private final AirbyteEdition airbyteEdition;
private final Optional<AirbyteKeycloakConfiguration> airbyteKeycloakConfiguration;
private final Optional<ActiveAirbyteLicense> activeAirbyteLicense;
private final ConfigRepository configRepository;
private final WorkspacePersistence workspacePersistence;
private final WorkspacesHandler workspacesHandler;
private final UserPersistence userPersistence;
private final OrganizationPersistence organizationPersistence;
Expand All @@ -54,22 +54,23 @@ public InstanceConfigurationHandler(@Value("${airbyte.webapp-url:null}") final S
final AirbyteEdition airbyteEdition,
final Optional<AirbyteKeycloakConfiguration> airbyteKeycloakConfiguration,
final Optional<ActiveAirbyteLicense> activeAirbyteLicense,
final ConfigRepository configRepository,
final WorkspacePersistence workspacePersistence,
final WorkspacesHandler workspacesHandler,
final UserPersistence userPersistence,
final OrganizationPersistence organizationPersistence) {
this.webappUrl = webappUrl;
this.airbyteEdition = airbyteEdition;
this.airbyteKeycloakConfiguration = airbyteKeycloakConfiguration;
this.activeAirbyteLicense = activeAirbyteLicense;
this.configRepository = configRepository;
this.workspacePersistence = workspacePersistence;
this.workspacesHandler = workspacesHandler;
this.userPersistence = userPersistence;
this.organizationPersistence = organizationPersistence;
}

public InstanceConfigurationResponse getInstanceConfiguration() throws IOException, ConfigNotFoundException {
final StandardWorkspace defaultWorkspace = getDefaultWorkspace();
public InstanceConfigurationResponse getInstanceConfiguration() throws IOException {
final UUID defaultOrganizationId = getDefaultOrganizationId();
final StandardWorkspace defaultWorkspace = getDefaultWorkspace(defaultOrganizationId);

return new InstanceConfigurationResponse()
.webappUrl(webappUrl)
Expand All @@ -78,20 +79,23 @@ public InstanceConfigurationResponse getInstanceConfiguration() throws IOExcepti
.auth(getAuthConfiguration())
.initialSetupComplete(defaultWorkspace.getInitialSetupComplete())
.defaultUserId(getDefaultUserId())
.defaultOrganizationId(getDefaultOrganizationId())
.defaultOrganizationId(defaultOrganizationId)
.defaultWorkspaceId(defaultWorkspace.getWorkspaceId());
}

public InstanceConfigurationResponse setupInstanceConfiguration(final InstanceConfigurationSetupRequestBody requestBody)
throws IOException, JsonValidationException, ConfigNotFoundException {

final UUID defaultOrganizationId = getDefaultOrganizationId();
final StandardWorkspace defaultWorkspace = getDefaultWorkspace(defaultOrganizationId);

// Update the default organization and user with the provided information
updateDefaultOrganization(requestBody);
updateDefaultUser(requestBody);

// Update the underlying workspace to mark the initial setup as complete
workspacesHandler.updateWorkspace(new WorkspaceUpdate()
.workspaceId(requestBody.getWorkspaceId())
.workspaceId(defaultWorkspace.getWorkspaceId())
.email(requestBody.getEmail())
.displaySetupWizard(requestBody.getDisplaySetupWizard())
.anonymousDataCollection(requestBody.getAnonymousDataCollection())
Expand Down Expand Up @@ -136,7 +140,7 @@ private void updateDefaultUser(final InstanceConfigurationSetupRequestBody reque
userPersistence.writeUser(defaultUser);
}

private UUID getDefaultOrganizationId() throws IOException, ConfigNotFoundException {
private UUID getDefaultOrganizationId() throws IOException {
return organizationPersistence.getDefaultOrganization()
.orElseThrow(() -> new IllegalStateException("Default organization does not exist."))
.getOrganizationId();
Expand All @@ -157,14 +161,12 @@ private void updateDefaultOrganization(final InstanceConfigurationSetupRequestBo
organizationPersistence.updateOrganization(defaultOrganization);
}

// Currently, the default workspace is simply the first workspace created by the bootloader. This is
// hacky, but
// historically, the first workspace is used to store instance-level preferences.
// TODO introduce a proper means of persisting instance-level preferences instead of using the first
// workspace as a proxy.
private StandardWorkspace getDefaultWorkspace() throws IOException {
return configRepository.listStandardWorkspaces(true).stream().findFirst()
.orElseThrow(() -> new IllegalStateException("Default workspace does not exist."));
// Historically, instance setup for an OSS installation of Airbyte was stored on the one and only
// workspace that was created for the instance. Now that OSS supports multiple workspaces, we
// use the default Organization ID to select a workspace to use for instance setup. This is a hack.
// TODO persist instance configuration to a separate resource, rather than using a workspace.
private StandardWorkspace getDefaultWorkspace(final UUID organizationId) throws IOException {
return workspacePersistence.getDefaultWorkspaceForOrganization(organizationId);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package io.airbyte.commons.server.handlers;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -24,15 +25,15 @@
import io.airbyte.config.StandardWorkspace;
import io.airbyte.config.User;
import io.airbyte.config.persistence.ConfigNotFoundException;
import io.airbyte.config.persistence.ConfigRepository;
import io.airbyte.config.persistence.OrganizationPersistence;
import io.airbyte.config.persistence.UserPersistence;
import io.airbyte.config.persistence.WorkspacePersistence;
import io.airbyte.validation.json.JsonValidationException;
import java.io.IOException;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
Expand All @@ -52,7 +53,7 @@ class InstanceConfigurationHandlerTest {
private static final String DEFAULT_USER_NAME = "Default User Name";

@Mock
private ConfigRepository mConfigRepository;
private WorkspacePersistence mWorkspacePersistence;
@Mock
private UserPersistence mUserPersistence;
@Mock
Expand All @@ -72,17 +73,6 @@ void setup() throws IOException {

activeAirbyteLicense = new ActiveAirbyteLicense();
activeAirbyteLicense.setLicense(new AirbyteLicense(LicenseType.PRO));

when(mUserPersistence.getDefaultUser()).thenReturn(
Optional.of(new User()
.withUserId(USER_ID)
.withName(DEFAULT_USER_NAME)));

when(mOrganizationPersistence.getDefaultOrganization()).thenReturn(
Optional.of(new Organization()
.withOrganizationId(ORGANIZATION_ID)
.withName(DEFAULT_ORG_NAME)
.withUserId(USER_ID)));
}

@ParameterizedTest
Expand All @@ -92,22 +82,16 @@ void setup() throws IOException {
"false, true",
"false, false"
})
void testGetInstanceConfiguration(final boolean isPro, final boolean isInitialSetupComplete)
throws IOException, ConfigNotFoundException {
when(mConfigRepository.listStandardWorkspaces(true)).thenReturn(
List.of(new StandardWorkspace()
void testGetInstanceConfiguration(final boolean isPro, final boolean isInitialSetupComplete) throws IOException {
stubGetDefaultUser();
stubGetDefaultOrganization();

when(mWorkspacePersistence.getDefaultWorkspaceForOrganization(ORGANIZATION_ID)).thenReturn(
new StandardWorkspace()
.withWorkspaceId(WORKSPACE_ID)
.withInitialSetupComplete(isInitialSetupComplete)));
.withInitialSetupComplete(isInitialSetupComplete));

instanceConfigurationHandler = new InstanceConfigurationHandler(
WEBAPP_URL,
isPro ? AirbyteEdition.PRO : AirbyteEdition.COMMUNITY,
isPro ? Optional.of(keycloakConfiguration) : Optional.empty(),
isPro ? Optional.of(activeAirbyteLicense) : Optional.empty(),
mConfigRepository,
mWorkspacesHandler,
mUserPersistence,
mOrganizationPersistence);
instanceConfigurationHandler = getInstanceConfigurationHandler(isPro);

final InstanceConfigurationResponse expected = new InstanceConfigurationResponse()
.edition(isPro ? EditionEnum.PRO : EditionEnum.COMMUNITY)
Expand All @@ -126,6 +110,25 @@ void testGetInstanceConfiguration(final boolean isPro, final boolean isInitialSe
assertEquals(expected, actual);
}

@Test
void testSetupInstanceConfigurationAlreadySetup() throws IOException {
stubGetDefaultOrganization();

when(mWorkspacePersistence.getDefaultWorkspaceForOrganization(ORGANIZATION_ID)).thenReturn(
new StandardWorkspace()
.withWorkspaceId(WORKSPACE_ID)
.withInitialSetupComplete(true)); // already setup, should trigger an error

instanceConfigurationHandler = getInstanceConfigurationHandler(true);

assertThrows(IllegalStateException.class, () -> instanceConfigurationHandler.setupInstanceConfiguration(
new InstanceConfigurationSetupRequestBody()
.email("[email protected]")
.displaySetupWizard(false)
.initialSetupComplete(false)
.anonymousDataCollection(false)));
}

@ParameterizedTest
@CsvSource({
"true, true",
Expand All @@ -135,23 +138,20 @@ void testGetInstanceConfiguration(final boolean isPro, final boolean isInitialSe
})
void testSetupInstanceConfiguration(final boolean userNamePresent, final boolean orgNamePresent)
throws IOException, JsonValidationException, ConfigNotFoundException {
when(mConfigRepository.listStandardWorkspaces(true))
.thenReturn(List.of(new StandardWorkspace()
.withWorkspaceId(WORKSPACE_ID)
.withInitialSetupComplete(true))); // after the handler's update, the workspace should have initialSetupComplete: true when retrieved

instanceConfigurationHandler = new InstanceConfigurationHandler(
WEBAPP_URL,
AirbyteEdition.PRO,
Optional.of(keycloakConfiguration),
Optional.of(activeAirbyteLicense),
mConfigRepository,
mWorkspacesHandler,
mUserPersistence,
mOrganizationPersistence);
stubGetDefaultOrganization();
stubGetDefaultUser();

// first time default workspace is fetched, initial setup complete is false.
// second time is after the workspace is updated, so initial setup complete is true.
when(mWorkspacePersistence.getDefaultWorkspaceForOrganization(ORGANIZATION_ID))
.thenReturn(
new StandardWorkspace().withWorkspaceId(WORKSPACE_ID).withInitialSetupComplete(false))
.thenReturn(
new StandardWorkspace().withWorkspaceId(WORKSPACE_ID).withInitialSetupComplete(true));

instanceConfigurationHandler = getInstanceConfigurationHandler(true);

final String userName = userNamePresent ? "test user" : DEFAULT_USER_NAME;
final String orgName = orgNamePresent ? "test org" : DEFAULT_ORG_NAME;
final String email = "[email protected]";

final InstanceConfigurationResponse expected = new InstanceConfigurationResponse()
Expand All @@ -166,28 +166,38 @@ void testSetupInstanceConfiguration(final boolean userNamePresent, final boolean
.defaultOrganizationId(ORGANIZATION_ID)
.defaultWorkspaceId(WORKSPACE_ID);

final InstanceConfigurationResponse actual = instanceConfigurationHandler.setupInstanceConfiguration(
new InstanceConfigurationSetupRequestBody()
.workspaceId(WORKSPACE_ID)
.email(email)
.initialSetupComplete(true)
.anonymousDataCollection(true)
.displaySetupWizard(true)
.userName(userName)
.organizationName(orgName));
final InstanceConfigurationSetupRequestBody requestBody = new InstanceConfigurationSetupRequestBody()
.email(email)
.displaySetupWizard(true)
.anonymousDataCollection(true)
.initialSetupComplete(true);

String expectedUserName = DEFAULT_USER_NAME;
if (userNamePresent) {
expectedUserName = "test user";
requestBody.setUserName(expectedUserName);
}

String expectedOrgName = DEFAULT_ORG_NAME;
if (orgNamePresent) {
expectedOrgName = "test org";
requestBody.setOrganizationName(expectedOrgName);
}

final InstanceConfigurationResponse actual = instanceConfigurationHandler.setupInstanceConfiguration(requestBody);

assertEquals(expected, actual);

// verify the user was updated with the email and name from the request
verify(mUserPersistence).writeUser(eq(new User()
.withUserId(USER_ID)
.withEmail(email)
.withName(userName)));
.withName(expectedUserName)));

// verify the organization was updated with the name from the request
verify(mOrganizationPersistence).updateOrganization(eq(new Organization()
.withOrganizationId(ORGANIZATION_ID)
.withName(orgName)
.withName(expectedOrgName)
.withEmail(email)
.withUserId(USER_ID)));

Expand All @@ -199,4 +209,31 @@ void testSetupInstanceConfiguration(final boolean userNamePresent, final boolean
.initialSetupComplete(true)));
}

private void stubGetDefaultUser() throws IOException {
when(mUserPersistence.getDefaultUser()).thenReturn(
Optional.of(new User()
.withUserId(USER_ID)
.withName(DEFAULT_USER_NAME)));
}

private void stubGetDefaultOrganization() throws IOException {
when(mOrganizationPersistence.getDefaultOrganization()).thenReturn(
Optional.of(new Organization()
.withOrganizationId(ORGANIZATION_ID)
.withName(DEFAULT_ORG_NAME)
.withUserId(USER_ID)));
}

private InstanceConfigurationHandler getInstanceConfigurationHandler(final boolean isPro) {
return new InstanceConfigurationHandler(
WEBAPP_URL,
isPro ? AirbyteEdition.PRO : AirbyteEdition.COMMUNITY,
isPro ? Optional.of(keycloakConfiguration) : Optional.empty(),
isPro ? Optional.of(activeAirbyteLicense) : Optional.empty(),
mWorkspacePersistence,
mWorkspacesHandler,
mUserPersistence,
mOrganizationPersistence);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,20 @@ public List<StandardWorkspace> listWorkspacesByOrganizationId(final ResourcesByO
}

/**
* Find the workspace with the given ID and check if its organization ID is null. If so, update it.
* Otherwise, log a warning and do nothing.
* Fetch the oldest, non-tombstoned Workspace that belongs to the given Organization.
*/
public void setOrganizationIdIfNull(final UUID workspaceId, final UUID organizationId) throws IOException {
database.transaction(ctx -> {
final boolean isExistingWorkspace = ctx.fetchExists(ctx.selectFrom(WORKSPACE).where(WORKSPACE.ID.eq(workspaceId)));
if (isExistingWorkspace) {
final boolean isNullOrganizationId =
ctx.fetchExists(ctx.selectFrom(WORKSPACE).where(WORKSPACE.ID.eq(workspaceId)).and(WORKSPACE.ORGANIZATION_ID.isNull()));
if (isNullOrganizationId) {
ctx.update(WORKSPACE)
.set(WORKSPACE.ORGANIZATION_ID, organizationId)
.where(WORKSPACE.ID.eq(workspaceId))
.execute();
} else {
log.warn("Workspace with ID {} already has an organization ID set. Skipping update.", workspaceId);
}
} else {
log.warn("Workspace with ID {} does not exist. Skipping update.", workspaceId);
}
return null;
});
public StandardWorkspace getDefaultWorkspaceForOrganization(final UUID organizationId) throws IOException {
return database.query(ctx -> ctx.select(WORKSPACE.asterisk())
.from(WORKSPACE)
.where(WORKSPACE.ORGANIZATION_ID.eq(organizationId))
.and(WORKSPACE.TOMBSTONE.notEqual(true))
.orderBy(WORKSPACE.CREATED_AT.asc())
.limit(1)
.fetch())
.stream()
.map(DbConverter::buildStandardWorkspace)
.findFirst()
.orElseThrow(() -> new RuntimeException("No workspace found for organization: " + organizationId));
}

}
Loading

0 comments on commit 6b4546f

Please sign in to comment.