Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check submission table when computing submission history #16200

Merged
merged 3 commits into from
Oct 17, 2024
Merged

Check submission table when computing submission history #16200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d1f6f5c
Check submission table when computing submission history
thetaurean Oct 11, 2024
5a3038a
Added mocks to fix 2 test failures
thetaurean Oct 11, 2024
80a4e91
Merge branch 'master' into platform/thetaurean/15822-submission-histo…
arnejduranovic Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions prime-router/src/main/kotlin/history/azure/ReportFileFunction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import gov.cdc.prime.router.CustomerStatus
import gov.cdc.prime.router.RESTTransportType
import gov.cdc.prime.router.azure.DataAccessTransaction
import gov.cdc.prime.router.azure.HttpUtilities
import gov.cdc.prime.router.azure.SubmissionTableService
import gov.cdc.prime.router.azure.WorkflowEngine
import gov.cdc.prime.router.azure.db.tables.pojos.Action
import gov.cdc.prime.router.history.ReportHistory
Expand Down Expand Up @@ -161,10 +162,15 @@ abstract class ReportFileFunction(
} catch (e: DataAccessException) {
logger.error("Unable to fetch history for ID $id", e)
return HttpUtilities.internalErrorResponse(request)
} catch (ex: IllegalStateException) {
logger.error(ex)
// Errors above are actionId or UUID not found errors.
return HttpUtilities.notFoundResponse(request, ex.message)
} catch (ex: IllegalStateException) { // actionId or UUID not found
val submission = SubmissionTableService.getInstance().getSubmission(id, "Received")

return if (submission == null) {
logger.error(ex)
HttpUtilities.notFoundResponse(request, ex.message)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why return the exception message to the caller?

The project is open source so it's not really an OWASP issue in the way it would be were our project proprietary. As such feel free to leave this alone if you feel strongly.

my view is - in general - to not return anything to the caller directly tied to a facet of system state and/or implementation detail unless the caller needs to have that information for some specific reason.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you however this was pre-existing code so I didn't feel comfortable changing it.

For the most part this is catching exceptions that we throw, so the error messages are things we set. Though IllegalStateException is pretty broad and could probably occur unexpectedly.

} else {
HttpUtilities.okJSONResponse(request, submission)
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions prime-router/src/test/kotlin/history/azure/DeliveryFunctionTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import gov.cdc.prime.router.azure.ApiSearchResult
import gov.cdc.prime.router.azure.DatabaseAccess
import gov.cdc.prime.router.azure.MockHttpRequestMessage
import gov.cdc.prime.router.azure.MockSettings
import gov.cdc.prime.router.azure.SubmissionTableService
import gov.cdc.prime.router.azure.WorkflowEngine
import gov.cdc.prime.router.azure.db.enums.TaskAction
import gov.cdc.prime.router.azure.db.tables.pojos.Action
Expand Down Expand Up @@ -461,6 +462,12 @@ class DeliveryFunctionTests : Logging {
every { AuthenticatedClaims.authenticate(any()) } returns
AuthenticatedClaims.generateTestClaims()

val submissionTableService = mockk<SubmissionTableService>()
every { submissionTableService.getSubmission(any(), any()) } returns null

mockkObject(SubmissionTableService.Companion)
every { SubmissionTableService.getInstance() } returns submissionTableService

// Invalid id: not a UUID nor a Long
var response = function.getDeliveryDetails(mockRequest, "bad")
assertThat(response.status).isEqualTo(HttpStatus.NOT_FOUND)
Expand Down
39 changes: 39 additions & 0 deletions prime-router/src/test/kotlin/history/azure/SubmissionFunctionIntegrationTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package gov.cdc.prime.router.history.azure
import assertk.assertThat
import assertk.assertions.isEqualTo
import assertk.assertions.isNotNull
import gov.cdc.prime.reportstream.shared.Submission
import gov.cdc.prime.router.ActionLog
import gov.cdc.prime.router.ActionLogLevel
import gov.cdc.prime.router.FileSettings
Expand All @@ -11,6 +12,7 @@ import gov.cdc.prime.router.Metadata
import gov.cdc.prime.router.MimeFormat
import gov.cdc.prime.router.Topic
import gov.cdc.prime.router.azure.MockHttpRequestMessage
import gov.cdc.prime.router.azure.SubmissionTableService
import gov.cdc.prime.router.azure.WorkflowEngine
import gov.cdc.prime.router.azure.db.enums.TaskAction
import gov.cdc.prime.router.common.BaseEngine
Expand All @@ -26,13 +28,16 @@ import gov.cdc.prime.router.tokens.oktaSystemAdminGroup
import gov.cdc.prime.router.unittest.UnitTestUtils
import io.mockk.clearAllMocks
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkObject
import io.mockk.unmockkAll
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.testcontainers.junit.jupiter.Testcontainers
import java.time.OffsetDateTime
import java.time.ZoneOffset

@Testcontainers
@ExtendWith(ReportStreamTestDatabaseSetupExtension::class)
Expand Down Expand Up @@ -69,6 +74,40 @@ class SubmissionFunctionIntegrationTests {
assertThat(historyNode.get("warnings").size()).isEqualTo(0)
}

@Test
fun `it should return a history for a submission received by the submissions microservice`() {
val timestamp = OffsetDateTime.now(ZoneOffset.UTC)
val submissionId = "some-submission-id"
val submissionTableService = mockk<SubmissionTableService>()
every { submissionTableService.getSubmission(any(), any()) } returns Submission(
submissionId,
"Received",
"some-url",
"some-detail",
timestamp
)

mockkObject(SubmissionTableService.Companion)
every { SubmissionTableService.getInstance() } returns submissionTableService

val httpRequestMessage = MockHttpRequestMessage()

val func = setupSubmissionFunction()

val history = func.getReportDetailedHistory(httpRequestMessage, submissionId)
assertThat(history).isNotNull()

val historyNode = JacksonMapperUtilities.defaultMapper.readTree(history.body.toString())
assertThat(historyNode.fieldNames().asSequence().toList()).isEqualTo(
listOf("submissionId", "overallStatus", "timestamp")
)
assertThat(historyNode.get("submissionId").asText()).isEqualTo(submissionId)
assertThat(historyNode.get("overallStatus").asText()).isEqualTo("Received")
assertThat(historyNode.get("timestamp").asText()).isEqualTo(
timestamp.format(JacksonMapperUtilities.timestampFormatter)
)
}

@Test
fun `it should return a history for partially delivered submission`() {
val submittedReport = reportGraph {
Expand Down
7 changes: 7 additions & 0 deletions prime-router/src/test/kotlin/history/azure/SubmissionFunctionTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import gov.cdc.prime.router.TranslatorConfiguration
import gov.cdc.prime.router.azure.DatabaseAccess
import gov.cdc.prime.router.azure.MockHttpRequestMessage
import gov.cdc.prime.router.azure.MockSettings
import gov.cdc.prime.router.azure.SubmissionTableService
import gov.cdc.prime.router.azure.WorkflowEngine
import gov.cdc.prime.router.azure.db.enums.TaskAction
import gov.cdc.prime.router.azure.db.tables.pojos.Action
Expand Down Expand Up @@ -440,6 +441,12 @@ class SubmissionFunctionTests : Logging {
every { AuthenticatedClaims.authenticate(any()) } returns
AuthenticatedClaims.generateTestClaims()

val submissionTableService = mockk<SubmissionTableService>()
every { submissionTableService.getSubmission(any(), any()) } returns null

mockkObject(SubmissionTableService.Companion)
every { SubmissionTableService.getInstance() } returns submissionTableService

// Invalid id: not a UUID nor a Long
var response = function.getReportDetailedHistory(mockRequest, "bad")
assertThat(response.status).isEqualTo(HttpStatus.NOT_FOUND)
Expand Down
12 changes: 11 additions & 1 deletion shared/src/main/kotlin/gov/cdc/prime/reportstream/shared/Submission.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package gov.cdc.prime.reportstream.shared

import com.azure.data.tables.models.TableEntity
import com.fasterxml.jackson.annotation.JsonIgnore
import com.fasterxml.jackson.annotation.JsonProperty
import com.fasterxml.jackson.annotation.JsonPropertyOrder
import java.time.OffsetDateTime

/**
* Represents a submission entity to be stored in Azure Table Storage.
Expand All @@ -10,11 +14,16 @@ import com.azure.data.tables.models.TableEntity
* @property bodyURL The URL pointing to the body of the submission.
* @property detail Optional additional details about the submission.
*/
@JsonPropertyOrder(value = ["submissionId", "overallStatus", "timestamp"])
data class Submission(
val submissionId: String,
@JsonProperty("overallStatus")
val status: String,
@JsonIgnore
val bodyURL: String,
@JsonIgnore
val detail: String? = null,
val timestamp: OffsetDateTime? = null
) {
companion object {
/**
Expand All @@ -28,7 +37,8 @@ data class Submission(
submissionId = tableEntity.partitionKey,
status = tableEntity.rowKey,
bodyURL = tableEntity.getProperty("body_url") as String,
detail = tableEntity.getProperty("detail") as String?
detail = tableEntity.getProperty("detail") as String?,
timestamp = tableEntity.timestamp
)
}
}
Expand Down