Skip to content

Commit

Permalink
Fix #3291: Add check for XML syntax validation (#3341)
Browse files Browse the repository at this point in the history
* Fix #3290: Add support for generic regex pattern matching

* Apply review suggestions for proto location

* Delete main directory

* Implment script logic and tests

* CI setup part 1

* syntax fix in yaml file

* import nits in dummy file

* Use regex patterns from script in script test

* Make PR suggestions into effect

* Make PR suggestions into effect

* Make PR suggestions into effect

* Improve naming of script_assets variables

* Make PR suggestions into effect

* Make PR suggestions

* Make PR suggestions

* Revamp testing approach

* Fix #3291: Add check for XML syntax correctness

* nit fixes

* Introduce XML syntax check in static checks workflow

* Make nit suggestions

* Make nit suggestions

* Add if: always()

* Apply review suggestions on PR

* Make review suggestions from regex pattern checks

* Refactor PR as per feedback recieved

* Implement feedback suggestions

* Implement review suggestions

* Implement review suggestions

* nits

* Nit fixes

* Nit fixes

* nit fix

* nit fix

* Review suggestions part 1

* Implement review suggestions part 2

* nit fixes

* update static_checks

* add test to repository file

* nit fix

* nit fix

* nit fix

* Do review suggestions

* Implement review suggestions

* Implement review suggestions

* nit fix

* bazel files nit fix

* Introduce a library for the regex assets

* Make directory structure same as that of #3374

* Make directory structure similar to #3374

* diable ktlint max-line-length

* disable ktlint-max-line

* disable ktlint max-length

* remove testonly attribute from tests

* nit fix

* nit fix

* Apply review suggestions

* ktlint fix

* nit fix

* Apply nit fixes

* Remove script constants

* nit fix

* add todo

* nit fixes

* add test

* add testOnly

* nit fix
  • Loading branch information
Sparsh1212 authored Jul 8, 2021
1 parent 72bd749 commit 128b386
Show file tree
Hide file tree
Showing 8 changed files with 464 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
7 changes: 7 additions & 0 deletions scripts/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)
24 changes: 24 additions & 0 deletions scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
],
)
88 changes: 88 additions & 0 deletions scripts/src/java/org/oppia/android/scripts/xml/XmlSyntaxCheck.kt
Original file line number Diff line number Diff line change
@@ -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 -- <path_to_directory_root>
*
* 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<Pair<SAXParseException, File>>) {
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()
}
}
Original file line number Diff line number Diff line change
@@ -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<SAXParseException>()

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<SAXParseException> {
return syntaxErrorList
}
}
27 changes: 27 additions & 0 deletions scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
],
)
Original file line number Diff line number Diff line change
@@ -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 =
"""
<?xml version="1.0" encoding="utf-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
<solid android:color="#3333334D" />
<size android:height="1dp" />
</shape>
""".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 =
"""
<?xml version="1.0" encoding="utf-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
<solid android:color="#3333334D" />
<size android:height="1dp" />
</shapes>
""".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 =
"""
<?xml version="1.0" encoding="utf-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
<<solid android:color="#3333334D" />
<size android:height="1dp" />
</shape>
""".trimIndent()
val invalidXmlForFile2 =
"""
<?xml version="1.0" encoding="utf-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
<solid android:color="#3333334D" />
<size android:height="1dp" />
</shapes>
""".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())
}
}
Loading

0 comments on commit 128b386

Please sign in to comment.