diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index d8ee52b0a68..a6b6bde9ebb 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -108,3 +108,8 @@ jobs: if: always() run: | bazel run //scripts:regex_pattern_validation_check -- $(pwd) + + - name: XML Syntax Validation Check + if: always() + run: | + bazel run //scripts:xml_syntax_check -- $(pwd) diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index 4f92eb6711b..311d03326f5 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -67,3 +67,10 @@ kt_jvm_binary( "//scripts/src/java/org/oppia/android/scripts/regex:regex_pattern_validation_check_lib", ], ) + +kt_jvm_binary( + name = "xml_syntax_check", + testonly = True, + main_class = "org.oppia.android.scripts.xml.XmlSyntaxCheckKt", + runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/xml:xml_syntax_check_lib"], +) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel new file mode 100644 index 00000000000..56dc1789827 --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel @@ -0,0 +1,24 @@ +""" +Libraries corresponding to XML syntax based check to ensure that all the XML files in the codebase +are syntactically correct. +""" + +load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library") + +kt_jvm_library( + name = "xml_syntax_error_handler", + testonly = True, + srcs = ["XmlSyntaxErrorHandler.kt"], + visibility = ["//scripts:oppia_script_test_visibility"], +) + +kt_jvm_library( + name = "xml_syntax_check_lib", + testonly = True, + srcs = ["XmlSyntaxCheck.kt"], + visibility = ["//scripts:oppia_script_binary_visibility"], + deps = [ + ":xml_syntax_error_handler", + "//scripts/src/java/org/oppia/android/scripts/common:repository_file", + ], +) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/XmlSyntaxCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/XmlSyntaxCheck.kt new file mode 100644 index 00000000000..55796bf1fec --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/xml/XmlSyntaxCheck.kt @@ -0,0 +1,88 @@ +package org.oppia.android.scripts.xml + +import org.oppia.android.scripts.common.RepositoryFile +import org.xml.sax.SAXParseException +import java.io.File +import javax.xml.parsers.DocumentBuilder +import javax.xml.parsers.DocumentBuilderFactory + +/** + * Script for ensuring that all XML files in the repo are syntactically correct. + * + * Usage: + * bazel run //scripts:xml_syntax_check -- + * + * Arguments: + * - path_to_directory_root: directory path to the root of the Oppia Android repository. + * + * Example: + * bazel run //scripts:xml_syntax_check -- $(pwd) + */ +fun main(vararg args: String) { + // Path of the repo to be analyzed. + val repoPath = "${args[0]}/" + + // A list of all XML files in the repo to be analyzed. + val searchFiles = RepositoryFile.collectSearchFiles( + repoPath = repoPath, + expectedExtension = ".xml" + ) + + // Builder factory which provides the builder to parse the XMl. + val builderFactory = DocumentBuilderFactory.newInstance() + + val allErrorsList = searchFiles.flatMap { file -> + val docBuilder = builderFactory.newDocumentBuilder() + val xmlSyntaxErrorHandler = XmlSyntaxErrorHandler() + docBuilder.setErrorHandler(xmlSyntaxErrorHandler) + parseXml(docBuilder, file) + val fileErrorList = xmlSyntaxErrorHandler.retrieveErrorList() + fileErrorList.map { error -> Pair(error, file) } + } + + // Check if the repo has any syntactically incorrect XML. + val hasXmlSyntaxFailure = allErrorsList.isNotEmpty() + + logXmlSyntaxFailures(allErrorsList) + + if (hasXmlSyntaxFailure) { + throw Exception("XML SYNTAX CHECK FAILED") + } else { + println("XML SYNTAX CHECK PASSED") + } +} + +/** + * Parses a given XML file. + * + * @param docBuilder the builder which will parse the XML file + * @param file the file to be checked for + */ +private fun parseXml(docBuilder: DocumentBuilder, file: File) { + try { + docBuilder.parse(file) + } catch (e: SAXParseException) { + // For any syntax error in the XML file, if the custom error handler does not throws any + // exception then the default error handler throws a [SaxParseException]. In order to prevent + // the script check from getting terminated in between, we need to catch and ignore the + // exception. + } +} + +/** + * Logs the failures for XML syntax validation. + * + * @param errorList a list of all the errors collected by the error handler + */ +private fun logXmlSyntaxFailures(errorList: List>) { + if (errorList.isNotEmpty()) { + errorList.forEach { errorPair -> + val error = errorPair.first + val errorFile = errorPair.second + val failureMessage = + "$errorFile:${error.getLineNumber()}:${error.getColumnNumber()}: ${error.message}" + println(failureMessage) + } + println() + } +} diff --git a/scripts/src/java/org/oppia/android/scripts/xml/XmlSyntaxErrorHandler.kt b/scripts/src/java/org/oppia/android/scripts/xml/XmlSyntaxErrorHandler.kt new file mode 100644 index 00000000000..94ceaaa27e3 --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/xml/XmlSyntaxErrorHandler.kt @@ -0,0 +1,32 @@ +package org.oppia.android.scripts.xml + +import org.xml.sax.ErrorHandler +import org.xml.sax.SAXParseException + +/** + * Custom XML parser error handler which collects syntax errors as they occur for later processing. + */ +class XmlSyntaxErrorHandler : ErrorHandler { + private val syntaxErrorList = mutableListOf() + + override fun warning(e: SAXParseException) { + syntaxErrorList.add(e) + } + + override fun error(e: SAXParseException) { + syntaxErrorList.add(e) + } + + override fun fatalError(e: SAXParseException) { + syntaxErrorList.add(e) + } + + /** + * Retrieves all the errors collected by the handler. + * + * @return a list of all the errors collected by the error handler + */ + fun retrieveErrorList(): List { + return syntaxErrorList + } +} diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel new file mode 100644 index 00000000000..445e7b6d76d --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel @@ -0,0 +1,27 @@ +""" +Tests corresponding to XML syntax based check to ensure that all the XML files in the codebase are +syntactically correct. +""" + +load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_test") + +kt_jvm_test( + name = "XmlSyntaxCheckTest", + srcs = ["XmlSyntaxCheckTest.kt"], + deps = [ + "//scripts/src/java/org/oppia/android/scripts/xml:xml_syntax_check_lib", + "//testing:assertion_helpers", + "//third_party:com_google_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +) + +kt_jvm_test( + name = "XmlSyntaxErrorHandlerTest", + srcs = ["XmlSyntaxErrorHandlerTest.kt"], + deps = [ + "//scripts/src/java/org/oppia/android/scripts/xml:xml_syntax_error_handler", + "//third_party:com_google_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxCheckTest.kt new file mode 100644 index 00000000000..4babfc5c806 --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxCheckTest.kt @@ -0,0 +1,120 @@ +package org.oppia.android.scripts.xml + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.oppia.android.testing.assertThrows +import java.io.ByteArrayOutputStream +import java.io.PrintStream + +/** Tests for [XmlSyntaxCheck]. */ +class XmlSyntaxCheckTest { + private val outContent: ByteArrayOutputStream = ByteArrayOutputStream() + private val originalOut: PrintStream = System.out + private val XML_SYNTAX_CHECK_PASSED_OUTPUT_INDICATOR: String = "XML SYNTAX CHECK PASSED" + private val XML_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR: String = "XML SYNTAX CHECK FAILED" + + @Rule + @JvmField + var tempFolder = TemporaryFolder() + + @Before + fun setUp() { + tempFolder.newFolder("testfiles") + System.setOut(PrintStream(outContent)) + } + + @After + fun restoreStreams() { + System.setOut(originalOut) + } + + @Test + fun testXmlSyntax_validXml_xmlSyntaxIsCorrect() { + val validXml = + """ + + + + + + """.trimIndent() + + val tempFile = tempFolder.newFile("testfiles/TestFile.xml") + tempFile.writeText(validXml) + + runScript() + + assertThat(outContent.toString().trim()).isEqualTo(XML_SYNTAX_CHECK_PASSED_OUTPUT_INDICATOR) + } + + @Test + fun testXmlSyntax_invalidOpeningTag_xmlSyntaxIsIncorrect() { + val invalidXml = + """ + + + + + + """.trimIndent() + val tempFile = tempFolder.newFile("testfiles/TestFile.xml") + tempFile.writeText(invalidXml) + + val exception = assertThrows(Exception::class) { + runScript() + } + + assertThat(exception).hasMessageThat().contains(XML_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + assertThat(outContent.toString()).contains("${retrieveTestFilesDirectoryPath()}/TestFile.xml") + } + + @Test + fun testXmlSyntax_multipleFilesHavingInvalidXml_xmlSyntaxIsIncorrect() { + val invalidXmlForFile1 = + """ + + + < + + + """.trimIndent() + val invalidXmlForFile2 = + """ + + + + + + """.trimIndent() + val tempFile1 = tempFolder.newFile("testfiles/TestFile1.xml") + val tempFile2 = tempFolder.newFile("testfiles/TestFile2.xml") + tempFile1.writeText(invalidXmlForFile1) + tempFile2.writeText(invalidXmlForFile2) + + val exception = assertThrows(Exception::class) { + runScript() + } + + assertThat(exception).hasMessageThat().contains(XML_SYNTAX_CHECK_FAILED_OUTPUT_INDICATOR) + assertThat(outContent.toString()).contains("${retrieveTestFilesDirectoryPath()}/TestFile2.xml") + assertThat(outContent.toString()).contains("${retrieveTestFilesDirectoryPath()}/TestFile1.xml") + } + + /** Retrieves the absolute path of testfiles directory. */ + private fun retrieveTestFilesDirectoryPath(): String { + return "${tempFolder.root}/testfiles" + } + + /** Runs the xml_syntax_check. */ + private fun runScript() { + main(retrieveTestFilesDirectoryPath()) + } +} diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxErrorHandlerTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxErrorHandlerTest.kt new file mode 100644 index 00000000000..ec3e896517d --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/XmlSyntaxErrorHandlerTest.kt @@ -0,0 +1,161 @@ +package org.oppia.android.scripts.xml + +import com.google.common.truth.Truth.assertThat +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.xml.sax.SAXParseException +import java.io.File +import javax.xml.parsers.DocumentBuilder +import javax.xml.parsers.DocumentBuilderFactory + +/** Tests for [XmlSyntaxErrorHandler]. */ +class XmlSyntaxErrorHandlerTest { + private val builderFactory = DocumentBuilderFactory.newInstance() + + @Rule + @JvmField + var tempFolder = TemporaryFolder() + + @Before + fun setUp() { + tempFolder.newFolder("testfiles") + } + + @Test + fun testXmlErrorHandler_validXml_noErrorShouldBeCollected() { + val validXml = + """ + + + + + + """.trimIndent() + val tempFile = tempFolder.newFile("testfiles/TestFile.xml") + tempFile.writeText(validXml) + val docBuilder = builderFactory.newDocumentBuilder() + val xmlSyntaxErrorHandler = XmlSyntaxErrorHandler() + docBuilder.setErrorHandler(xmlSyntaxErrorHandler) + + parseXml(docBuilder = docBuilder, file = tempFile) + + val errorList = xmlSyntaxErrorHandler.retrieveErrorList() + assertThat(errorList).isEmpty() + } + + @Test + fun testXmlErrorHandler_invalidXml_errorShouldBeCollected() { + val invalidXml = + """ + + + + + > + """.trimIndent() + val tempFile = tempFolder.newFile("testfiles/TestFile.xml") + tempFile.writeText(invalidXml) + val docBuilder = builderFactory.newDocumentBuilder() + val xmlSyntaxErrorHandler = XmlSyntaxErrorHandler() + docBuilder.setErrorHandler(xmlSyntaxErrorHandler) + + parseXml(docBuilder = docBuilder, file = tempFile) + + val errorList = xmlSyntaxErrorHandler.retrieveErrorList() + assertThat(errorList).hasSize(1) + assertThat(errorList.first().message).isEqualTo( + "Content is not allowed in trailing section." + ) + assertThat(errorList.first().getLineNumber()).isEqualTo(6) + assertThat(errorList.first().getColumnNumber()).isEqualTo(9) + } + + @Test + fun testXmlErrorHandler_invokeWarningCase_errorShouldBeCollected() { + val xmlSyntaxErrorHandler = XmlSyntaxErrorHandler() + xmlSyntaxErrorHandler.warning(SAXParseException("test_error_message", "", "", 1, 1)) + val errorList = xmlSyntaxErrorHandler.retrieveErrorList() + assertThat(errorList).hasSize(1) + assertThat(errorList.first().message).isEqualTo("test_error_message") + assertThat(errorList.first().getLineNumber()).isEqualTo(1) + assertThat(errorList.first().getColumnNumber()).isEqualTo(1) + } + + @Test + fun testXmlErrorHandler_invokeErrorCase_errorShouldBeCollected() { + val xmlSyntaxErrorHandler = XmlSyntaxErrorHandler() + xmlSyntaxErrorHandler.error(SAXParseException("test_error_message", "", "", 1, 1)) + val errorList = xmlSyntaxErrorHandler.retrieveErrorList() + assertThat(errorList).hasSize(1) + assertThat(errorList.first().message).isEqualTo("test_error_message") + assertThat(errorList.first().getLineNumber()).isEqualTo(1) + assertThat(errorList.first().getColumnNumber()).isEqualTo(1) + } + + @Test + fun testXmlErrorHandler_invokeFatalErrorCase_errorShouldBeCollected() { + val xmlSyntaxErrorHandler = XmlSyntaxErrorHandler() + xmlSyntaxErrorHandler.fatalError(SAXParseException("test_error_message", "", "", 2, 2)) + val errorList = xmlSyntaxErrorHandler.retrieveErrorList() + assertThat(errorList).hasSize(1) + assertThat(errorList.first().message).isEqualTo("test_error_message") + assertThat(errorList.first().getLineNumber()).isEqualTo(2) + assertThat(errorList.first().getColumnNumber()).isEqualTo(2) + } + + @Test + fun testXmlErrorHandler_multipleErrors_allErrorsShouldBeCollected() { + val invalidXml = + """ + + + + + > + """.trimIndent() + val tempFile = tempFolder.newFile("testfiles/TestFile.xml") + tempFile.writeText(invalidXml) + val docBuilder = builderFactory.newDocumentBuilder() + val xmlSyntaxErrorHandler = XmlSyntaxErrorHandler() + docBuilder.setErrorHandler(xmlSyntaxErrorHandler) + xmlSyntaxErrorHandler.warning(SAXParseException("error1", "", "", 1, 1)) + xmlSyntaxErrorHandler.error(SAXParseException("error2", "", "", 2, 2)) + parseXml(docBuilder = docBuilder, file = tempFile) + + val errorList = xmlSyntaxErrorHandler.retrieveErrorList() + assertThat(errorList).hasSize(3) + assertThat(errorList.elementAt(0).message).isEqualTo("error1") + assertThat(errorList.elementAt(0).getLineNumber()).isEqualTo(1) + assertThat(errorList.elementAt(0).getColumnNumber()).isEqualTo(1) + assertThat(errorList.elementAt(1).message).isEqualTo("error2") + assertThat(errorList.elementAt(1).getLineNumber()).isEqualTo(2) + assertThat(errorList.elementAt(1).getColumnNumber()).isEqualTo(2) + assertThat(errorList.elementAt(2).message).isEqualTo( + "Content is not allowed in trailing section." + ) + assertThat(errorList.elementAt(2).getLineNumber()).isEqualTo(6) + assertThat(errorList.elementAt(2).getColumnNumber()).isEqualTo(9) + } + + /** + * Parses a given XML file. + * + * @param docBuilder the builder which will parse the XML file + * @param file the file to be checked for + */ + private fun parseXml(docBuilder: DocumentBuilder, file: File) { + try { + docBuilder.parse(file) + } catch (e: SAXParseException) { + // For any syntax error in the XML file, if the custom error handler does not throws any + // exception then the default error handler throws a [SaxParseException]. In order to prevent + // the script check from getting terminated in between, we need to catch and ignore the + // exception. + } + } +}