Skip to content

Commit

Permalink
Pivotal ID # 185361632: Avoid Files Modification On Public Studies (#855
Browse files Browse the repository at this point in the history
)

https://www.pivotaltracker.com/story/show/185361632

- Create a new stage to validate file changes
- Add new permission to allow file modifications on public studies
- Mark the submission as invalid in case the file validation fails
- Add integration tests
  • Loading branch information
jhoanmanuelms authored Jul 25, 2024
1 parent 47d7bc4 commit 8d99502
Show file tree
Hide file tree
Showing 57 changed files with 738 additions and 1,748 deletions.
2 changes: 1 addition & 1 deletion buildSrc/src/main/kotlin/Dependencies.kt
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ object Versions {
const val CommonsFileUploadVersion = "1.4"
const val CommonsLang3Version = "3.8.1"
const val CommonsIOVersion = "2.6"
const val CommonsNetVersion = "3.6"
const val CommonsNetVersion = "3.11.1"
const val CommonsPoolVersion = "2.12.0"
const val CommonsCsvVersion = "1.8"
const val MySqlVersion = "8.3.0"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ebi.ac.uk.ftp

import org.apache.commons.net.ftp.FTP
import org.apache.commons.net.ftp.FTPClient
import org.apache.commons.net.ftp.FTPFile
import java.io.InputStream
Expand Down Expand Up @@ -72,6 +73,7 @@ private class SimpleFtpClient(
ftpClientPool.execute { ftp ->
for ((path, inputStream) in files) {
ftp.createFtpFolder(path.parent)
ftp.setFileType(FTP.BINARY_FILE_TYPE)
inputStream().use { ftp.storeFile(path.toString(), it) }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ class FtpClientTest {
testInstance.uploadFile(rootPath.resolve("a-folder").resolve("file1.txt")) { tempFile.inputStream() }

val files = testInstance.listFiles(rootPath)
assertThat(files).hasOnlyOneElementSatisfying { it.name == "a-folder" && it.isDirectory }
assertThat(files).satisfiesOnlyOnce { it.name == "a-folder" && it.isDirectory } // .hasOnlyOneElementSatisfying { }

val folderFiles = testInstance.listFiles(rootPath.resolve("a-folder"))
assertThat(folderFiles).hasOnlyOneElementSatisfying { it.name == "file1.txt" && it.isFile }
assertThat(folderFiles).satisfiesOnlyOnce { it.name == "file1.txt" && it.isFile }
}

@RetryingTest(TEST_RETRY)
Expand Down Expand Up @@ -71,9 +71,9 @@ class FtpClientTest {
val file = files.first()
assertThat(file.name).isEqualTo("test-file.txt")

val outputFile = createTempFile()
val outputFile = createTempFile("")
outputFile.outputStream().use { testInstance.downloadFile(filePath, it) }
assertThat(outputFile).hasSameContentAs(tempFile)
assertThat(outputFile).hasSameBinaryContentAs(tempFile)
}

companion object {
Expand All @@ -87,7 +87,7 @@ class FtpClientTest {
)
}

val HOME = Paths.get("")
private val HOME = Paths.get("")
const val FTP_USER = "ftpUser"
const val FTP_PASSWORD = "ftpPassword"
const val FTP_ROOT_PATH = ".test"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ private val validPathPattern =
append("\$") // End of the line
}.trimIndent().toRegex()

class FileSourcesList(val checkFilesPath: Boolean, val sources: List<FilesSource>) {
class FileSourcesList(private val checkFilesPath: Boolean, val sources: List<FilesSource>) {
suspend fun findExtFile(
path: String,
type: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ enum class RequestStatus {
REQUESTED,
INDEXED,
LOADED,
VALIDATED,
INDEXED_CLEANED,
CLEANED,
FILES_COPIED,
CHECK_RELEASED,
PERSISTED,
PROCESSED,
INVALID,
;

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@ data class RequestIndexed(
@JsonProperty("version") override val version: Int,
) : RequestMessage

data class RequestLoaded(
@JsonProperty("accNo") override val accNo: String,
@JsonProperty("version") override val version: Int,
) : RequestMessage

data class RequestToCleanIndexed(
@JsonProperty("accNo") override val accNo: String,
@JsonProperty("version") override val version: Int,
) : RequestMessage

data class RequestLoaded(
data class RequestValidated(
@JsonProperty("accNo") override val accNo: String,
@JsonProperty("version") override val version: Int,
) : RequestMessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ExtFileExtensionsTest {
val newAttributes = listOf(ExtAttribute("Override", "New"))
val copied = fireFile.copyWithAttributes(newAttributes)

assertThat(copied).isEqualToIgnoringGivenFields(fireFile, "attributes")
assertThat(copied).usingRecursiveComparison().ignoringFields("attributes").isEqualTo(fireFile)
assertThat(copied.attributes).isEqualTo(newAttributes)
}

Expand All @@ -49,7 +49,7 @@ class ExtFileExtensionsTest {
val newAttributes = listOf(ExtAttribute("Override", "New"))
val copied = nfsFile.copyWithAttributes(newAttributes)

assertThat(copied).isEqualToIgnoringGivenFields(nfsFile, "attributes")
assertThat(copied).usingRecursiveComparison().ignoringFields("attributes").isEqualTo(nfsFile)
assertThat(copied.attributes).isEqualTo(newAttributes)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class JsonSerializerTest(
fileSystem.outputStream().use { jsonSerializer.serializeFileList(files.asSequence(), it) }
val response = fileSystem.inputStream().use { jsonSerializer.deserializeFileList(it).toList() }

assertThat(response).allSatisfy { assertThat(it).isEqualToComparingFieldByField(iterator.next()) }
assertThat(response).allSatisfy { assertThat(it).usingRecursiveComparison().isEqualTo(iterator.next()) }
assertThat(response).hasSize(20_000)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class FileListTsvStreamDeserializerTest(
output.outputStream().use { testInstance.serializeFileList(files.asFlow(), it) }

val result = output.inputStream().use { testInstance.deserializeFileList(it).toList() }
assertThat(result).allSatisfy { assertThat(it).isEqualToComparingFieldByField(iterator.next()) }
assertThat(result).allSatisfy { assertThat(it).usingRecursiveComparison().isEqualTo(iterator.next()) }
assertThat(result).hasSize(20000)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class TsvDeserializerTest {
fun `basic submission`() {
val result = deserializer.deserialize(basicSubmission().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC123") {
attribute("Title", "Basic Submission")
attribute("DataSource", "EuropePMC")
Expand All @@ -68,7 +68,7 @@ class TsvDeserializerTest {
fun `submission with empty attribute`() {
val result = deserializer.deserialize(submissionWithEmptyAttribute().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC123") {
attribute("Title", "Basic Submission")
attribute("DataSource", "EuropePMC")
Expand All @@ -81,7 +81,7 @@ class TsvDeserializerTest {
fun `submission with blank attribute`() {
val result = deserializer.deserialize(submissionWithBlankAttribute().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC123") {
attribute("Title", "Basic Submission")
attribute("DataSource", "EuropePMC")
Expand All @@ -94,7 +94,7 @@ class TsvDeserializerTest {
fun `submission with null attribute`() {
val result = deserializer.deserialize(submissionWithNullAttribute().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC123") {
attribute("Title", "Basic Submission")
attribute("DataSource", "EuropePMC")
Expand All @@ -107,7 +107,7 @@ class TsvDeserializerTest {
fun `submission with quoted value`() {
val result = deserializer.deserialize(submissionWithQuoteValue().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC123") {
attribute("Title", "The \"Submission\": title.")
attribute("Abstract", "\"The Submission\": this is description.")
Expand All @@ -121,7 +121,7 @@ class TsvDeserializerTest {
fun `basic submission with comments`() {
val result = deserializer.deserialize(basicSubmissionWithComments().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC123") {
attribute("Title", "Basic Submission")
attribute("DataSource", "EuropePMC")
Expand All @@ -134,7 +134,7 @@ class TsvDeserializerTest {
fun `submission with multiline attribute value`() {
val result = deserializer.deserialize(basicSubmissionWithMultiline().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC123") {
attribute("Title", "This is a really long title \n with a break line")
attribute("Another", "another attribute")
Expand All @@ -146,7 +146,7 @@ class TsvDeserializerTest {
fun `detailed attributes`() {
val result = deserializer.deserialize(submissionWithDetailedAttributes().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC124") {
attribute("Title", "Submission With Detailed Attributes")

Expand All @@ -167,7 +167,7 @@ class TsvDeserializerTest {
fun `submission with root section`() {
val result = deserializer.deserialize(submissionWithRootSection().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand All @@ -183,7 +183,7 @@ class TsvDeserializerTest {
fun `submission with generic root section`() {
val result = deserializer.deserialize(submissionWithGenericRootSection().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")
section("Compound") {
Expand All @@ -197,7 +197,7 @@ class TsvDeserializerTest {
fun `submission with multiple line breaks`() {
val result = deserializer.deserialize(submissionWithMultipleLineBreaks().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand All @@ -213,7 +213,7 @@ class TsvDeserializerTest {
fun subsection() {
val result = deserializer.deserialize(submissionWithSubsection().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand All @@ -235,7 +235,7 @@ class TsvDeserializerTest {
fun `inner subsections`() {
val result = deserializer.deserialize(submissionWithInnerSubsections().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand Down Expand Up @@ -278,7 +278,7 @@ class TsvDeserializerTest {
fun `inner subsections table`() {
val result = deserializer.deserialize(submissionWithInnerSubsectionsTable().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand Down Expand Up @@ -321,7 +321,7 @@ class TsvDeserializerTest {
fun links() {
val result = deserializer.deserialize(submissionWithLinks().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand All @@ -340,7 +340,7 @@ class TsvDeserializerTest {
fun `links table with attribute details`() {
val result = deserializer.deserialize(submissionWithLinksTable().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand Down Expand Up @@ -376,7 +376,7 @@ class TsvDeserializerTest {
fun files() {
val result = deserializer.deserialize(submissionWithFiles().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand All @@ -396,7 +396,7 @@ class TsvDeserializerTest {
fun `files table`() {
val result = deserializer.deserialize(submissionWithFilesTable().toString())

assertThat(result).isEqualToComparingFieldByField(
assertThat(result).usingRecursiveComparison().isEqualTo(
submission("S-EPMC125") {
attribute("Title", "Test Submission")

Expand Down Expand Up @@ -526,7 +526,7 @@ class TsvDeserializerTest {
assertEither(sections.first()).hasRightValueSatisfying {
val innerSections = it.elements
assertThat(innerSections).hasSize(1)
assertThat(innerSections.first()).isEqualToComparingFieldByField(
assertThat(innerSections.first()).usingRecursiveComparison().isEqualTo(
Section(
type = "Data",
accNo = "DT-1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class TsvSingleElementDeserializationTest {
line()
}.toString()

assertThat(deserializer.deserializeElement<BioFile>(tsv)).isEqualToComparingFieldByField(
assertThat(deserializer.deserializeElement<BioFile>(tsv)).usingRecursiveComparison().isEqualTo(
file("File1.txt") {
attribute("Attr", "Value")
},
Expand All @@ -43,7 +43,7 @@ class TsvSingleElementDeserializationTest {
line()
}.toString()

assertThat(deserializer.deserializeElement<FilesTable>(tsv)).isEqualToComparingFieldByField(
assertThat(deserializer.deserializeElement<FilesTable>(tsv)).usingRecursiveComparison().isEqualTo(
filesTable {
file("File1.txt") {
attribute("Attr", "Value")
Expand All @@ -61,7 +61,7 @@ class TsvSingleElementDeserializationTest {
line()
}.toString()

assertThat(deserializer.deserializeElement<Link>(tsv)).isEqualToComparingFieldByField(
assertThat(deserializer.deserializeElement<Link>(tsv)).usingRecursiveComparison().isEqualTo(
link("http://alink.org") {
attribute("Attr", "Value")
},
Expand All @@ -77,7 +77,7 @@ class TsvSingleElementDeserializationTest {
line()
}.toString()

assertThat(deserializer.deserializeElement<LinksTable>(tsv)).isEqualToComparingFieldByField(
assertThat(deserializer.deserializeElement<LinksTable>(tsv)).usingRecursiveComparison().isEqualTo(
linksTable {
link("http://alink.org") {
attribute("Attr", "Value")
Expand All @@ -98,7 +98,7 @@ class TsvSingleElementDeserializationTest {
line()
}.toString()

assertThat(deserializer.deserializeElement<LinksTable>(tsv)).isEqualToComparingFieldByField(
assertThat(deserializer.deserializeElement<LinksTable>(tsv)).usingRecursiveComparison().isEqualTo(
linksTable {
link("AF069307") {
attribute("Attr1", "Value 1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ internal class FileUtilsTest(private val temporaryFolder: TemporaryFolder) {

assertThat(target).isDirectory()
assertThat(nestedDir).isDirectory()
assertThat(nestedFile).hasSameContentAs(subDirFile)
assertThat(nestedFile).hasSameTextualContentAs(subDirFile)
assertThat(getPosixFilePermissions(target.toPath())).containsExactlyInAnyOrderElementsOf(RWX______)
assertThat(getPosixFilePermissions(nestedDir.toPath())).containsExactlyInAnyOrderElementsOf(RWX______)
assertThat(getPosixFilePermissions(nestedFile.toPath())).containsExactlyInAnyOrderElementsOf(RW_______)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import ebi.ac.uk.extended.events.RequestLoaded
import ebi.ac.uk.extended.events.RequestMessage
import ebi.ac.uk.extended.events.RequestPersisted
import ebi.ac.uk.extended.events.RequestToCleanIndexed
import ebi.ac.uk.extended.events.RequestValidated
import ebi.ac.uk.extended.events.SecurityNotification
import ebi.ac.uk.extended.events.SubmissionMessage
import org.springframework.amqp.rabbit.core.RabbitTemplate
Expand Down Expand Up @@ -77,6 +78,15 @@ class EventsPublisherService(
RequestToCleanIndexed(accNo, version),
)

fun requestValidated(
accNo: String,
version: Int,
) = rabbitTemplate.convertAndSend(
BIOSTUDIES_EXCHANGE,
notificationsProperties.requestRoutingKey,
RequestValidated(accNo, version),
)

fun requestCleaned(
accNo: String,
version: Int,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ enum class AccessType {
ATTACH,
UPDATE,
DELETE,
UPDATE_PUBLIC,
ADMIN,
}

Expand Down
Loading

0 comments on commit 8d99502

Please sign in to comment.