Skip to content

Commit

Permalink
JHP-103: Check for xdat_user table before creating JupyterHub user ac…
Browse files Browse the repository at this point in the history
…count (#2)
  • Loading branch information
andylassiter authored Jul 19, 2024
1 parent 4841a0f commit b8cd617
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 7 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.MD
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- [JHP-103]: Check for xdat_user table before creating JupyterHub user account.

## [1.2.0] - 2024-06-27

### Added
Expand Down Expand Up @@ -80,4 +84,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[JHP-88]: https://radiologics.atlassian.net/jira/software/c/projects/JHP/issues/JHP-88
[JHP-89]: https://radiologics.atlassian.net/jira/software/c/projects/JHP/issues/JHP-89
[JHP-91]: https://radiologics.atlassian.net/jira/software/c/projects/JHP/issues/JHP-91
[JHP-92]: https://radiologics.atlassian.net/jira/software/c/projects/JHP/issues/JHP-92
[JHP-92]: https://radiologics.atlassian.net/jira/software/c/projects/JHP/issues/JHP-92
[JHP-103]: https://radiologics.atlassian.net/jira/software/c/projects/JHP/issues/JHP-103
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.nrg.xnatx.plugins.jupyterhub.initialize;

import lombok.extern.slf4j.Slf4j;
import org.nrg.framework.orm.DatabaseHelper;
import org.nrg.xapi.rest.users.UsersApi;
import org.nrg.xdat.security.helpers.Users;
import org.nrg.xdat.security.services.RoleHolder;
Expand All @@ -16,6 +17,8 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import java.sql.SQLException;

/**
* Initializing task which creates a user account for JupyterHub allowing it to communicate with XNAT
*/
Expand All @@ -28,18 +31,22 @@ public class JupyterHubUserInitializer extends AbstractInitializingTask {
private final XFTManagerHelper xftManagerHelper;
private final XnatAppInfo appInfo;
private final SystemHelper systemHelper;
private final DatabaseHelper databaseHelper;

@Autowired
public JupyterHubUserInitializer(final UserManagementServiceI userManagementService,
final RoleHolder roleHolder,
final XFTManagerHelper xftManagerHelper,
final XnatAppInfo appInfo,
final SystemHelper systemHelper) {
final SystemHelper systemHelper,
final DatabaseHelper databaseHelper) {

this.userManagementService = userManagementService;
this.roleHolder = roleHolder;
this.xftManagerHelper = xftManagerHelper;
this.appInfo = appInfo;
this.systemHelper = systemHelper;
this.databaseHelper = databaseHelper;
}

@Override
Expand All @@ -56,9 +63,16 @@ public String getTaskName() {
protected void callImpl() throws InitializingTaskException {
log.info("Creating `jupyterhub` user account.");

if (!xftManagerHelper.isInitialized()) {
log.info("XFT not initialized, deferring execution.");
throw new InitializingTaskException(InitializingTaskException.Level.RequiresInitialization);
try {
if (!xftManagerHelper.isInitialized() || !databaseHelper.tableExists("xdat_user")) {
log.info("XFT and database not initialized, deferring execution.");
throw new InitializingTaskException(InitializingTaskException.Level.RequiresInitialization);
}
} catch (SQLException e) {
throw new InitializingTaskException(
InitializingTaskException.Level.Error,
"An error occurred trying to access the database to check for the table 'xdat_user'.",
e);
}

if (!appInfo.isInitialized()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.nrg.xnatx.plugins.jupyterhub.config;

import org.nrg.framework.orm.DatabaseHelper;
import org.nrg.xdat.security.services.RoleHolder;
import org.nrg.xdat.security.services.UserManagementServiceI;
import org.nrg.xnat.services.XnatAppInfo;
Expand All @@ -19,13 +20,15 @@ public JupyterHubUserInitializer defaultJupyterHubUserInitializer(final UserMana
final RoleHolder mockRoleHolder,
final XFTManagerHelper mockXFTManagerHelper,
final XnatAppInfo mockXnatAppInfo,
final SystemHelper mockSystemHelper) {
final SystemHelper mockSystemHelper,
final DatabaseHelper mockDatabaseHelper) {
return new JupyterHubUserInitializer(
mockUserManagementService,
mockRoleHolder,
mockXFTManagerHelper,
mockXnatAppInfo,
mockSystemHelper
mockSystemHelper,
mockDatabaseHelper
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.mockito.Mockito;
import org.nrg.framework.configuration.ConfigPaths;
import org.nrg.framework.orm.DatabaseHelper;
import org.nrg.framework.services.NrgEventServiceI;
import org.nrg.framework.services.SerializerService;
import org.nrg.framework.utilities.OrderedProperties;
Expand Down Expand Up @@ -152,6 +153,11 @@ public XnatAppInfo mockXnatAppInfo() {
return Mockito.mock(XnatAppInfo.class);
}

@Bean
public DatabaseHelper mockDatabaseHelper() {
return Mockito.mock(DatabaseHelper.class);
}

@Bean
public ComputeEnvironmentConfigService mockComputeEnvironmentConfigService() {
return Mockito.mock(ComputeEnvironmentConfigService.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.nrg.framework.orm.DatabaseHelper;
import org.nrg.xdat.security.services.RoleServiceI;
import org.nrg.xdat.security.services.UserManagementServiceI;
import org.nrg.xft.event.EventDetails;
Expand All @@ -31,6 +32,7 @@ public class JupyterHubUserInitializerTest {
@Autowired private XnatAppInfo mockXnatAppInfo;
@Autowired private RoleServiceI mockRoleService;
@Autowired private SystemHelper mockSystemHelper;
@Autowired private DatabaseHelper mockDatabaseHelper;

private final String username = "jupyterhub";

Expand All @@ -41,6 +43,7 @@ public void after() {
Mockito.reset(mockRoleService);
Mockito.reset(mockXnatAppInfo);
Mockito.reset(mockSystemHelper);
Mockito.reset(mockDatabaseHelper);
}

@Test(expected = InitializingTaskException.class)
Expand All @@ -52,10 +55,21 @@ public void test_XFTManagerNotInitialized() throws Exception {
jupyterHubUserInitializer.callImpl();
}

@Test(expected = InitializingTaskException.class)
public void test_DatabaseTableNotExists() throws Exception {
// XFT initialized but database table not exists
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(false);

// Should throw InitializingTaskException
jupyterHubUserInitializer.callImpl();
}

@Test(expected = InitializingTaskException.class)
public void test_AppNotInitialized() throws Exception {
// XFT initialized but app not initialized
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(true);
when(mockXnatAppInfo.isInitialized()).thenReturn(false);

// Should throw InitializingTaskException
Expand All @@ -67,6 +81,7 @@ public void test_JHUserAlreadyExists() throws Exception {
// Setup
// XFT initialized and user already exists
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(true);
when(mockXnatAppInfo.isInitialized()).thenReturn(true);
when(mockUserManagementService.exists(username)).thenReturn(true);

Expand All @@ -87,6 +102,7 @@ public void test_JHUserAlreadyExists() throws Exception {
public void test_FailedToSaveUser() throws Exception {
// Setup
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(true);
when(mockXnatAppInfo.isInitialized()).thenReturn(true);
when(mockUserManagementService.exists(username)).thenReturn(false);
when(mockUserManagementService.createUser()).thenReturn(mock(UserI.class));
Expand All @@ -110,6 +126,7 @@ public void test_FailedToSaveUser() throws Exception {
public void test_FailedAddUserRole() throws Exception {
// Setup
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(true);
when(mockXnatAppInfo.isInitialized()).thenReturn(true);
when(mockUserManagementService.exists(username)).thenReturn(false);
when(mockUserManagementService.createUser()).thenReturn(mock(UserI.class));
Expand All @@ -134,6 +151,7 @@ public void test_FailedAddUserRole() throws Exception {
public void test_FailedAddUserRole_Exception() throws Exception {
// Setup
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(true);
when(mockXnatAppInfo.isInitialized()).thenReturn(true);
when(mockUserManagementService.exists(username)).thenReturn(false);
when(mockUserManagementService.createUser()).thenReturn(mock(UserI.class));
Expand All @@ -159,6 +177,7 @@ public void test_FailedAddUserRole_Exception() throws Exception {
public void test_UserCreated() throws Exception {
// Setup
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(true);
when(mockXnatAppInfo.isInitialized()).thenReturn(true);
when(mockUserManagementService.exists(username)).thenReturn(false);
UserI mockUser = mock(UserI.class);
Expand Down Expand Up @@ -187,6 +206,7 @@ public void test_UserCreated() throws Exception {
public void test_UserCreatedFromEnv() throws Exception {
// Setup
when(mockXFTManagerHelper.isInitialized()).thenReturn(true);
when(mockDatabaseHelper.tableExists("xdat_user")).thenReturn(true);
when(mockXnatAppInfo.isInitialized()).thenReturn(true);
when(mockUserManagementService.exists(username)).thenReturn(false);
UserI mockUser = mock(UserI.class);
Expand Down

0 comments on commit b8cd617

Please sign in to comment.