feat: [#129] Change M1 implementation from RisRecord.number to RisRec…

…ord.miscellaneous1 using String

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 -- [email protected]
- * @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,

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. */

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);
+    }
 }

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

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,22 +73,24 @@ 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  - "
             )
             val risRecords = runBlocking { KRis.process(lines.asFlow()).toList() }
 
             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 }
         }
 

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,

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",
             )
-        )
     }
 }

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<String> = 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<String> = 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()
+                }
+            }
+        }
+    }
 })