From 12d0bac62d07b02a2f2d59eb868ac0fb8a95249a Mon Sep 17 00:00:00 2001 From: Urs Joss Date: Sun, 26 Feb 2023 10:13:16 +0100 Subject: [PATCH] feat: [#129] Change M1 implementation from RisRecord.number to RisRecord.miscellaneous1 using String --- .../kotlin/ch/difty/kris/domain/RisRecord.kt | 16 +++-- .../kotlin/ch/difty/kris/domain/RisTag.kt | 17 +++-- .../kris/domain/RisRecordBuilderTest.java | 26 +++++++- .../ch/difty/kris/KRisProcessFromListTest.kt | 4 +- .../ch/difty/kris/KRisProcessingSpec.kt | 12 ++-- .../ch/difty/kris/domain/RisRecordSpec.kt | 32 ++++++++-- .../kotlin/ch/difty/kris/domain/RisTagTest.kt | 11 ++-- .../ch/difty/kris/usage/KRisUsageSpec.kt | 62 +++++++++++++++++++ 8 files changed, 150 insertions(+), 30 deletions(-) diff --git a/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisRecord.kt b/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisRecord.kt index 22a9e4ae..7777ec8f 100644 --- a/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisRecord.kt +++ b/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisRecord.kt @@ -2,10 +2,6 @@ package ch.difty.kris.domain /** * A single RIS record. It contains all the allowed tag from RIS format. - * - * @author Gianluca Colaianni -- g.colaianni5@gmail.com - * @version 1.0 - * @since 22 apr 2017 */ @Suppress("ParameterListWrapping", "SpellCheckingInspection", "TooManyFunctions") public data class RisRecord( @@ -172,7 +168,12 @@ public data class RisRecord( /** LK */ public var websiteLink: String? = null, - /** M1 */ + /** M1. Often used for Number. This is an alphanumeric field, thus suporting e.g. ranges or chars */ + public var miscellaneous1: String? = null, + + // TODO remove in #132 + /** M1 - deprecated */ + @Deprecated("Use miscellaneous1 (returning a nullable String) instead", ReplaceWith("miscellaneous1")) public var number: Long? = null, /** @@ -339,6 +340,7 @@ public data class RisRecord( private var language: String? = null private var label: String? = null private var websiteLink: String? = null + private var miscellaneous1: String? = null private var number: Long? = null private var miscellaneous2: String? = null private var typeOfWork: String? = null @@ -460,6 +462,9 @@ public data class RisRecord( public fun language(language: String?): Builder = apply { this.language = language } public fun label(label: String?): Builder = apply { this.label = label } public fun websiteLink(websiteLink: String?): Builder = apply { this.websiteLink = websiteLink } + public fun miscellaneous1(miscellaneous1: String?): Builder = apply { this.miscellaneous1 = miscellaneous1 } + + @Deprecated("use miscellaneous1(number.toString()) instead", ReplaceWith("miscellaneous1(number.toString())")) public fun number(number: Long?): Builder = apply { this.number = number } public fun miscellaneous2(miscellaneous2: String?): Builder = apply { this.miscellaneous2 = miscellaneous2 } public fun typeOfWork(typeOfWork: String?): Builder = apply { this.typeOfWork = typeOfWork } @@ -549,6 +554,7 @@ public data class RisRecord( language, label, websiteLink, + miscellaneous1, number, miscellaneous2, typeOfWork, diff --git a/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisTag.kt b/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisTag.kt index db0f3d19..b41a4cf9 100644 --- a/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisTag.kt +++ b/subprojects/kris-core/src/main/kotlin/ch/difty/kris/domain/RisTag.kt @@ -381,12 +381,19 @@ public enum class RisTag( getFrom = { r: RisRecord -> r.websiteLink } ), - /** Number */ + /** Miscellaneous 1. Often used for Number or Range of Numbers */ + @Suppress("Deprecation") M1( - description = "Number", - setInto = { r, v -> r.number = v as Long? }, - getFrom = { r: RisRecord -> r.number }, - kClass = Long::class + description = "Miscellaneous 1 (often Number)", + setInto = { r, v -> + r.miscellaneous1 = v as String? + r.number = v?.toLongOrNull() + }, + getFrom = { r: RisRecord -> + r.miscellaneous1.takeUnless { it.isNullOrBlank() } + ?: r.number + }, + kClass = String::class ), /** Miscellaneous 2. This is an alphanumeric field and there is no practical limit to the length of this field. */ diff --git a/subprojects/kris-core/src/test/java/ch/difty/kris/domain/RisRecordBuilderTest.java b/subprojects/kris-core/src/test/java/ch/difty/kris/domain/RisRecordBuilderTest.java index 1456e259..758e52ad 100644 --- a/subprojects/kris-core/src/test/java/ch/difty/kris/domain/RisRecordBuilderTest.java +++ b/subprojects/kris-core/src/test/java/ch/difty/kris/domain/RisRecordBuilderTest.java @@ -55,7 +55,7 @@ class RisRecordBuilderTest { .language("language") .label("label") .websiteLink("websiteLink") - .number(1L) + .miscellaneous1("number") .miscellaneous2("miscellaneous2") .typeOfWork("typeOfWork") .notes("notes") @@ -138,7 +138,7 @@ void canFillAllFieldsIntoDataClassAppropriately() { assertThat(risRecord.getLanguage()).isEqualTo("language"); assertThat(risRecord.getLabel()).isEqualTo("label"); assertThat(risRecord.getWebsiteLink()).isEqualTo("websiteLink"); - assertThat(risRecord.getNumber()).isEqualTo(1L); + assertThat(risRecord.getMiscellaneous1()).isEqualTo("number"); assertThat(risRecord.getMiscellaneous2()).isEqualTo("miscellaneous2"); assertThat(risRecord.getTypeOfWork()).isEqualTo("typeOfWork"); assertThat(risRecord.getNotes()).isEqualTo("notes"); @@ -222,7 +222,7 @@ void emptyRecord() { assertThat(risRecord.getLanguage()).isNull(); assertThat(risRecord.getLabel()).isNull(); assertThat(risRecord.getWebsiteLink()).isNull(); - assertThat(risRecord.getNumber()).isNull(); + assertThat(risRecord.getMiscellaneous1()).isNull(); assertThat(risRecord.getMiscellaneous2()).isNull(); assertThat(risRecord.getTypeOfWork()).isNull(); assertThat(risRecord.getNotes()).isNull(); @@ -256,4 +256,24 @@ void emptyRecord() { assertThat(risRecord.getPrimaryDate()).isNull(); assertThat(risRecord.getAccessDate()).isNull(); } + + @SuppressWarnings("deprecation") + @Test + public void givenRisRecordWithMisc1_providesMisc1ButNotNumber() { + RisRecord risRecord = new RisRecord.Builder() + .miscellaneous1("1234") + .build(); + assertThat(risRecord.getMiscellaneous1()).isEqualTo("1234"); + assertThat(risRecord.getNumber()).isNull(); + } + + @SuppressWarnings("deprecation") + @Test + public void givenRisRecordWithNumber_providesNumberButNotMisc1() { + RisRecord risRecord = new RisRecord.Builder() + .number(1234L) + .build(); + assertThat(risRecord.getMiscellaneous1()).isNull(); + assertThat(risRecord.getNumber()).isEqualTo(1234L); + } } diff --git a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessFromListTest.kt b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessFromListTest.kt index 732a7bc8..d99556c0 100644 --- a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessFromListTest.kt +++ b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessFromListTest.kt @@ -56,7 +56,7 @@ class KRisProcessFromListTest { language = "language", label = "label", websiteLink = "websiteLink", - number = 1L, + miscellaneous1 = "number", miscellaneous2 = "miscellaneous2", typeOfWork = "typeOfWork", notes = "notes", @@ -148,7 +148,7 @@ class KRisProcessFromListTest { |LA - language |LB - label |LK - websiteLink - |M1 - 1 + |M1 - number |M2 - miscellaneous2 |M3 - typeOfWork |N1 - notes diff --git a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessingSpec.kt b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessingSpec.kt index e63dffb1..0505249a 100644 --- a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessingSpec.kt +++ b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/KRisProcessingSpec.kt @@ -44,7 +44,9 @@ object KRisProcessingSpec : DescribeSpec({ it("should have author $author") { risRecords.first().authors.first() shouldBeEqualTo author } it("should have publication year $pubYear") { risRecords.first().publicationYear shouldBeEqualTo pubYear } it("should have title $title") { risRecords.first().title shouldBeEqualTo title } - it("should have secondary/journal title $journalTitle") { risRecords.first().secondaryTitle shouldBeEqualTo journalTitle } + it("should have secondary/journal title $journalTitle") { + risRecords.first().secondaryTitle shouldBeEqualTo journalTitle + } it("should have start page $startPage") { risRecords.first().startPage shouldBeEqualTo startPage } it("should have end page $endPage") { risRecords.first().endPage shouldBeEqualTo endPage } it("should have volume $volume") { risRecords.first().volumeNumber shouldBeEqualTo volume } @@ -71,14 +73,14 @@ object KRisProcessingSpec : DescribeSpec({ describe("with RIS file with other fields") { val type = "JOUR" - val number = 1999L + val miscellaneous1 = "1999" val abstract = "abstr line 1" val abstract2 = "abstr line 2" describe("with number and abstract") { val lines = listOf( "TY - $type", - "M1 - $number", + "M1 - $miscellaneous1", "AB - $abstract", "ER - " ) @@ -86,7 +88,9 @@ object KRisProcessingSpec : DescribeSpec({ it("should be parsed into one single RisRecord") { risRecords shouldHaveSize 1 } it("should have the reference type $type") { risRecords.first().type shouldBeEqualTo RisType.JOUR } - it("should have number $number") { risRecords.first().number shouldBeEqualTo number } + it("should have number $miscellaneous1") { + risRecords.first().miscellaneous1 shouldBeEqualTo miscellaneous1 + } it("should have abstract $abstract") { risRecords.first().abstr shouldBeEqualTo abstract } } diff --git a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisRecordSpec.kt b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisRecordSpec.kt index e8be7d59..30012f9a 100644 --- a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisRecordSpec.kt +++ b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisRecordSpec.kt @@ -72,7 +72,7 @@ object RisRecordSpec : DescribeSpec({ language = null, label = null, websiteLink = null, - number = null, + miscellaneous1 = null, miscellaneous2 = null, typeOfWork = null, notes = null, @@ -159,7 +159,7 @@ object RisRecordSpec : DescribeSpec({ language = "language", label = "label", websiteLink = "websiteLink", - number = 1234, + miscellaneous1 = "number", miscellaneous2 = "miscellaneous2", typeOfWork = "typeOfWork", notes = "notes", @@ -244,7 +244,7 @@ object RisRecordSpec : DescribeSpec({ .language("language") .label("label") .websiteLink("websiteLink") - .number(1234) + .miscellaneous1("number") .miscellaneous2("miscellaneous2") .typeOfWork("typeOfWork") .notes("notes") @@ -281,6 +281,28 @@ object RisRecordSpec : DescribeSpec({ assertSpecifiedValues(record) } } + + @Suppress("DEPRECATION") + describe("Deprecated fields") { + describe("with RisRecord constructed with new properties") { + val record = RisRecord(miscellaneous1 = "1234") + it("should return null as Number") { + record.number.shouldBeNull() + } + it("should return '1234' as miscellaneous1") { + record.miscellaneous1 shouldBeEqualTo "1234" + } + } + describe("with RisRecord constructed with deprecated properties") { + val record = RisRecord(number = 1234L) + it("should return 1234 as Number") { + record.number shouldBeEqualTo 1234L + } + it("should return null as miscellaneous1") { + record.miscellaneous1.shouldBeNull() + } + } + } }) @Suppress("LongMethod") @@ -322,7 +344,7 @@ private suspend fun DescribeSpecContainerScope.assertDefaultValues(record: RisRe "language" to record.language, "label" to record.label, "websiteLink" to record.websiteLink, - "number" to record.number, + "number" to record.miscellaneous1, "miscellaneous2" to record.miscellaneous2, "typeOfWork" to record.typeOfWork, "notes" to record.notes, @@ -418,7 +440,7 @@ private suspend fun DescribeSpecContainerScope.assertSpecifiedValues(record: Ris "language" to record.language, "label" to record.label, "websiteLink" to record.websiteLink, - "1234" to record.number, + "number" to record.miscellaneous1, "miscellaneous2" to record.miscellaneous2, "typeOfWork" to record.typeOfWork, "notes" to record.notes, diff --git a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisTagTest.kt b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisTagTest.kt index 62cd8e99..fe0a2a38 100644 --- a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisTagTest.kt +++ b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/domain/RisTagTest.kt @@ -1,5 +1,6 @@ package ch.difty.kris.domain +import io.kotest.matchers.collections.shouldContainAll import org.junit.jupiter.api.Test @Suppress("SpellCheckingInspection") @@ -8,7 +9,7 @@ internal class RisTagTest { @Test @Suppress("LongMethod") fun description() { - RisTag.values().map { it.description }.containsAll( + RisTag.values().map { it.description } shouldContainAll listOf( "Type of reference", "First Author", @@ -56,7 +57,7 @@ internal class RisTagTest { "Language", "Label", "Website Link", - "Number", + "Miscellaneous 1 (often Number)", "Miscellaneous 2.", "Type of Work", "Notes", @@ -74,8 +75,7 @@ internal class RisTagTest { "Start Page", "Short Title", "Primary Title", - "Secondary Title (journal title", - "if applicable)", + "Secondary Title (journal title, if applicable)", "Tertiary Title", "Translated Author", "Title", @@ -90,8 +90,7 @@ internal class RisTagTest { "Published Standard number", "Primary Date", "Access Date", - "End of Reference" + "End of Reference", ) - ) } } diff --git a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/usage/KRisUsageSpec.kt b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/usage/KRisUsageSpec.kt index 5dde7623..f25725b5 100644 --- a/subprojects/kris-core/src/test/kotlin/ch/difty/kris/usage/KRisUsageSpec.kt +++ b/subprojects/kris-core/src/test/kotlin/ch/difty/kris/usage/KRisUsageSpec.kt @@ -12,6 +12,7 @@ import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.toList import kotlinx.coroutines.runBlocking import org.amshove.kluent.shouldBeEqualTo +import org.amshove.kluent.shouldBeNull import org.amshove.kluent.shouldHaveSize /** @@ -147,6 +148,7 @@ object KRisUsageSpec : DescribeSpec({ } it("can get list of all RisTag names") { + @Suppress("MaxLineLength") risTagNames.joinToString() shouldBeEqualTo "TY, A1, A2, A3, A4, AB, AD, AN, AU, AV, BT, C1, C2, C3, C4, C5, C6, C7, C8, CA, CN, CP, CT, CY, DA, DB, DO, " + "DP, ED, EP, ET, ID, IS, J1, J2, JA, JF, JO, KW, L1, L2, L3, L4, LA, LB, LK, M1, M2, M3, N1, N2, NV, OP, PB, " + @@ -157,4 +159,64 @@ object KRisUsageSpec : DescribeSpec({ KRis.risTagNames() shouldBeEqualTo risTagNames } } + + @Suppress("DEPRECATION") + describe("deprecated RisRecord Properties") { + describe("importing from RIS") { + describe("given M1 with numeric value") { + val risLines: List = listOf( + "M1 - 1234", + "ER - " + ) + it("can be processed") { + val risRecords = risLines.toRisRecords() + risRecords.shouldHaveSize(1) + } + it("can retrieve it with new property miscellaneous1") { + val risRecords = risLines.toRisRecords() + risRecords.first().miscellaneous1 shouldBeEqualTo "1234" + } + it("can retrieve it with deprecated property number") { + val risRecords = risLines.toRisRecords() + risRecords.first().number shouldBeEqualTo 1234L + } + } + describe("given M1 with non-numeric value") { + val risLines: List = listOf( + "M1 - 1234-5678", + "ER - " + ) + it("can be processed") { + val risRecords = risLines.toRisRecords() + risRecords.shouldHaveSize(1) + } + it("can retrieve it with new property miscellaneous1") { + val risRecords = risLines.toRisRecords() + risRecords.first().miscellaneous1 shouldBeEqualTo "1234-5678" + } + it("can retrieve null with deprecated property number") { + val risRecords = risLines.toRisRecords() + risRecords.first().number.shouldBeNull() + } + } + } + describe("exporting to RIS") { + describe("using new properties miscellaneous1") { + val risRecord1 = RisRecord( + miscellaneous1 = "1234-5678", + ) + val risRecord2 = RisRecord( + number = 4567L, + ) + it("should export to M1") { + listOf(risRecord1, risRecord2).toRisLines().joinToString(separator = "") shouldBeEqualTo """M1 - 1234-5678 + |ER - + | + |M1 - 4567 + |ER - + |""".trimMargin() + } + } + } + } })