Added check that there is not clash with parent upload page

Fixes #142

cli/src/main/kotlin/com/github/zeldigas/text2confl/cli/Upload.kt

@@ -11,6 +11,7 @@ import com.github.ajalt.clikt.parameters.types.enum
 import com.github.zeldigas.confclient.ConfluenceClient
 import com.github.zeldigas.confclient.ConfluenceClientConfig
 import com.github.zeldigas.confclient.PasswordAuth
+import com.github.zeldigas.confclient.model.ConfluencePage
 import com.github.zeldigas.text2confl.convert.EditorVersion
 import com.github.zeldigas.text2confl.core.ServiceProvider
 import com.github.zeldigas.text2confl.core.config.*
@@ -82,18 +83,20 @@ class Upload : CliktCommand(name = "upload", help = "Converts source files and u
         val clientConfig = createClientConfig(directoryStoredParams)
         val conversionConfig = createConversionConfig(directoryStoredParams, editorVersion, clientConfig.server)
         val converter = serviceProvider.createConverter(uploadConfig.space, conversionConfig)
-        val result = if (docs.isFile) {
+        val pagesToPublish = if (docs.isFile) {
             listOf(converter.convertFile(docs.toPath()))
         } else {
             converter.convertDir(docs.toPath())
         }
-        serviceProvider.createContentValidator().validate(result)
+        val contentValidator = serviceProvider.createContentValidator()
+        contentValidator.validate(pagesToPublish)
         val confluenceClient = serviceProvider.createConfluenceClient(clientConfig, dryRun)
         val publishUnder = resolveParent(confluenceClient, uploadConfig, directoryStoredParams)
 
         val contentUploader = serviceProvider.createUploader(confluenceClient, uploadConfig, conversionConfig, operationsTracker(clientConfig.server))
         withContext(Dispatchers.Default) {
-            contentUploader.uploadPages(pages = result, uploadConfig.space, publishUnder)
+            contentValidator.checkNoClashWithParent(publishUnder, pagesToPublish)
+            contentUploader.uploadPages(pages = pagesToPublish, uploadConfig.space, publishUnder.id)
         }
     }
 
@@ -140,12 +143,12 @@ class Upload : CliktCommand(name = "upload", help = "Converts source files and u
         confluenceClient: ConfluenceClient,
         uploadConfig: UploadConfig,
         directoryConfig: DirectoryConfig
-    ): String {
+    ): ConfluencePage {
         val anyParentId = listOf(parentId, directoryConfig.defaultParentId).firstOrNull { it != null }
-        if (anyParentId != null) return anyParentId
+        if (anyParentId != null) return confluenceClient.getPageById(anyParentId, emptySet())
         val anyTitle = listOf(parentTitle, directoryConfig.defaultParent).firstOrNull { it != null }
-        if (anyTitle != null) return confluenceClient.getPage(uploadConfig.space, anyTitle).id
+        if (anyTitle != null) return confluenceClient.getPage(uploadConfig.space, anyTitle)
 
-        return confluenceClient.describeSpace(uploadConfig.space, listOf("homepage")).homepage?.id!!
+        return confluenceClient.describeSpace(uploadConfig.space, listOf("homepage")).homepage!!
     }
 }

cli/src/test/kotlin/com/github/zeldigas/text2confl/cli/UploadTest.kt

@@ -65,6 +65,11 @@ internal class UploadTest(
         every { converter.convertDir(tempDir) } returns result
         coEvery { contentUploader.uploadPages(result, "TR", "1234") } just Runs
         every { contentValidator.validate(result) } just Runs
+        coEvery { confluenceClient.getPageById("1234", emptySet()) } returns mockk {
+            every { title } returns "parent page"
+            every { id } returns "1234"
+        }
+        every { contentValidator.checkNoClashWithParent(any(), any()) } just Runs
 
         command.parse(
             listOf(
@@ -116,6 +121,11 @@ internal class UploadTest(
         every { converter.convertDir(tempDir) } returns result
         coEvery { contentUploader.uploadPages(result, "TR", "1234") } just Runs
         every { contentValidator.validate(result) } just Runs
+        coEvery { confluenceClient.getPageById("1234", emptySet()) } returns mockk {
+            every { title } returns "parent page"
+            every { id } returns "1234"
+        }
+        every { contentValidator.checkNoClashWithParent(any(), any()) } just Runs
 
         command.parse(
             listOf(
@@ -210,9 +220,13 @@ internal class UploadTest(
     internal fun `Resolution of page by title`(@TempDir tempDir: Path) {
         val result = mockk<List<Page>>()
         every { converter.convertDir(tempDir) } returns result
-        coEvery { confluenceClient.getPage("TR", "Test page").id } returns "1234"
+        coEvery { confluenceClient.getPage("TR", "Test page") } returns mockk {
+            every { id } returns "1234"
+            every { title } returns "Test page"
+        }
         coEvery { contentUploader.uploadPages(result, "TR", "1234") } just Runs
         every { contentValidator.validate(result) } just Runs
+        every { contentValidator.checkNoClashWithParent(any(), result) } just Runs
         command.parse(
             listOf(
                 "--confluence-url", "https://test.atlassian.net/wiki",
@@ -232,9 +246,13 @@ internal class UploadTest(
     internal fun `Using home page if not specified`(@TempDir tempDir: Path) {
         val result = mockk<List<Page>>()
         every { converter.convertDir(tempDir) } returns result
-        coEvery { confluenceClient.describeSpace("TR", listOf("homepage")).homepage?.id } returns "1234"
+        coEvery { confluenceClient.describeSpace("TR", listOf("homepage")).homepage } returns mockk {
+            every { id } returns "1234"
+            every { title } returns "home page"
+        }
         coEvery { contentUploader.uploadPages(result, "TR", "1234") } just Runs
         every { contentValidator.validate(result) } just Runs
+        every { contentValidator.checkNoClashWithParent(any(), result) } just Runs
         command.parse(
             listOf(
                 "--confluence-url", "https://test.atlassian.net/wiki",

core/src/main/kotlin/com/github/zeldigas/text2confl/core/ContentValidator.kt

@@ -1,12 +1,15 @@
 package com.github.zeldigas.text2confl.core
 
+import com.github.zeldigas.confclient.model.ConfluencePage
 import com.github.zeldigas.text2confl.convert.Page
 import com.github.zeldigas.text2confl.convert.Validation
+import com.github.zeldigas.text2confl.core.upload.ContentUploadException
 
 class ContentValidationFailedException(val errors: List<String>) : RuntimeException()
 
 interface ContentValidator {
     fun validate(content: List<Page>)
+    fun checkNoClashWithParent(publishUnder: ConfluencePage, pagesToPublish: List<Page>)
 }
 
 class ContentValidatorImpl : ContentValidator {
@@ -27,4 +30,10 @@ class ContentValidatorImpl : ContentValidator {
             collectErrors(page.children, foundIssues)
         }
     }
+
+    override fun checkNoClashWithParent(publishUnder: ConfluencePage, pagesToPublish: List<Page>) {
+        val conflictWithParent = pagesToPublish.find { it.title == publishUnder.title } ?: return
+
+        throw ContentUploadException("Page to publish clashes with parent under which pages will be published. Problem file - ${conflictWithParent.source}, parent confluence page - ${publishUnder.title} (id=${publishUnder.id})")
+    }
 }

core/src/main/kotlin/com/github/zeldigas/text2confl/core/upload/PageUploadOperations.kt

@@ -67,6 +67,9 @@ abstract class PageOperationException(message: String, cause: Exception? = null)
 data class PageNotFoundException(val space: String, val title: String) :
     PageOperationException("Page $title in space $space not found")
 
+data class PageCycleException(val parentId: String, val title: String) :
+    PageOperationException("Can't publish page $title with parent $parentId - this is id of page itself")
+
 enum class ChangeDetector(
     val extraData: Set<String>,
     val strategy: (serverPage: ConfluencePage, content: PageContent) -> Boolean

core/src/main/kotlin/com/github/zeldigas/text2confl/core/upload/PageUploadOperationsImpl.kt

@@ -62,6 +62,7 @@ internal class PageUploadOperationsImpl(
         page: Page,
         parentPageId: String
     ): PageOperationResult {
+        checkNoCycle(parentPageId, confluencePageToUpdate)
         checkTenantBeforeUpdate(confluencePageToUpdate)
         val (renamed, confluencePage) = adjustTitleIfRequired(confluencePageToUpdate, page)
 
@@ -143,6 +144,12 @@ internal class PageUploadOperationsImpl(
         )
     }
 
+    private fun checkNoCycle(parentPageId: String, pageToUpdate: ConfluencePage) {
+        if (pageToUpdate.id == parentPageId) {
+            throw PageCycleException(parentPageId, pageToUpdate.title)
+        }
+    }
+
     private fun checkTenantBeforeUpdate(serverPage: ConfluencePage) {
         val pageTenant = serverPage.pageProperty(TENANT_PROPERTY)?.value ?: return
 

core/src/test/kotlin/com/github/zeldigas/text2confl/core/ContentValidatorImplTest.kt

@@ -1,6 +1,7 @@
 package com.github.zeldigas.text2confl.core
 
 import assertk.assertFailure
+import assertk.assertions.hasMessage
 import assertk.assertions.isEqualTo
 import assertk.assertions.isInstanceOf
 import com.github.zeldigas.text2confl.convert.Validation
@@ -9,6 +10,7 @@ import io.mockk.mockk
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertDoesNotThrow
 import java.nio.file.Path
+import kotlin.io.path.Path
 
 internal class ContentValidatorImplTest {
 
@@ -51,4 +53,29 @@ internal class ContentValidatorImplTest {
         }.isInstanceOf(ContentValidationFailedException::class)
             .transform { it.errors }.isEqualTo(listOf("${Path.of("a", "b.txt")}: err1", "c.txt: err2"))
     }
+
+    @Test
+    fun `Clash with parent page is detected`() {
+        val pagePath = Path("test.md")
+        assertFailure {
+            validator.checkNoClashWithParent(mockk {
+                every { title } returns "parent"
+                every { id } returns "1234"
+            }, listOf(mockk {
+                every { title } returns "parent"
+                every { source } returns pagePath
+            }))
+        }.hasMessage("Page to publish clashes with parent under which pages will be published. Problem file - ${pagePath}, parent confluence page - parent (id=1234)")
+    }
+
+    @Test
+    fun `No clash with parent page does nothing`() {
+        assertDoesNotThrow {
+            validator.checkNoClashWithParent(mockk {
+                every { title } returns "parent"
+            }, listOf(mockk {
+                every { title } returns "another title"
+            }))
+        }
+    }
 }

core/src/test/kotlin/com/github/zeldigas/text2confl/core/upload/PageUploadOperationsImplTest.kt

@@ -366,7 +366,7 @@ internal class PageUploadOperationsImplTest(
             every { title } returns "Page title"
         }
         if (parentChanged) {
-            coEvery { client.changeParent(any(), any(), 44, targetParent, any())} returns mockk()
+            coEvery { client.changeParent(any(), any(), 44, targetParent, any()) } returns mockk()
         }
         coEvery { client.setPageProperty(any(), any(), any()) } just Runs
         coEvery { client.fetchAllAttachments(any()) } returns listOf(serverAttachment("one", "HASH:123"))
@@ -471,7 +471,12 @@ internal class PageUploadOperationsImplTest(
             )
         }
 
-        assertThat(result).isEqualTo(LabelsUpdateResult.Updated(added = listOf("four", "five"), removed = listOf("three")))
+        assertThat(result).isEqualTo(
+            LabelsUpdateResult.Updated(
+                added = listOf("four", "five"),
+                removed = listOf("three")
+            )
+        )
 
         coVerify { client.addLabels(PAGE_ID, listOf("four", "five")) }
         coVerify { client.deleteLabel(PAGE_ID, "three") }
@@ -494,7 +499,12 @@ internal class PageUploadOperationsImplTest(
             )
         }
 
-        assertThat(result).isEqualTo(LabelsUpdateResult.Updated(added = listOf("one", "two", "four", "five"), removed = emptyList()))
+        assertThat(result).isEqualTo(
+            LabelsUpdateResult.Updated(
+                added = listOf("one", "two", "four", "five"),
+                removed = emptyList()
+            )
+        )
 
         coVerify { client.addLabels(PAGE_ID, listOf("one", "two", "four", "five")) }
     }
@@ -558,13 +568,15 @@ internal class PageUploadOperationsImplTest(
             )
         }
 
-        assertThat(result).isEqualTo(AttachmentsUpdateResult.Updated(
-            added = listOf(localAttachments[3], localAttachments[4]),
-            modified = listOf(localAttachments[0], localAttachments[1]),
-            removed = listOf(
-                serverAttachments[3]
+        assertThat(result).isEqualTo(
+            AttachmentsUpdateResult.Updated(
+                added = listOf(localAttachments[3], localAttachments[4]),
+                modified = listOf(localAttachments[0], localAttachments[1]),
+                removed = listOf(
+                    serverAttachments[3]
+                )
             )
-        ))
+        )
 
         coVerifyAll {
             client.deleteAttachment("id_four")
@@ -778,4 +790,24 @@ internal class PageUploadOperationsImplTest(
         }.isInstanceOf(PageNotFoundException::class)
             .isEqualTo(PageNotFoundException(space = "TEST", title = "page title"))
     }
+
+    @Test
+    fun `Cycle with parent is detected for found page`() {
+        coEvery {
+            client.getPageOrNull(any(), any(), expansions = any())
+        } returns mockk {
+            every { id } returns "parentId"
+            every { title } returns "Page title"
+        }
+
+        assertFailure {
+            runBlocking {
+                uploadOperations().createOrUpdatePageContent(mockk() {
+                    every { title } returns "Page title"
+                    every { properties } returns emptyMap()
+                }, "TEST", "parentId")
+            }
+        }.isInstanceOf(PageCycleException::class)
+            .isEqualTo(PageCycleException("parentId", "Page title"))
+    }
 }