Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
fbc7539
Initial write enabled flag
tombch Jun 16, 2026
cc74644
Merge branch 'main' into crossref-write-enabled
tombch Jun 16, 2026
e37199c
Change crossref active test to check for null or blank
tombch Jun 16, 2026
d603981
Check crossref service active and writeable before creating seqset do…
tombch Jun 16, 2026
14e549b
Update tests to use mock crossref as active, with crossref generate a…
tombch Jun 16, 2026
8f3a463
Update schema description
tombch Jun 16, 2026
2d691f0
Merge branch 'crossref-write-enabled' of github.com:loculus-project/l…
tombch Jun 16, 2026
bc7eb8d
Fixed typo
tombch Jun 16, 2026
369396c
Fixed tests
tombch Jun 16, 2026
36f88f9
feat: get rid of unnecessary toString() conversion
anna-parker Jun 16, 2026
e073d08
Remove use of non-existing github env vars that were set to blank and…
tombch Jun 16, 2026
7c9044f
Merge branch 'crossref-write-enabled' of github.com:loculus-project/l…
tombch Jun 16, 2026
d33d50f
Add tests for forbidden when creating seqset DOI when crossref is not…
tombch Jun 16, 2026
1842c76
To revert: testing active crossref but write disabled
tombch Jun 16, 2026
962c75b
Fixing dummy username/password
tombch Jun 16, 2026
3653a74
There is already a dummy secret
tombch Jun 16, 2026
e84af52
Merge branch 'main' into crossref-write-enabled
tombch Jun 16, 2026
a506be9
Reverting dummy crossref values
tombch Jun 16, 2026
c6e90e3
Merge branch 'crossref-write-enabled' of github.com:loculus-project/l…
tombch Jun 16, 2026
a3c9eba
Merge branch 'main' into crossref-write-enabled
tombch Jun 16, 2026
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
4 changes: 0 additions & 4 deletions .github/workflows/backend-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: 15
env:
CROSSREF_USERNAME: ${{ secrets.CROSSREF_USERNAME }}
CROSSREF_PASSWORD: ${{ secrets.CROSSREF_PASSWORD }}
CROSSREF_ENDPOINT: ${{ secrets.CROSSREF_ENDPOINT }}
CROSSREF_DOI_PREFIX: ${{ secrets.CROSSREF_DOI_PREFIX }}
RUN_EXTRA_TESTS: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' && 'true' || 'false' }}
steps:
- name: Login to Docker Hub
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ data class CrossRefServiceProperties(
val email: String?,
val organization: String?,
val hostUrl: String?,
val writeEnabled: Boolean?,
)

data class DoiEntry(
Expand All @@ -54,14 +55,15 @@ data class CrossRefCitedByResult(

@Service
class CrossRefService(private val properties: CrossRefServiceProperties, private val dateProvider: DateProvider) {
val isActive = properties.endpoint != null &&
properties.username != null &&
properties.password != null &&
properties.doiPrefix != null &&
properties.databaseName != null &&
properties.email != null &&
properties.organization != null &&
properties.hostUrl != null
val isActive = !properties.endpoint.isNullOrBlank() &&
!properties.username.isNullOrBlank() &&
!properties.password.isNullOrBlank() &&
!properties.doiPrefix.isNullOrBlank() &&
!properties.databaseName.isNullOrBlank() &&
!properties.email.isNullOrBlank() &&
!properties.organization.isNullOrBlank() &&
!properties.hostUrl.isNullOrBlank()
val isWriteEnabled = properties.writeEnabled == true
val doiPrefix: String? = properties.doiPrefix
val dateTimeFormatterMM: DateTimeFormatter = DateTimeFormatter.ofPattern("MM")
val dateTimeFormatterdd: DateTimeFormatter = DateTimeFormatter.ofPattern("dd")
Expand All @@ -73,6 +75,12 @@ class CrossRefService(private val properties: CrossRefServiceProperties, private
}
}

private fun checkIsWriteEnabled() {
if (!isWriteEnabled) {
throw RuntimeException("The CrossRefService is read-only so this action is not permitted.")
}
}

fun parseCrossRefCitedByXML(citedByXML: String): CrossRefCitedByResult {
val parser = Parser.xmlParser().setTrackErrors(1)
val doc = Jsoup.parse(citedByXML, "", parser)
Expand Down Expand Up @@ -280,6 +288,7 @@ class CrossRefService(private val properties: CrossRefServiceProperties, private

fun postCrossRefXML(XML: String): String {
checkIsActive()
checkIsWriteEnabled()

// This is needed per their API specification
val formData = mapOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import org.loculus.backend.api.Status.APPROVED_FOR_RELEASE
import org.loculus.backend.api.SubmittedSeqSetRecord
import org.loculus.backend.auth.AuthenticatedUser
import org.loculus.backend.config.BackendConfig
import org.loculus.backend.controller.ForbiddenException
import org.loculus.backend.controller.NotFoundException
import org.loculus.backend.controller.UnprocessableEntityException
import org.loculus.backend.service.crossref.CrossRefService
Expand Down Expand Up @@ -107,7 +108,7 @@ class SeqSetCitationsDatabaseService(
}

return ResponseSeqSet(
insertedSet[SeqSetsTable.seqSetId].toString(),
insertedSet[SeqSetsTable.seqSetId],
insertedSet[SeqSetsTable.seqSetVersion],
)
}
Expand Down Expand Up @@ -186,7 +187,7 @@ class SeqSetCitationsDatabaseService(
}

return ResponseSeqSet(
insertedSet[SeqSetsTable.seqSetId].toString(),
insertedSet[SeqSetsTable.seqSetId],
insertedSet[SeqSetsTable.seqSetVersion],
)
}
Expand Down Expand Up @@ -342,6 +343,14 @@ class SeqSetCitationsDatabaseService(
val username = authenticatedUser.username
log.info { "Create DOI for seqSet $seqSetId, version $version, user $username" }

if (!crossRefService.isActive) {
throw ForbiddenException("The crossref service is not active, so creating DOIs is forbidden.")
}

if (!crossRefService.isWriteEnabled) {
throw ForbiddenException("The crossref service is not write-enabled, so creating DOIs is forbidden.")
}

validateCreateSeqSetDOI(username, seqSetId, version)

val doiPrefix = crossRefService.doiPrefix ?: "no-prefix-configured"
Expand All @@ -359,18 +368,16 @@ class SeqSetCitationsDatabaseService(
it[SeqSetsTable.seqSetDOI] = seqSetDOI
}

if (crossRefService.isActive) {
val crossRefXml = crossRefService.generateCrossRefXML(
DoiEntry(
LocalDate.now(),
"SeqSet: ${seqSet[0].name}",
seqSetDOI,
"/seqsets/$seqSetId.$version",
null,
),
)
crossRefService.postCrossRefXML(crossRefXml)
}
val crossRefXml = crossRefService.generateCrossRefXML(
DoiEntry(
LocalDate.now(),
"SeqSet: ${seqSet[0].name}",
seqSetDOI,
"/seqsets/$seqSetId.$version",
null,
),
)
crossRefService.postCrossRefXML(crossRefXml)

return ResponseSeqSet(
seqSetId,
Expand Down Expand Up @@ -474,10 +481,10 @@ class SeqSetCitationsDatabaseService(
mutableListOf(),
)

val uniqueSeqSetIds = latestSeqSetWithUserAccession.map { it.seqSetId.toString() }.toSet()
val uniqueSeqSetIds = latestSeqSetWithUserAccession.map { it.seqSetId }.toSet()
for (seqSetId in uniqueSeqSetIds) {
val year = latestSeqSetWithUserAccession
.first { it.seqSetId.toString() == seqSetId }
.first { it.seqSetId == seqSetId }
.createdAt.toLocalDateTime().year.toLong()
if (citedBy.years.contains(year)) {
val index = citedBy.years.indexOf(year)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ class CitationEndpointsTest(
} returns listOf(AccessionVersion(MOCK_SEQ_ACCESSION, MOCK_SEQ_VERSION))
every { accessionPreconditionValidator.validate(any()) } returns Unit
every { crossRefService.doiPrefix } returns MOCK_DOI_PREFIX
every { crossRefService.isActive } returns false
every { crossRefService.isActive } returns true
every { crossRefService.isWriteEnabled } returns true
every { crossRefService.generateCrossRefXML(any()) } returns "<doi_batch/>"
every { crossRefService.postCrossRefXML(any()) } returns "Crossref API response"
}

@ParameterizedTest
Expand Down Expand Up @@ -117,7 +120,6 @@ class CitationEndpointsTest(
val seqSetVersion = JsonPath.read<Int>(seqSetResult.response.contentAsString, "$.seqSetVersion").toLong()

// Simulate running the crossref citations task
every { crossRefService.isActive } returns true
every { crossRefService.getCrossRefCitedBy(MOCK_DOI_PREFIX) } returns
CrossRefCitedByResult(emptyList(), emptyList())
seqSetCrossRefCitationsTask.task()
Expand Down Expand Up @@ -157,7 +159,6 @@ class CitationEndpointsTest(
),
seqSetDOIs = setOf(seqSetDOI),
)
every { crossRefService.isActive } returns true
every { crossRefService.getCrossRefCitedBy(MOCK_DOI_PREFIX) } returns
CrossRefCitedByResult(listOf(seqSetCitationSource), emptyList())
seqSetCrossRefCitationsTask.task()
Expand Down Expand Up @@ -204,8 +205,6 @@ class CitationEndpointsTest(
val (seqSetIdA, seqSetVersionA, seqSetDOIA) = createSeqSetWithDOI()
val (seqSetIdB, seqSetVersionB, seqSetDOIB) = createSeqSetWithDOI()

every { crossRefService.isActive } returns true

val citationSource = SeqSetCitationSource(
CitationSource(
sourceDOI = "10.5678/citing-paper",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ class SeqSetEndpointsTest(@Autowired private val client: SeqSetCitationsControll
fun setup() {
every { accessionPreconditionValidator.validate(any()) } returns Unit
every { crossRefService.doiPrefix } returns MOCK_DOI_PREFIX
every { crossRefService.isActive } returns false
every { crossRefService.isActive } returns true
every { crossRefService.isWriteEnabled } returns true
every { crossRefService.generateCrossRefXML(any()) } returns "<doi_batch/>"
every { crossRefService.postCrossRefXML(any()) } returns "Crossref API response"
}

@ParameterizedTest
Expand Down Expand Up @@ -122,6 +125,36 @@ class SeqSetEndpointsTest(@Autowired private val client: SeqSetCitationsControll
.andExpect(jsonPath("\$[0].seqSetDOI").isString)
}

@Test
fun `WHEN calling create seqSet DOI while write is disabled THEN returns forbidden`() {
every { crossRefService.isWriteEnabled } returns false

val seqSetResult = client.createSeqSet()
.andExpect(status().isOk)
.andReturn()
val seqSetId = JsonPath.read<String>(seqSetResult.response.contentAsString, "$.seqSetId")

client.createSeqSetDOI(seqSetId)
.andExpect(status().isForbidden)
.andExpect(content().contentType(MediaType.APPLICATION_PROBLEM_JSON))
.andExpect(jsonPath("\$.detail", containsString("not write-enabled")))
}

@Test
fun `WHEN calling create seqSet DOI while crossref is inactive THEN returns forbidden`() {
every { crossRefService.isActive } returns false

val seqSetResult = client.createSeqSet()
.andExpect(status().isOk)
.andReturn()
val seqSetId = JsonPath.read<String>(seqSetResult.response.contentAsString, "$.seqSetId")

client.createSeqSetDOI(seqSetId)
.andExpect(status().isForbidden)
.andExpect(content().contentType(MediaType.APPLICATION_PROBLEM_JSON))
.andExpect(jsonPath("\$.detail", containsString("not active")))
}

@Test
fun `WHEN calling update seqSet THEN returns updated seqSet`() {
val seqSetResult = client.createSeqSet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ class SeqSetValidationEndpointsTest(
@BeforeEach
fun setup() {
every { crossRefService.doiPrefix } returns MOCK_DOI_PREFIX
every { crossRefService.isActive } returns false
every { crossRefService.isActive } returns true
every { crossRefService.isWriteEnabled } returns true
every { crossRefService.generateCrossRefXML(any()) } returns "<doi_batch/>"
every { crossRefService.postCrossRefXML(any()) } returns "Crossref API response"
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.loculus.backend.service.crossref

import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
Expand All @@ -10,6 +11,7 @@ import org.loculus.backend.SpringBootTestWithoutDatabase
import org.loculus.backend.api.CitationContributor
import org.loculus.backend.api.CitationSource
import org.loculus.backend.api.SeqSetCitationSource
import org.loculus.backend.utils.DateProvider
import org.springframework.beans.factory.annotation.Autowired
import java.time.Instant
import java.time.LocalDate
Expand All @@ -30,6 +32,21 @@ class CrossRefServiceTest(@Autowired private val crossRefService: CrossRefServic
<doi_batch version="5.3.1" xmlns="http://www.crossref.org/schema/5.3.1" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.crossref.org/schema/5.3.1 http://data.crossref.org/schemas/crossref5.3.1.xsd"><head><doi_batch_id>$doiBatchID</doi_batch_id><timestamp>1711411200000</timestamp><depositor><depositor_name>Loculus Database</depositor_name><email_address>dois@loculus.org</email_address></depositor><registrant>Loculus Database</registrant></head><body><database><database_metadata><titles><title>Loculus Database</title></titles></database_metadata><dataset><contributors><organization contributor_role="author" sequence="first">loculus.org</organization></contributors><titles><title>SeqSet: My test set</title></titles><database_date><publication_date><month>03</month><day>26</day><year>2024</year></publication_date></database_date><doi_data><doi>$doi</doi><resource>https://main.loculus.org/seqsets/LOC_SS_1.1</resource></doi_data></dataset></database></body></doi_batch>
""".trimIndent()

private fun crossRefService(writeEnabled: Boolean?) = CrossRefService(
CrossRefServiceProperties(
endpoint = "dummy",
username = "dummy",
password = "dummy",
doiPrefix = "placeholder",
databaseName = "Loculus Database",
email = "dois@loculus.org",
organization = "loculus.org",
hostUrl = "https://main.loculus.org",
writeEnabled = writeEnabled,
),
DateProvider(),
)

@Test
fun `parseCrossRefCitedByXML returns citations from valid XML across different citation types`() {
val xml = """
Expand Down Expand Up @@ -270,6 +287,21 @@ class CrossRefServiceTest(@Autowired private val crossRefService: CrossRefServic
)
}

@Test
fun `postCrossRefXML is rejected when write is not enabled`() {
val readOnlyService = crossRefService(writeEnabled = false)
val ex = assertThrows<RuntimeException> {
readOnlyService.postCrossRefXML(crossRefXMLReference)
}
assertTrue(ex.message!!.contains("read-only", ignoreCase = true))
}

@Test
fun `crossref write-enabled=true property string is coerced to the boolean flag`() {
// Application properties sets crossref.write-enabled=true
assertTrue(crossRefService.isWriteEnabled)
}

@Test
fun `Create an XML metadata string complying with CrossRef's schema`() {
val crossRefXML = crossRefService.generateCrossRefXML(
Expand Down
1 change: 1 addition & 0 deletions backend/src/test/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ crossref.database-name=Loculus Database
crossref.email=dois@loculus.org
crossref.organization=loculus.org
crossref.host-url=https://main.loculus.org
crossref.write-enabled=true

keycloak.user=dummy
keycloak.password=dummy
Expand Down
3 changes: 3 additions & 0 deletions kubernetes/loculus/templates/loculus-backend.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ spec:
- "--crossref.email=$(CROSSREF_EMAIL)"
- "--crossref.organization=$(CROSSREF_ORGANIZATION)"
- "--crossref.host-url=$(CROSSREF_HOST_URL)"
- "--crossref.write-enabled=$(CROSSREF_WRITE_ENABLED)"
{{- end }}
- "--keycloak.password=$(BACKEND_KEYCLOAK_PASSWORD)"
- "--keycloak.realm=loculus"
Expand Down Expand Up @@ -108,6 +109,8 @@ spec:
value: {{$.Values.seqSets.crossRef.organization | quote }}
- name: CROSSREF_HOST_URL
value: {{$.Values.seqSets.crossRef.hostUrl | quote }}
- name: CROSSREF_WRITE_ENABLED
value: {{$.Values.seqSets.crossRef.writeEnabled | quote }}
{{- end }}
- name: BACKEND_KEYCLOAK_PASSWORD
valueFrom:
Expand Down
6 changes: 6 additions & 0 deletions kubernetes/loculus/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,12 @@
"groups": ["general"],
"type": "string",
"description": "The host URL for CrossRef callbacks"
},
"writeEnabled": {
"groups": ["general"],
"type": "boolean",
"default": false,
"description": "Enable/disable Crossref write functionality, such as generating DOIs"
}
}
},
Expand Down
Loading