Skip to content

Commit c687d13

Browse files
GekkioGekkio
committed
Redesign S3 document interface
- add DocumentKey and DocumentLocation - don't pass BucketEnv to all use sites
1 parent 9656611 commit c687d13

22 files changed

+330
-302
lines changed

‎service/src/integrationTest/kotlin/fi/espoo/evaka/attachments/AttachmentServiceTest.kt

+17-25
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44

55
package fi.espoo.evaka.attachments
66

7-
import fi.espoo.evaka.BucketEnv
87
import fi.espoo.evaka.PureJdbiTest
98
import fi.espoo.evaka.attachment.AttachmentParent
109
import fi.espoo.evaka.attachment.AttachmentService
1110
import fi.espoo.evaka.attachment.getAttachment
1211
import fi.espoo.evaka.attachment.insertAttachment
1312
import fi.espoo.evaka.attachment.userAttachmentCount
1413
import fi.espoo.evaka.s3.Document
14+
import fi.espoo.evaka.s3.DocumentKey
1515
import fi.espoo.evaka.s3.DocumentLocation
1616
import fi.espoo.evaka.s3.DocumentService
1717
import fi.espoo.evaka.shared.async.AsyncJob
@@ -24,7 +24,7 @@ import fi.espoo.evaka.shared.dev.DevPersonType
2424
import fi.espoo.evaka.shared.dev.insert
2525
import fi.espoo.evaka.shared.domain.EvakaClock
2626
import fi.espoo.evaka.shared.domain.MockEvakaClock
27-
import java.net.URI
27+
import java.io.InputStream
2828
import java.time.Duration
2929
import kotlin.test.assertEquals
3030
import kotlin.test.assertNotNull
@@ -39,38 +39,30 @@ import org.springframework.http.ResponseEntity
3939
class AttachmentServiceTest : PureJdbiTest(resetDbBeforeEach = true) {
4040
private lateinit var asyncJobRunner: AsyncJobRunner<AsyncJob>
4141

42-
class MockDocumentService(
43-
private val uploadFn: () -> DocumentLocation,
44-
private val deleteFn: () -> Unit,
45-
) : DocumentService {
46-
override fun get(bucketName: String, key: String): Document = error("Not implemented")
42+
class MockDocumentService(private val uploadFn: () -> Unit, private val deleteFn: () -> Unit) :
43+
DocumentService {
44+
override fun locate(key: DocumentKey): DocumentLocation =
45+
DocumentLocation(bucket = "bucket", key = key.value)
46+
47+
override fun get(location: DocumentLocation): Document = error("Not implemented")
4748

4849
override fun response(
49-
bucketName: String,
50-
key: String,
50+
location: DocumentLocation,
5151
contentDisposition: ContentDisposition,
5252
): ResponseEntity<Any> = error("Not implemented")
5353

54-
override fun upload(bucketName: String, document: Document): DocumentLocation = uploadFn()
54+
override fun upload(
55+
location: DocumentLocation,
56+
inputStream: InputStream,
57+
size: Long,
58+
contentType: String,
59+
) = uploadFn()
5560

56-
override fun delete(bucketName: String, key: String) = deleteFn()
61+
override fun delete(location: DocumentLocation) = deleteFn()
5762
}
5863

5964
private fun createAttachmentService(documentService: DocumentService) =
60-
AttachmentService(
61-
documentService,
62-
asyncJobRunner,
63-
BucketEnv(
64-
// none of these values matter, because FailingDocumentService doesn't use them
65-
s3MockUrl = URI(""),
66-
proxyThroughNginx = false,
67-
data = "",
68-
attachments = "",
69-
decisions = "",
70-
feeDecisions = "",
71-
voucherValueDecisions = "",
72-
),
73-
)
65+
AttachmentService(documentService, asyncJobRunner)
7466

7567
@BeforeEach
7668
fun beforeEach() {

‎service/src/integrationTest/kotlin/fi/espoo/evaka/pis/service/MergeServiceIntegrationTest.kt

+1-4
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
package fi.espoo.evaka.pis.service
66

7-
import fi.espoo.evaka.BucketEnv
87
import fi.espoo.evaka.FullApplicationTest
98
import fi.espoo.evaka.childimages.replaceImage
109
import fi.espoo.evaka.messaging.MessageService
@@ -57,7 +56,6 @@ class MergeServiceIntegrationTest : FullApplicationTest(resetDbBeforeEach = true
5756
@Autowired private lateinit var asyncJobRunner: AsyncJobRunner<AsyncJob>
5857
@Autowired private lateinit var messageService: MessageService
5958
@Autowired private lateinit var documentClient: DocumentService
60-
@Autowired private lateinit var bucketEnv: BucketEnv
6159

6260
private lateinit var mergeService: MergeService
6361
private lateinit var mergeServiceAsyncJobRunnerMock: AsyncJobRunner<AsyncJob>
@@ -71,7 +69,7 @@ class MergeServiceIntegrationTest : FullApplicationTest(resetDbBeforeEach = true
7169
@BeforeEach
7270
fun setUp() {
7371
mergeServiceAsyncJobRunnerMock = mock {}
74-
mergeService = MergeService(mergeServiceAsyncJobRunnerMock, documentClient, bucketEnv)
72+
mergeService = MergeService(mergeServiceAsyncJobRunnerMock, documentClient)
7573
db.transaction { tx ->
7674
tx.insert(area)
7775
tx.insert(unit)
@@ -540,7 +538,6 @@ class MergeServiceIntegrationTest : FullApplicationTest(resetDbBeforeEach = true
540538
replaceImage(
541539
db,
542540
documentClient,
543-
bucketEnv.data,
544541
childId,
545542
MockMultipartFile("file", imageName, "image/jpeg", imageData),
546543
"image/jpeg",

‎service/src/integrationTest/kotlin/fi/espoo/evaka/s3/S3DocumentServiceIntegrationTest.kt

+21-32
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ package fi.espoo.evaka.s3
66

77
import fi.espoo.evaka.BucketEnv
88
import fi.espoo.evaka.FullApplicationTest
9+
import fi.espoo.evaka.shared.AttachmentId
10+
import java.util.UUID
911
import kotlin.test.assertContentEquals
1012
import kotlin.test.assertEquals
1113
import kotlin.test.assertNotNull
@@ -26,6 +28,8 @@ class S3DocumentServiceIntegrationTest : FullApplicationTest(resetDbBeforeEach =
2628

2729
private lateinit var documentClient: DocumentService
2830

31+
val documentRef = DocumentKey.Attachment(AttachmentId(UUID.randomUUID()))
32+
2933
@BeforeEach
3034
fun beforeEach() {
3135
documentClient =
@@ -36,50 +40,41 @@ class S3DocumentServiceIntegrationTest : FullApplicationTest(resetDbBeforeEach =
3640
fun `redirects when not proxying through nginx`() {
3741
val documentClientNoProxy =
3842
S3DocumentService(s3Client, s3Presigner, bucketEnv.copy(proxyThroughNginx = false))
39-
documentClientNoProxy.upload(
40-
bucketEnv.data,
41-
Document("test", byteArrayOf(0x11, 0x22, 0x33), "text/plain"),
42-
)
43+
val location =
44+
documentClientNoProxy.upload(documentRef, byteArrayOf(0x11, 0x22, 0x33), "text/plain")
4345

44-
val response = documentClientNoProxy.responseAttachment(bucketEnv.data, "test", null)
46+
val response = documentClientNoProxy.responseAttachment(location, null)
4547
assertEquals(HttpStatus.FOUND, response.statusCode)
4648
assertNotNull(response.headers["Location"])
4749
assertNull(response.headers["X-Accel-Redirect"])
4850
}
4951

5052
@Test
5153
fun `uses X-Accel-Redirect when proxying through nginx`() {
52-
documentClient.upload(
53-
bucketEnv.data,
54-
Document("test", byteArrayOf(0x33, 0x22, 0x11), "text/plain"),
55-
)
54+
val location =
55+
documentClient.upload(documentRef, byteArrayOf(0x33, 0x22, 0x11), "text/plain")
5656

57-
val response = documentClient.responseAttachment(bucketEnv.data, "test", null)
57+
val response = documentClient.responseAttachment(location, null)
5858
assertEquals(HttpStatus.OK, response.statusCode)
5959
assertNull(response.headers["Location"])
6060
assertNotNull(response.headers["X-Accel-Redirect"])
6161
}
6262

6363
@Test
6464
fun `upload-download round trip with get`() {
65-
documentClient.upload(
66-
bucketEnv.data,
67-
Document("test", byteArrayOf(0x11, 0x33, 0x22), "text/plain"),
68-
)
65+
val location =
66+
documentClient.upload(documentRef, byteArrayOf(0x11, 0x33, 0x22), "text/plain")
6967

70-
val document = documentClient.get(bucketEnv.data, "test")
68+
val document = documentClient.get(location)
7169

7270
assertContentEquals(byteArrayOf(0x11, 0x33, 0x22), document.bytes)
7371
}
7472

7573
@Test
7674
fun `responseAttachment works without filename`() {
77-
documentClient.upload(
78-
bucketEnv.data,
79-
Document("test", byteArrayOf(0x22, 0x11, 0x33), "text/csv"),
80-
)
75+
val location = documentClient.upload(documentRef, byteArrayOf(0x22, 0x11, 0x33), "text/csv")
8176

82-
val response = documentClient.responseAttachment(bucketEnv.data, "test", null)
77+
val response = documentClient.responseAttachment(location, null)
8378
val s3Url = responseEntityToS3URL(response)
8479
val (_, s3response, s3data) = http.get(s3Url).response()
8580

@@ -90,13 +85,10 @@ class S3DocumentServiceIntegrationTest : FullApplicationTest(resetDbBeforeEach =
9085

9186
@Test
9287
fun `responseAttachment works with filename`() {
93-
documentClient.upload(
94-
bucketEnv.data,
95-
Document("test", byteArrayOf(0x33, 0x11, 0x22), "application/pdf"),
96-
)
88+
val location =
89+
documentClient.upload(documentRef, byteArrayOf(0x33, 0x11, 0x22), "application/pdf")
9790

98-
val response =
99-
documentClient.responseAttachment(bucketEnv.data, "test", "overridden-filename.pdf")
91+
val response = documentClient.responseAttachment(location, "overridden-filename.pdf")
10092
val s3Url = responseEntityToS3URL(response)
10193
val (_, s3response, s3data) = http.get(s3Url).response()
10294

@@ -112,13 +104,10 @@ class S3DocumentServiceIntegrationTest : FullApplicationTest(resetDbBeforeEach =
112104

113105
@Test
114106
fun `responseInline works`() {
115-
documentClient.upload(
116-
bucketEnv.data,
117-
Document("test", byteArrayOf(0x12, 0x34, 0x56), "text/plain"),
118-
)
107+
val location =
108+
documentClient.upload(documentRef, byteArrayOf(0x12, 0x34, 0x56), "text/plain")
119109

120-
val response =
121-
documentClient.responseInline(bucketEnv.data, "test", "overridden-filename.txt")
110+
val response = documentClient.responseInline(location, "overridden-filename.txt")
122111
val s3Url = responseEntityToS3URL(response)
123112
val (_, s3response, s3data) = http.get(s3Url).response()
124113

‎service/src/integrationTest/kotlin/fi/espoo/evaka/sficlient/SoapStackIntegrationTest.kt

+5-4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import fi.espoo.evaka.SfiContactPersonEnv
1313
import fi.espoo.evaka.SfiEnv
1414
import fi.espoo.evaka.SfiPrintingEnv
1515
import fi.espoo.evaka.s3.Document
16+
import fi.espoo.evaka.s3.DocumentLocation
1617
import fi.espoo.evaka.sficlient.soap.KyselyWS1
1718
import fi.espoo.evaka.sficlient.soap.KyselyWS10
1819
import fi.espoo.evaka.sficlient.soap.KyselyWS2
@@ -95,10 +96,10 @@ class SoapStackIntegrationTest {
9596
)
9697
private val dummyContent = byteArrayOf(0x11, 0x22, 0x33, 0x44)
9798

98-
private fun dummyGetDocument(bucketName: String, key: String): Document {
99-
assertEquals(message.documentBucket, bucketName)
100-
assertEquals(message.documentKey, key)
101-
return Document("name", dummyContent, "text/plain")
99+
private fun dummyGetDocument(location: DocumentLocation): Document {
100+
assertEquals(message.documentBucket, location.bucket)
101+
assertEquals(message.documentKey, location.key)
102+
return Document(location.key, dummyContent, "text/plain")
102103
}
103104

104105
private fun defaultEnv() =

‎service/src/integrationTest/kotlin/fi/espoo/evaka/sficlient/rest/SfiMessagesRestClientIntegrationTest.kt

+4-4
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ class SfiMessagesRestClientIntegrationTest : FullApplicationTest(resetDbBeforeEa
9191
client =
9292
SfiMessagesRestClient(
9393
sfiEnv,
94-
getDocument = { bucketName, key ->
95-
assertEquals(message.documentBucket, bucketName)
96-
assertEquals(message.documentKey, key)
97-
Document(key, fileContent, contentType = "content-type")
94+
getDocument = { location ->
95+
assertEquals(message.documentBucket, location.bucket)
96+
assertEquals(message.documentKey, location.key)
97+
Document(location.key, fileContent, contentType = "content-type")
9898
},
9999
passwordStore =
100100
MockPasswordStore(

‎service/src/main/kotlin/fi/espoo/evaka/assistanceneed/decision/AssistanceNeedDecisionService.kt

+20-25
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
package fi.espoo.evaka.assistanceneed.decision
66

7-
import fi.espoo.evaka.BucketEnv
87
import fi.espoo.evaka.EmailEnv
98
import fi.espoo.evaka.daycare.domain.Language
109
import fi.espoo.evaka.decision.getSendAddress
@@ -24,7 +23,7 @@ import fi.espoo.evaka.pis.service.getChildGuardiansAndFosterParents
2423
import fi.espoo.evaka.process.ArchivedProcessState
2524
import fi.espoo.evaka.process.getArchiveProcessByAssistanceNeedDecisionId
2625
import fi.espoo.evaka.process.insertProcessHistoryRow
27-
import fi.espoo.evaka.s3.Document
26+
import fi.espoo.evaka.s3.DocumentKey
2827
import fi.espoo.evaka.s3.DocumentService
2928
import fi.espoo.evaka.sficlient.SfiMessage
3029
import fi.espoo.evaka.shared.AssistanceNeedDecisionId
@@ -46,8 +45,6 @@ import org.thymeleaf.context.Context
4645

4746
private val logger = KotlinLogging.logger {}
4847

49-
const val assistanceNeedDecisionsBucketPrefix = "assistance-need-decisions/"
50-
5148
@Component
5249
class AssistanceNeedDecisionService(
5350
private val emailClient: EmailClient,
@@ -56,12 +53,9 @@ class AssistanceNeedDecisionService(
5653
private val documentClient: DocumentService,
5754
private val templateProvider: ITemplateProvider,
5855
private val messageProvider: IMessageProvider,
59-
bucketEnv: BucketEnv,
6056
private val emailEnv: EmailEnv,
6157
private val asyncJobRunner: AsyncJobRunner<AsyncJob>,
6258
) {
63-
private val bucket = bucketEnv.data
64-
6559
init {
6660
asyncJobRunner.registerHandler(::runCreateAssistanceNeedDecisionPdf)
6761
asyncJobRunner.registerHandler(::runSendAssistanceNeedDecisionEmail)
@@ -84,14 +78,7 @@ class AssistanceNeedDecisionService(
8478
val pdf = generatePdf(clock.today(), decision)
8579
val key =
8680
documentClient
87-
.upload(
88-
bucket,
89-
Document(
90-
"${assistanceNeedDecisionsBucketPrefix}assistance_need_decision_$decisionId.pdf",
91-
pdf,
92-
"application/pdf",
93-
),
94-
)
81+
.upload(DocumentKey.AssistanceNeedDecision(decisionId), pdf, "application/pdf")
9582
.key
9683
tx.updateAssistanceNeedDocumentKey(decision.id, key)
9784

@@ -171,11 +158,15 @@ class AssistanceNeedDecisionService(
171158
)
172159
}
173160

174-
val documentKey =
175-
tx.getAssistanceNeedDecisionDocumentKey(decisionId)
176-
?: throw IllegalStateException(
177-
"Assistance need decision pdf has not yet been generated"
161+
val documentLocation =
162+
documentClient.locate(
163+
DocumentKey.AssistanceNeedDecision(
164+
tx.getAssistanceNeedDecisionDocumentKey(decisionId)
165+
?: throw IllegalStateException(
166+
"Assistance need decision pdf has not yet been generated"
167+
)
178168
)
169+
)
179170

180171
val lang =
181172
if (decision.language == OfficialLanguage.SV) OfficialLanguage.SV
@@ -206,8 +197,8 @@ class AssistanceNeedDecisionService(
206197
documentId = messageId,
207198
documentDisplayName =
208199
suomiFiDocumentFileName(decision.language),
209-
documentKey = documentKey,
210-
documentBucket = bucket,
200+
documentKey = documentLocation.key,
201+
documentBucket = documentLocation.bucket,
211202
firstName = guardian.firstName,
212203
lastName = guardian.lastName,
213204
streetAddress = sendAddress.street,
@@ -233,10 +224,14 @@ class AssistanceNeedDecisionService(
233224
dbc: Database.Connection,
234225
decisionId: AssistanceNeedDecisionId,
235226
): ResponseEntity<Any> {
236-
val documentKey =
237-
dbc.read { it.getAssistanceNeedDecisionDocumentKey(decisionId) }
238-
?: throw NotFound("No assistance need decision found with ID ($decisionId)")
239-
return documentClient.responseAttachment(bucket, documentKey, null)
227+
val documentLocation =
228+
documentClient.locate(
229+
DocumentKey.AssistanceNeedDecision(
230+
dbc.read { it.getAssistanceNeedDecisionDocumentKey(decisionId) }
231+
?: throw NotFound("No assistance need decision found with ID ($decisionId)")
232+
)
233+
)
234+
return documentClient.responseAttachment(documentLocation, null)
240235
}
241236

242237
fun generatePdf(sentDate: LocalDate, decision: AssistanceNeedDecision): ByteArray {

0 commit comments

Comments
 (0)