From b8cd6174672a03ff2321ba8b48a1f24c324f0a90 Mon Sep 17 00:00:00 2001 From: Andy Lassiter Date: Fri, 19 Jul 2024 16:15:36 -0600 Subject: [PATCH] JHP-103: Check for xdat_user table before creating JupyterHub user account (#2) --- CHANGELOG.MD | 7 +++++- .../initialize/JupyterHubUserInitializer.java | 22 +++++++++++++++---- .../JupyterHubUserInitializerConfig.java | 7 ++++-- .../plugins/jupyterhub/config/MockConfig.java | 6 +++++ .../JupyterHubUserInitializerTest.java | 20 +++++++++++++++++ 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.MD b/CHANGELOG.MD index 841005c..bde79e9 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -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 @@ -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 \ No newline at end of file +[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 \ No newline at end of file diff --git a/src/main/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializer.java b/src/main/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializer.java index fb71a35..945ecbb 100644 --- a/src/main/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializer.java +++ b/src/main/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializer.java @@ -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; @@ -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 */ @@ -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 @@ -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()) { diff --git a/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/JupyterHubUserInitializerConfig.java b/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/JupyterHubUserInitializerConfig.java index 14ae81c..b4f66a3 100644 --- a/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/JupyterHubUserInitializerConfig.java +++ b/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/JupyterHubUserInitializerConfig.java @@ -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; @@ -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 ); } diff --git a/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/MockConfig.java b/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/MockConfig.java index cb27187..012b5ce 100644 --- a/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/MockConfig.java +++ b/src/test/java/org/nrg/xnatx/plugins/jupyterhub/config/MockConfig.java @@ -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; @@ -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); diff --git a/src/test/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializerTest.java b/src/test/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializerTest.java index c049122..d8fee66 100644 --- a/src/test/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializerTest.java +++ b/src/test/java/org/nrg/xnatx/plugins/jupyterhub/initialize/JupyterHubUserInitializerTest.java @@ -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; @@ -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"; @@ -41,6 +43,7 @@ public void after() { Mockito.reset(mockRoleService); Mockito.reset(mockXnatAppInfo); Mockito.reset(mockSystemHelper); + Mockito.reset(mockDatabaseHelper); } @Test(expected = InitializingTaskException.class) @@ -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 @@ -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); @@ -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)); @@ -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)); @@ -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)); @@ -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); @@ -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);