Skip to content

Commit

Permalink
[#366] Fix NullPointerException when accessing unfilled displayName i…
Browse files Browse the repository at this point in the history
…n StandaloneConfig (#368)

In our user guide, we allow most of the fields under the authors'
configuration to be optional other than the GitHub Id. Particularly
`Display Name`, which is suppose to use GitHub Id value when it is not
provided. However, when the display name is not filled in the
standalone config, the program crashes due to a NullPointerException.

This is because standalone config adopts the json format. When a field
is not filled in, in the json file, it will be initialized as null when
deserialized into a java object. This was unforeseen and was left
unchecked when accessing the variable.

Let's fix this by returning an empty string as the default value of
display name in the event of a null case, and also include unit tests
for StandaloneConfig parser.
  • Loading branch information
yong24s authored and eugenepeh committed Oct 13, 2018
1 parent 8229759 commit 0337d36
Show file tree
Hide file tree
Showing 11 changed files with 194 additions and 6 deletions.
8 changes: 8 additions & 0 deletions src/main/java/reposense/model/Author.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ public Author(StandaloneAuthor sa) {
updateIgnoreGlobMatcher();
}

public Author(Author another) {
this.gitId = another.gitId;
this.displayName = another.gitId;
this.authorAliases = another.authorAliases;
this.ignoreGlobList = another.authorAliases;
this.ignoreGlobMatcher = another.ignoreGlobMatcher;
}

/**
* Checks that all the strings in the {@code ignoreGlobList} only contains commonly used glob patterns.
* @throws IllegalArgumentException if any of the values do not meet the criteria.
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/reposense/model/StandaloneAuthor.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ public String getGithubId() {
}

public String getDisplayName() {
if (displayName == null) {
return "";
}

return displayName;
}

Expand Down
20 changes: 14 additions & 6 deletions src/test/java/reposense/parser/RepoConfigurationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ public class RepoConfigurationTest {

private static final List<String> REPO_LEVEL_GLOB_LIST = Arrays.asList("collated**");
private static final List<String> FIRST_AUTHOR_GLOB_LIST =
Arrays.asList("collated**", "*.aa1", "**.aa2", "**.java");
private static final List<String> SECOND_AUTHOR_GLOB_LIST = Arrays.asList("collated**", "**[!(.md)]");
Arrays.asList("*.aa1", "**.aa2", "**.java", "collated**");
private static final List<String> SECOND_AUTHOR_GLOB_LIST = Arrays.asList("**[!(.md)]", "collated**");
private static final List<String> THIRD_AUTHOR_GLOB_LIST = Arrays.asList("", "collated**");

private static final List<String> CONFIG_FORMATS = Arrays.asList("java", "adoc", "md");
private static final List<String> CLI_FORMATS = Arrays.asList("css", "html");
Expand All @@ -59,9 +60,14 @@ public class RepoConfigurationTest {

@BeforeClass
public static void setUp() throws InvalidLocationException {
FIRST_AUTHOR.setAuthorAliases(FIRST_AUTHOR_ALIASES);
SECOND_AUTHOR.setAuthorAliases(SECOND_AUTHOR_ALIASES);
THIRD_AUTHOR.setAuthorAliases(THIRD_AUTHOR_ALIASES);
FOURTH_AUTHOR.setAuthorAliases(FOURTH_AUTHOR_ALIASES);

FIRST_AUTHOR.setIgnoreGlobList(FIRST_AUTHOR_GLOB_LIST);
SECOND_AUTHOR.setIgnoreGlobList(SECOND_AUTHOR_GLOB_LIST);
THIRD_AUTHOR.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);
THIRD_AUTHOR.setIgnoreGlobList(THIRD_AUTHOR_GLOB_LIST);
FOURTH_AUTHOR.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);

List<Author> expectedAuthors = new ArrayList<>();
Expand Down Expand Up @@ -115,12 +121,14 @@ public void repoConfig_usesStandaloneConfig_success() throws InvalidLocationExce
public void repoConfig_ignoresStandaloneConfig_success()
throws ParseException, GitDownloaderException, IOException {
List<Author> expectedAuthors = new ArrayList<>();
expectedAuthors.add(FIRST_AUTHOR);
Author author = new Author(FIRST_AUTHOR);
author.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);
expectedAuthors.add(author);

RepoConfiguration expectedConfig = new RepoConfiguration(TEST_REPO_DELTA, "master");
expectedConfig.setAuthorList(expectedAuthors);
expectedConfig.addAuthorAliases(FIRST_AUTHOR, FIRST_AUTHOR_ALIASES);
expectedConfig.setAuthorDisplayName(FIRST_AUTHOR, "Ahm");
expectedConfig.addAuthorAliases(author, FIRST_AUTHOR_ALIASES);
expectedConfig.setAuthorDisplayName(author, "Ahm");

expectedConfig.setIgnoreGlobList(REPO_LEVEL_GLOB_LIST);
expectedConfig.setFormats(CONFIG_FORMATS);
Expand Down
107 changes: 107 additions & 0 deletions src/test/java/reposense/parser/StandaloneConfigJsonParserTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package reposense.parser;

import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Arrays;

import org.junit.BeforeClass;
import org.junit.Test;

import com.google.gson.JsonSyntaxException;

import reposense.model.Author;
import reposense.model.RepoConfiguration;
import reposense.model.StandaloneConfig;
import reposense.util.TestUtil;

public class StandaloneConfigJsonParserTest {

private static final Path STANDALONE_MALFORMED_CONFIG = new File(
StandaloneConfigJsonParserTest.class.getClassLoader()
.getResource("StandaloneConfigJsonParserTest/standaloneConfig_malformedJson.json").getFile()).toPath();

private static final Path STANDALONE_UNKNOWN_PROPERTY_CONFIG = new File(
StandaloneConfigJsonParserTest.class.getClassLoader().getResource(
"StandaloneConfigJsonParserTest/standaloneConfig_unknownPropertyInJson.json").getFile()).toPath();

private static final Path STANDALONE_CONFIG_FULL = new File(
StandaloneConfigJsonParserTest.class.getClassLoader().getResource(
"StandaloneConfigJsonParserTest/standaloneConfig_full.json").getFile()).toPath();

private static final Path STANDALONE_CONFIG_EMPTY_TEXT_FILE = new File(
StandaloneConfigJsonParserTest.class.getClassLoader().getResource(
"StandaloneConfigJsonParserTest/standaloneConfig_emptyText.json").getFile()).toPath();

private static final Path STANDALONE_CONFIG_EMPTY_JSON_FILE = new File(
StandaloneConfigJsonParserTest.class.getClassLoader().getResource(
"StandaloneConfigJsonParserTest/standaloneConfig_emptyJson.json").getFile()).toPath();

private static final Path STANDALONE_CONFIG_GITHUBID_ONLY = new File(
StandaloneConfigJsonParserTest.class.getClassLoader().getResource(
"StandaloneConfigJsonParserTest/standaloneConfig_githubId_only.json").getFile()).toPath();

private static final String TEST_DUMMY_LOCATION = "https://github.com/reposense/RepoSense.git";

private static RepoConfiguration EXPECTED_GITHUBID_ONLY_REPOCONFIG;
private static RepoConfiguration EXPECTED_FULL_REPOCONFIG;

@BeforeClass
public static void setUp() throws InvalidLocationException {
Author author = new Author("yong24s");
author.setAuthorAliases(Arrays.asList("Yong Hao TENG"));
author.setIgnoreGlobList(Arrays.asList("**.css", "**.html", "**.jade", "**.js"));

EXPECTED_GITHUBID_ONLY_REPOCONFIG = new RepoConfiguration(TEST_DUMMY_LOCATION);
EXPECTED_GITHUBID_ONLY_REPOCONFIG.setFormats(ArgsParser.DEFAULT_FORMATS);
EXPECTED_GITHUBID_ONLY_REPOCONFIG.setAuthorList(Arrays.asList(new Author("yong24s")));

EXPECTED_FULL_REPOCONFIG = new RepoConfiguration(TEST_DUMMY_LOCATION);
EXPECTED_FULL_REPOCONFIG.setFormats(Arrays.asList("gradle", "jade", "java", "js", "md", "scss", "yml"));
EXPECTED_FULL_REPOCONFIG.setIgnoreCommitList(Arrays.asList("7b96c563eb2d3612aa5275364333664a18f01491"));
EXPECTED_FULL_REPOCONFIG.setIgnoreGlobList(Arrays.asList("**.adoc", "collate**"));
EXPECTED_FULL_REPOCONFIG.setAuthorList(Arrays.asList(author));
EXPECTED_FULL_REPOCONFIG.setAuthorDisplayName(author, "Yong Hao");
EXPECTED_FULL_REPOCONFIG.addAuthorAliases(author, Arrays.asList(author.getGitId()));
EXPECTED_FULL_REPOCONFIG.addAuthorAliases(author, author.getAuthorAliases());
}

@Test
public void standaloneConfig_parseEmptyTextFile_success() throws IOException {
new StandaloneConfigJsonParser().parse(STANDALONE_CONFIG_EMPTY_TEXT_FILE);
}

@Test
public void standaloneConfig_parseEmptyJsonFile_success() throws IOException {
new StandaloneConfigJsonParser().parse(STANDALONE_CONFIG_EMPTY_JSON_FILE);
}

@Test
public void standaloneConfig_ignoresUnknownProperty_success() throws IOException {
new StandaloneConfigJsonParser().parse(STANDALONE_UNKNOWN_PROPERTY_CONFIG);
}

@Test
public void standaloneConfig_correctConfig_success() throws IOException, InvalidLocationException {
StandaloneConfig config = new StandaloneConfigJsonParser().parse(STANDALONE_CONFIG_FULL);
assertSameConfig(EXPECTED_FULL_REPOCONFIG, config);
}

@Test
public void standaloneConfig_githubIdOnlyConfig_success() throws IOException, InvalidLocationException {
StandaloneConfig config = new StandaloneConfigJsonParser().parse(STANDALONE_CONFIG_GITHUBID_ONLY);
assertSameConfig(EXPECTED_GITHUBID_ONLY_REPOCONFIG, config);
}

@Test(expected = JsonSyntaxException.class)
public void standaloneConfig_malformedJsonFile_throwsJsonSyntaxException() throws IOException {
new StandaloneConfigJsonParser().parse(STANDALONE_MALFORMED_CONFIG);
}

private void assertSameConfig(RepoConfiguration expectedRepoConfig, StandaloneConfig actualStandaloneConfig)
throws InvalidLocationException {
RepoConfiguration actualRepoConfig = new RepoConfiguration(TEST_DUMMY_LOCATION);
actualRepoConfig.update(actualStandaloneConfig);
TestUtil.compareRepoConfig(expectedRepoConfig, actualRepoConfig);
}
}
19 changes: 19 additions & 0 deletions src/test/java/reposense/util/TestUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.junit.Assert;

import reposense.model.Author;
import reposense.model.RepoConfiguration;

public class TestUtil {
Expand Down Expand Up @@ -114,6 +115,24 @@ public static void compareRepoConfig(RepoConfiguration expectedRepoConfig, RepoC
expectedRepoConfig.getAuthorAliasMap().hashCode(), actualRepoConfig.getAuthorAliasMap().hashCode());
Assert.assertEquals(expectedRepoConfig.getIgnoreGlobList(), actualRepoConfig.getIgnoreGlobList());
Assert.assertEquals(expectedRepoConfig.getFormats(), actualRepoConfig.getFormats());

for (int i = 0; i < expectedRepoConfig.getAuthorList().size(); i++) {
compareAuthor(expectedRepoConfig.getAuthorList().get(i), actualRepoConfig.getAuthorList().get(i));
}
}

/**
* Compares attributes of {@code expectedAuthor} and {@code actualAuthor}, with exception of it's display name.
*
* The display name is not compared as it varies with object construction.
* It is a transient value and it is not needed for object matching.
*
* @throws AssertionError if any attributes fail equality check.
*/
public static void compareAuthor(Author expectedAuthor, Author actualAuthor) {
Assert.assertEquals(expectedAuthor.getGitId(), actualAuthor.getGitId());
Assert.assertEquals(expectedAuthor.getIgnoreGlobList(), actualAuthor.getIgnoreGlobList());
Assert.assertEquals(expectedAuthor.getAuthorAliases(), actualAuthor.getAuthorAliases());
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"ignoreGlobList": ["**.adoc", "collate**"],
"formats": ["gradle", "jade", "java", "js", "md", "scss", "yml"],
"ignoreCommitList": ["7b96c563eb2d3612aa5275364333664a18f01491"],
"authors":
[
{
"githubId": "yong24s",
"displayName": "Yong Hao",
"authorNames": ["Yong Hao TENG"],
"ignoreGlobList": ["**.css", "**.html", "**.jade", "**.js"]
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"authors":
[
{
"githubId": "yong24s"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"authors":
[
{
"githubId": "yong24s"
},
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"authors":
[
{
"githubId": "yong24s",
"level": "1"
},
]
}

0 comments on commit 0337d36

Please sign in to comment.