Skip to content

Commit 0b5783d

Browse files
jreynard-codejreynard-code
committed
Make files parameter in dataset creation and update optional
- Updated `DatasetServiceImpl` to handle `files` parameter as optional in both dataset creation and update operations. - Added integration tests to verify behavior when `files` parameter is omitted. - Updated mustache template and OpenAPI generator configuration to reflect the optional nature of `files`.
1 parent 0862207 commit 0b5783d

File tree

4 files changed

+265
-6
lines changed

4 files changed

+265
-6
lines changed

‎api/src/integrationTest/kotlin/com/cosmotech/api/home/dataset/DatasetControllerTests.kt‎

Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ import com.cosmotech.api.rbac.ROLE_EDITOR
3333
import com.cosmotech.api.rbac.ROLE_NONE
3434
import com.cosmotech.api.rbac.ROLE_VIEWER
3535
import com.cosmotech.dataset.domain.DatasetAccessControl
36+
import com.cosmotech.dataset.domain.DatasetCreateRequest
37+
import com.cosmotech.dataset.domain.DatasetPart
3638
import com.cosmotech.dataset.domain.DatasetPartCreateRequest
3739
import com.cosmotech.dataset.domain.DatasetPartTypeEnum
3840
import com.cosmotech.dataset.domain.DatasetPartUpdateRequest
@@ -44,6 +46,7 @@ import com.cosmotech.solution.domain.RunTemplateResourceSizing
4446
import java.io.InputStream
4547
import kotlin.test.Ignore
4648
import org.apache.commons.io.IOUtils
49+
import org.hamcrest.Matchers.empty
4750
import org.hamcrest.Matchers.greaterThan
4851
import org.json.JSONArray
4952
import org.json.JSONObject
@@ -160,6 +163,91 @@ class DatasetControllerTests : ControllerTestBase() {
160163
.andDo(document("organizations/{organization_id}/workspaces/{workspace_id}/datasets/POST"))
161164
}
162165

166+
@Test
167+
@WithMockOauth2User
168+
fun create_dataset_with_no_files_parameter() {
169+
val datasetCreateRequest =
170+
MockMultipartFile(
171+
"datasetCreateRequest",
172+
null,
173+
MediaType.APPLICATION_JSON_VALUE,
174+
JSONObject(
175+
DatasetCreateRequest(
176+
name = DATASET_NAME,
177+
description = DATASET_DESCRIPTION,
178+
tags = mutableListOf("tag1", "tag2"),
179+
runnerId = "r-12345678910"))
180+
.toString()
181+
.byteInputStream())
182+
183+
mvc.perform(
184+
multipart("/organizations/$organizationId/workspaces/$workspaceId/datasets")
185+
.file(datasetCreateRequest)
186+
.accept(MediaType.APPLICATION_JSON)
187+
.with(csrf()))
188+
.andExpect(status().is2xxSuccessful)
189+
.andExpect(jsonPath("$.name").value(DATASET_NAME))
190+
.andExpect(jsonPath("$.description").value(DATASET_DESCRIPTION))
191+
.andExpect(jsonPath("$.organizationId").value(organizationId))
192+
.andExpect(jsonPath("$.workspaceId").value(workspaceId))
193+
.andExpect(jsonPath("$.parts", empty<DatasetPart>()))
194+
.andExpect(jsonPath("$.createInfo.userId").value(PLATFORM_ADMIN_EMAIL))
195+
.andExpect(jsonPath("$.createInfo.runnerId").value("r-12345678910"))
196+
.andExpect(jsonPath("$.createInfo.timestamp").isNumber)
197+
.andExpect(jsonPath("$.createInfo.timestamp").value(greaterThan(0.toLong())))
198+
.andExpect(jsonPath("$.updateInfo.userId").value(PLATFORM_ADMIN_EMAIL))
199+
.andExpect(jsonPath("$.updateInfo.timestamp").isNumber)
200+
.andExpect(jsonPath("$.updateInfo.timestamp").value(greaterThan(0.toLong())))
201+
.andExpect(jsonPath("$.tags").value(mutableListOf("tag1", "tag2")))
202+
.andExpect(jsonPath("$.security.default").value(ROLE_NONE))
203+
.andExpect(jsonPath("$.security.accessControlList[0].role").value(ROLE_ADMIN))
204+
.andExpect(jsonPath("$.security.accessControlList[0].id").value(PLATFORM_ADMIN_EMAIL))
205+
.andDo(MockMvcResultHandlers.print())
206+
}
207+
208+
@Test
209+
@WithMockOauth2User
210+
fun create_dataset_with_no_files_parameter_empty_parts() {
211+
val datasetCreateRequest =
212+
MockMultipartFile(
213+
"datasetCreateRequest",
214+
null,
215+
MediaType.APPLICATION_JSON_VALUE,
216+
JSONObject(
217+
DatasetCreateRequest(
218+
name = DATASET_NAME,
219+
description = DATASET_DESCRIPTION,
220+
tags = mutableListOf("tag1", "tag2"),
221+
parts = mutableListOf(),
222+
runnerId = "r-12345678910"))
223+
.toString()
224+
.byteInputStream())
225+
226+
mvc.perform(
227+
multipart("/organizations/$organizationId/workspaces/$workspaceId/datasets")
228+
.file(datasetCreateRequest)
229+
.accept(MediaType.APPLICATION_JSON)
230+
.with(csrf()))
231+
.andExpect(status().is2xxSuccessful)
232+
.andExpect(jsonPath("$.name").value(DATASET_NAME))
233+
.andExpect(jsonPath("$.description").value(DATASET_DESCRIPTION))
234+
.andExpect(jsonPath("$.organizationId").value(organizationId))
235+
.andExpect(jsonPath("$.workspaceId").value(workspaceId))
236+
.andExpect(jsonPath("$.parts", empty<DatasetPart>()))
237+
.andExpect(jsonPath("$.createInfo.userId").value(PLATFORM_ADMIN_EMAIL))
238+
.andExpect(jsonPath("$.createInfo.runnerId").value("r-12345678910"))
239+
.andExpect(jsonPath("$.createInfo.timestamp").isNumber)
240+
.andExpect(jsonPath("$.createInfo.timestamp").value(greaterThan(0.toLong())))
241+
.andExpect(jsonPath("$.updateInfo.userId").value(PLATFORM_ADMIN_EMAIL))
242+
.andExpect(jsonPath("$.updateInfo.timestamp").isNumber)
243+
.andExpect(jsonPath("$.updateInfo.timestamp").value(greaterThan(0.toLong())))
244+
.andExpect(jsonPath("$.tags").value(mutableListOf("tag1", "tag2")))
245+
.andExpect(jsonPath("$.security.default").value(ROLE_NONE))
246+
.andExpect(jsonPath("$.security.accessControlList[0].role").value(ROLE_ADMIN))
247+
.andExpect(jsonPath("$.security.accessControlList[0].id").value(PLATFORM_ADMIN_EMAIL))
248+
.andDo(MockMvcResultHandlers.print())
249+
}
250+
163251
@Test
164252
@WithMockOauth2User
165253
fun download_dataset_part() {
@@ -901,6 +989,169 @@ class DatasetControllerTests : ControllerTestBase() {
901989
"organizations/{organization_id}/workspaces/{workspace_id}/datasets/{dataset_id}/PATCH"))
902990
}
903991

992+
@Test
993+
@WithMockOauth2User
994+
fun update_dataset_with_no_files_parameter() {
995+
val datasetId =
996+
createDatasetAndReturnId(
997+
mvc,
998+
organizationId,
999+
workspaceId,
1000+
constructDatasetCreateRequest(
1001+
security =
1002+
DatasetSecurity(
1003+
default = ROLE_NONE,
1004+
accessControlList =
1005+
mutableListOf(
1006+
DatasetAccessControl(id = PLATFORM_ADMIN_EMAIL, role = ROLE_ADMIN),
1007+
DatasetAccessControl(
1008+
id = ORGANIZATION_USER_EMAIL, role = ROLE_EDITOR)))))
1009+
1010+
val newName = "this_a_new_name_for_dataset"
1011+
val newDescription = "this_a_new_description_for_dataset"
1012+
val newTags = mutableListOf("tag1_updated", "tag2_updated")
1013+
1014+
val datasetUpdateRequest =
1015+
DatasetUpdateRequest(
1016+
name = newName,
1017+
description = newDescription,
1018+
tags = newTags,
1019+
security =
1020+
DatasetSecurity(
1021+
default = ROLE_NONE,
1022+
accessControlList =
1023+
mutableListOf(
1024+
DatasetAccessControl(id = PLATFORM_ADMIN_EMAIL, role = ROLE_ADMIN),
1025+
DatasetAccessControl(
1026+
id = ORGANIZATION_USER_EMAIL, role = ROLE_VIEWER))))
1027+
1028+
val datasetUpdateRequestMultipartFile =
1029+
MockMultipartFile(
1030+
"datasetUpdateRequest",
1031+
null,
1032+
MediaType.APPLICATION_JSON_VALUE,
1033+
JSONObject(datasetUpdateRequest).toString().byteInputStream())
1034+
mvc.perform(
1035+
multipart("/organizations/$organizationId/workspaces/$workspaceId/datasets/$datasetId")
1036+
.file(datasetUpdateRequestMultipartFile)
1037+
.accept(MediaType.APPLICATION_JSON)
1038+
.with(csrf())
1039+
// By default, behind multipart, the HTTP verb used is POST
1040+
// We can override the HTTP verb as following
1041+
// https://stackoverflow.com/questions/38571716/how-to-put-multipart-form-data-using-spring-mockmvc
1042+
.with { request ->
1043+
request.method = "PATCH"
1044+
request
1045+
})
1046+
.andExpect(status().is2xxSuccessful)
1047+
.andExpect(jsonPath("$.name").value(newName))
1048+
.andExpect(jsonPath("$.description").value(newDescription))
1049+
.andExpect(jsonPath("$.organizationId").value(organizationId))
1050+
.andExpect(jsonPath("$.workspaceId").value(workspaceId))
1051+
.andExpect(jsonPath("$.createInfo.userId").value(PLATFORM_ADMIN_EMAIL))
1052+
.andExpect(jsonPath("$.createInfo.timestamp").isNumber)
1053+
.andExpect(jsonPath("$.createInfo.timestamp").value(greaterThan(0.toLong())))
1054+
.andExpect(jsonPath("$.updateInfo.userId").value(PLATFORM_ADMIN_EMAIL))
1055+
.andExpect(jsonPath("$.updateInfo.timestamp").isNumber)
1056+
.andExpect(jsonPath("$.updateInfo.timestamp").value(greaterThan(0.toLong())))
1057+
.andExpect(jsonPath("$.parts[0].name").value(DATASET_PART_NAME))
1058+
.andExpect(jsonPath("$.parts[0].description").value(DATASET_PART_DESCRIPTION))
1059+
.andExpect(jsonPath("$.parts[0].organizationId").value(organizationId))
1060+
.andExpect(jsonPath("$.parts[0].workspaceId").value(workspaceId))
1061+
.andExpect(jsonPath("$.parts[0].sourceName").value(TEST_FILE_NAME))
1062+
.andExpect(jsonPath("$.parts[0].tags").value(mutableListOf("tag_part1", "tag_part2")))
1063+
.andExpect(jsonPath("$.parts[0].type").value(DatasetPartTypeEnum.File.value))
1064+
.andExpect(jsonPath("$.parts[0].createInfo.userId").value(PLATFORM_ADMIN_EMAIL))
1065+
.andExpect(jsonPath("$.parts[0].createInfo.timestamp").isNumber)
1066+
.andExpect(jsonPath("$.parts[0].createInfo.timestamp").value(greaterThan(0.toLong())))
1067+
.andExpect(jsonPath("$.parts[0].updateInfo.userId").value(PLATFORM_ADMIN_EMAIL))
1068+
.andExpect(jsonPath("$.parts[0].updateInfo.timestamp").isNumber)
1069+
.andExpect(jsonPath("$.parts[0].updateInfo.timestamp").value(greaterThan(0.toLong())))
1070+
.andExpect(jsonPath("$.tags").value(newTags))
1071+
.andExpect(jsonPath("$.security.default").value(ROLE_NONE))
1072+
.andExpect(jsonPath("$.security.accessControlList[0].role").value(ROLE_ADMIN))
1073+
.andExpect(jsonPath("$.security.accessControlList[0].id").value(PLATFORM_ADMIN_EMAIL))
1074+
.andExpect(jsonPath("$.security.accessControlList[1].role").value(ROLE_VIEWER))
1075+
.andExpect(jsonPath("$.security.accessControlList[1].id").value(ORGANIZATION_USER_EMAIL))
1076+
.andDo(MockMvcResultHandlers.print())
1077+
}
1078+
1079+
@Test
1080+
@WithMockOauth2User
1081+
fun update_dataset_with_no_files_parameter_empty_parts() {
1082+
val datasetId =
1083+
createDatasetAndReturnId(
1084+
mvc,
1085+
organizationId,
1086+
workspaceId,
1087+
constructDatasetCreateRequest(
1088+
security =
1089+
DatasetSecurity(
1090+
default = ROLE_NONE,
1091+
accessControlList =
1092+
mutableListOf(
1093+
DatasetAccessControl(id = PLATFORM_ADMIN_EMAIL, role = ROLE_ADMIN),
1094+
DatasetAccessControl(
1095+
id = ORGANIZATION_USER_EMAIL, role = ROLE_EDITOR)))))
1096+
1097+
val newName = "this_a_new_name_for_dataset"
1098+
val newDescription = "this_a_new_description_for_dataset"
1099+
val newTags = mutableListOf("tag1_updated", "tag2_updated")
1100+
1101+
val datasetUpdateRequest =
1102+
DatasetUpdateRequest(
1103+
name = newName,
1104+
description = newDescription,
1105+
tags = newTags,
1106+
parts = mutableListOf(),
1107+
security =
1108+
DatasetSecurity(
1109+
default = ROLE_NONE,
1110+
accessControlList =
1111+
mutableListOf(
1112+
DatasetAccessControl(id = PLATFORM_ADMIN_EMAIL, role = ROLE_ADMIN),
1113+
DatasetAccessControl(
1114+
id = ORGANIZATION_USER_EMAIL, role = ROLE_VIEWER))))
1115+
1116+
val datasetUpdateRequestMultipartFile =
1117+
MockMultipartFile(
1118+
"datasetUpdateRequest",
1119+
null,
1120+
MediaType.APPLICATION_JSON_VALUE,
1121+
JSONObject(datasetUpdateRequest).toString().byteInputStream())
1122+
mvc.perform(
1123+
multipart("/organizations/$organizationId/workspaces/$workspaceId/datasets/$datasetId")
1124+
.file(datasetUpdateRequestMultipartFile)
1125+
.accept(MediaType.APPLICATION_JSON)
1126+
.with(csrf())
1127+
// By default, behind multipart, the HTTP verb used is POST
1128+
// We can override the HTTP verb as following
1129+
// https://stackoverflow.com/questions/38571716/how-to-put-multipart-form-data-using-spring-mockmvc
1130+
.with { request ->
1131+
request.method = "PATCH"
1132+
request
1133+
})
1134+
.andExpect(status().is2xxSuccessful)
1135+
.andExpect(jsonPath("$.name").value(newName))
1136+
.andExpect(jsonPath("$.description").value(newDescription))
1137+
.andExpect(jsonPath("$.organizationId").value(organizationId))
1138+
.andExpect(jsonPath("$.workspaceId").value(workspaceId))
1139+
.andExpect(jsonPath("$.parts", empty<DatasetPart>()))
1140+
.andExpect(jsonPath("$.createInfo.userId").value(PLATFORM_ADMIN_EMAIL))
1141+
.andExpect(jsonPath("$.createInfo.timestamp").isNumber)
1142+
.andExpect(jsonPath("$.createInfo.timestamp").value(greaterThan(0.toLong())))
1143+
.andExpect(jsonPath("$.updateInfo.userId").value(PLATFORM_ADMIN_EMAIL))
1144+
.andExpect(jsonPath("$.updateInfo.timestamp").isNumber)
1145+
.andExpect(jsonPath("$.updateInfo.timestamp").value(greaterThan(0.toLong())))
1146+
.andExpect(jsonPath("$.tags").value(newTags))
1147+
.andExpect(jsonPath("$.security.default").value(ROLE_NONE))
1148+
.andExpect(jsonPath("$.security.accessControlList[0].role").value(ROLE_ADMIN))
1149+
.andExpect(jsonPath("$.security.accessControlList[0].id").value(PLATFORM_ADMIN_EMAIL))
1150+
.andExpect(jsonPath("$.security.accessControlList[1].role").value(ROLE_VIEWER))
1151+
.andExpect(jsonPath("$.security.accessControlList[1].id").value(ORGANIZATION_USER_EMAIL))
1152+
.andDo(MockMvcResultHandlers.print())
1153+
}
1154+
9041155
@Test
9051156
@WithMockOauth2User
9061157
fun update_dataset_access_control() {

‎build.gradle.kts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,9 @@ subprojects {
444444
tasks.withType<GenerateTask> {
445445
inputSpec.set("${projectDir}/src/main/openapi/${projectDirName}.yaml")
446446
outputDir.set(openApiServerSourcesGenerationDir)
447+
// Templates here were enabled due to an open PR in OpenAPITools/openapi-generator
448+
// https://github.com/OpenAPITools/openapi-generator/pull/21994
449+
templateDir.set("${rootDir}/openapi/templates")
447450
generatorName.set("kotlin-spring")
448451
apiPackage.set("com.cosmotech.${projectDirName}.api")
449452
modelPackage.set("com.cosmotech.${projectDirName}.domain")

‎dataset/src/main/kotlin/com/cosmotech/dataset/service/DatasetServiceImpl.kt‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,12 @@ class DatasetServiceImpl(
121121
organizationId: String,
122122
workspaceId: String,
123123
datasetCreateRequest: DatasetCreateRequest,
124-
files: Array<MultipartFile>
124+
files: Array<MultipartFile>?
125125
): Dataset {
126+
val filesUploaded = files ?: emptyArray()
126127
workspaceService.getVerifiedWorkspace(organizationId, workspaceId, PERMISSION_CREATE_CHILDREN)
127128
logger.debug("Registering Dataset: {}", datasetCreateRequest)
128-
validDatasetCreateRequest(datasetCreateRequest, files)
129+
validDatasetCreateRequest(datasetCreateRequest, filesUploaded)
129130

130131
val datasetId = idGenerator.generate("dataset")
131132
val now = Instant.now().toEpochMilli()
@@ -144,7 +145,8 @@ class DatasetServiceImpl(
144145
val constructDatasetPart =
145146
constructDatasetPart(organizationId, workspaceId, datasetId, part)
146147
datasetPartManagementFactory.storeData(
147-
constructDatasetPart, files.first { it.originalFilename == part.sourceName })
148+
constructDatasetPart,
149+
filesUploaded.first { it.originalFilename == part.sourceName })
148150
constructDatasetPart
149151
}
150152
?.toMutableList()
@@ -290,20 +292,22 @@ class DatasetServiceImpl(
290292
workspaceId: String,
291293
datasetId: String,
292294
datasetUpdateRequest: DatasetUpdateRequest,
293-
files: Array<MultipartFile>
295+
files: Array<MultipartFile>?
294296
): Dataset {
295297
logger.debug("Updating Dataset: {}", datasetUpdateRequest)
298+
val filesUploaded = files ?: emptyArray()
296299
val previousDataset =
297300
getVerifiedDataset(organizationId, workspaceId, datasetId, PERMISSION_WRITE)
298-
validDatasetUpdateRequest(datasetUpdateRequest, files)
301+
validDatasetUpdateRequest(datasetUpdateRequest, filesUploaded)
299302

300303
val newDatasetParts =
301304
datasetUpdateRequest.parts
302305
?.map { part ->
303306
val constructDatasetPart =
304307
constructDatasetPart(organizationId, workspaceId, datasetId, part)
305308
datasetPartManagementFactory.storeData(
306-
constructDatasetPart, files.first { it.originalFilename == part.sourceName })
309+
constructDatasetPart,
310+
filesUploaded.first { it.originalFilename == part.sourceName })
307311
constructDatasetPart
308312
}
309313
?.toMutableList()

‎openapi/templates/optionalDataType.mustache‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{{^isFile}}{{{dataType}}}{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{/isFile}}{{#isFile}}{{#isArray}}Array<{{/isArray}}org.springframework.web.multipart.MultipartFile{{#isArray}}>{{/isArray}}{{^required}}?{{/required}}{{/isFile}}

0 commit comments

Comments
 (0)