From a55fab8d47ab185d5764f4a58907f7550e619b90 Mon Sep 17 00:00:00 2001 From: andrewlalis Date: Sat, 8 Sep 2018 17:20:33 +0200 Subject: [PATCH] Fixed concurrent modification exception. --- src/main/java/nl/andrewlalis/Main.java | 9 +++------ .../nl/andrewlalis/git_api/GithubManager.java | 19 ++++++++----------- .../java/nl/andrewlalis/model/Student.java | 11 +---------- .../executables/DelegateStudentTeams.java | 6 +++--- .../command/executables/ListErrors.java | 13 +++++++++++-- .../command/executables/ReadStudentsFile.java | 11 ++++++++++- .../executables/SetupStudentRepos.java | 15 ++++++++++++++- .../andrewlalis/ui/view/InitializerApp.java | 7 ++++++- .../DelegateStudentTeamsDialog.java | 2 +- .../nl/andrewlalis/util/TeamGenerator.java | 10 +++++++--- 10 files changed, 64 insertions(+), 39 deletions(-) diff --git a/src/main/java/nl/andrewlalis/Main.java b/src/main/java/nl/andrewlalis/Main.java index f2c82f4..2249e3a 100644 --- a/src/main/java/nl/andrewlalis/Main.java +++ b/src/main/java/nl/andrewlalis/Main.java @@ -28,23 +28,20 @@ public class Main { // Command executor which will be used by all actions the user can do. CommandExecutor executor = new CommandExecutor(); - // Main application model is stored as a static variable that is accessible everywhere. - InitializerApp.organization = new Organization(); - // Initialize User Interface. InitializerApp app = new InitializerApp(executor); app.begin(); app.setAccessToken(userOptions.get("token")); // Initialize executable commands. - executor.registerCommand("read_students", new ReadStudentsFile()); + executor.registerCommand("read_students", new ReadStudentsFile(app)); executor.registerCommand("archive_all", new ArchiveRepos()); executor.registerCommand("generate_assignments", new GenerateAssignmentsRepo()); executor.registerCommand("define_ta_teams", new DefineTaTeams(app)); - executor.registerCommand("list_errors", new ListErrors()); + executor.registerCommand("list_errors", new ListErrors(app)); executor.registerCommand("delete_repos", new DeleteRepos()); executor.registerCommand("delegate_student_teams", new DelegateStudentTeams(app)); - executor.registerCommand("setup_student_repos", new SetupStudentRepos()); + executor.registerCommand("setup_student_repos", new SetupStudentRepos(app)); logger.info("GithubManager for Github Repositories in Educational Organizations.\n" + "© Andrew Lalis (2018), All rights reserved.\n" + diff --git a/src/main/java/nl/andrewlalis/git_api/GithubManager.java b/src/main/java/nl/andrewlalis/git_api/GithubManager.java index 8a4c67a..315bb72 100644 --- a/src/main/java/nl/andrewlalis/git_api/GithubManager.java +++ b/src/main/java/nl/andrewlalis/git_api/GithubManager.java @@ -111,7 +111,6 @@ public class GithubManager { // Check if the repository already exists. GHRepository existingRepo = this.organization.getRepository(assignmentsRepoName); if (existingRepo != null) { - InitializerApp.organization.addError(new Error(Severity.MINOR, "Assignments repository already existed, deleting it.")); existingRepo.delete(); logger.fine("Deleted pre-existing assignments repository."); } @@ -172,7 +171,7 @@ public class GithubManager { repo.delete(); logger.info("Deleted repository: " + repo.getName()); } catch (IOException e) { - InitializerApp.organization.addError(new Error(Severity.HIGH, "Could not delete repository: " + repo.getName())); + logger.severe("Could not delete repository: " + repo.getName()); e.printStackTrace(); } } @@ -212,7 +211,8 @@ public class GithubManager { } logger.info("Archived repository: " + repo.getFullName()); } catch (IOException e) { - InitializerApp.organization.addError(new Error(Severity.HIGH, "Could not archive repository: " + repo.getName())); + logger.severe("Could not archive repository: " + repo.getName()); + e.printStackTrace(); } } @@ -233,8 +233,8 @@ public class GithubManager { repo.addCollaborators(users); this.assignmentsRepo.addCollaborators(users); } catch (IOException e) { - InitializerApp.organization.addError(new Error(Severity.HIGH, "Students in team: " + team + " could not be added as collaborators to assignments repo or their repository.")); - logger.warning("Could not add students as collaborators to assignments or their repo."); + logger.severe("Could not add students as collaborators to assignments or their repo.\n" + team); + e.printStackTrace(); } } @@ -248,8 +248,8 @@ public class GithubManager { taTeam.add(studentRepo, GHOrganization.Permission.ADMIN); logger.fine("Added team " + taTeam.getName() + " as admin to repository: " + studentRepo.getName()); } catch (IOException e) { - InitializerApp.organization.addError(new Error(Severity.HIGH, "Could not add TA Team: " + taTeam.getName() + " as ADMIN to repository: " + studentRepo.getName())); logger.severe("Could not add TA Team: " + taTeam.getName() + " as admins to repository: " + studentRepo.getName()); + e.printStackTrace(); } } @@ -269,7 +269,6 @@ public class GithubManager { protectionBuilder.enable(); logger.fine("Protected master branch of repository: " + repo.getName()); } catch (IOException e) { - InitializerApp.organization.addError(new Error(Severity.HIGH, "Could not protect master branch of repository: " + repo.getName())); logger.severe("Could not protect master branch of repository: " + repo.getName()); e.printStackTrace(); } @@ -285,8 +284,7 @@ public class GithubManager { repo.createRef("refs/heads/development", sha1); logger.fine("Created development branch of repository: " + repo.getName()); } catch (IOException e) { - InitializerApp.organization.addError(new Error(Severity.HIGH, "Could not create development branch of repository: " + repo.getName())); - logger.severe("Could not create development branch for repository: " + repo.getName() + '\n' + e.getMessage()); + logger.severe("Could not create development branch for repository: " + repo.getName()); e.printStackTrace(); } } @@ -314,8 +312,7 @@ public class GithubManager { logger.fine("Created repository: " + repo.getName()); return repo; } catch (IOException e) { - logger.severe("Could not create repository: " + name + '\n' + e.getMessage()); - InitializerApp.organization.addError(new Error(Severity.CRITICAL, "Could not create repository: " + name)); + logger.severe("Could not create repository: " + name); e.printStackTrace(); return null; } diff --git a/src/main/java/nl/andrewlalis/model/Student.java b/src/main/java/nl/andrewlalis/model/Student.java index 67e2f3e..e347b58 100644 --- a/src/main/java/nl/andrewlalis/model/Student.java +++ b/src/main/java/nl/andrewlalis/model/Student.java @@ -1,9 +1,5 @@ package nl.andrewlalis.model; -import nl.andrewlalis.model.error.Error; -import nl.andrewlalis.model.error.Severity; -import nl.andrewlalis.ui.view.InitializerApp; - import java.util.List; import java.util.Map; import java.util.logging.Logger; @@ -49,12 +45,7 @@ public class Student extends Person { public StudentTeam getPreferredTeam(Map studentMap) { StudentTeam t = new StudentTeam(); for (int partnerNumber : this.getPreferredPartners()) { - if (!studentMap.containsKey(partnerNumber)) { - InitializerApp.organization.addError(new Error(Severity.MEDIUM, "Student " + this.getNumber() + " has non-existent preferred partner with id: " + partnerNumber)); - logger.warning("Student has non-existent partner id: " + partnerNumber + '\n' + this); - } else { - t.addMember(studentMap.get(partnerNumber)); - } + t.addMember(studentMap.get(partnerNumber)); } t.addMember(this); return t; diff --git a/src/main/java/nl/andrewlalis/ui/control/command/executables/DelegateStudentTeams.java b/src/main/java/nl/andrewlalis/ui/control/command/executables/DelegateStudentTeams.java index 414f92f..69ed042 100644 --- a/src/main/java/nl/andrewlalis/ui/control/command/executables/DelegateStudentTeams.java +++ b/src/main/java/nl/andrewlalis/ui/control/command/executables/DelegateStudentTeams.java @@ -38,7 +38,7 @@ public class DelegateStudentTeams extends GithubExecutable { @Override protected boolean executeWithManager(GithubManager manager, String[] args) { - if (InitializerApp.organization.getStudentTeams().isEmpty()) { + if (this.app.getOrganization().getStudentTeams().isEmpty()) { JOptionPane.showMessageDialog(this.app, "There are no student teams! Please read some from a CSV file first.", "No Student Teams", JOptionPane.ERROR_MESSAGE); return false; } @@ -47,7 +47,7 @@ public class DelegateStudentTeams extends GithubExecutable { if (dialog.isSuccessful()) { Map results = dialog.getResult(); - List teams = InitializerApp.organization.getStudentTeams(); + List teams = this.app.getOrganization().getStudentTeams(); int initialTeamsSize = teams.size(); Stack teamsStack = new Stack<>(); // Randomize the ordering of the student teams here. @@ -61,7 +61,7 @@ public class DelegateStudentTeams extends GithubExecutable { for (int i = 0; i < entry.getValue(); i++) { team.addStudentTeam(teamsStack.pop()); } - InitializerApp.organization.getTaTeams().add(team); + this.app.getOrganization().getTaTeams().add(team); } } else { return false; diff --git a/src/main/java/nl/andrewlalis/ui/control/command/executables/ListErrors.java b/src/main/java/nl/andrewlalis/ui/control/command/executables/ListErrors.java index 9d3e1c7..b99c07a 100644 --- a/src/main/java/nl/andrewlalis/ui/control/command/executables/ListErrors.java +++ b/src/main/java/nl/andrewlalis/ui/control/command/executables/ListErrors.java @@ -11,6 +11,11 @@ import java.util.logging.Logger; */ public class ListErrors implements Executable { + /** + * A reference to the current application. + */ + private InitializerApp app; + /** * The logger for outputting debug info. */ @@ -19,13 +24,17 @@ public class ListErrors implements Executable { logger.setParent(Logger.getGlobal()); } + public ListErrors(InitializerApp app) { + this.app = app; + } + @Override public boolean execute(String[] args) { StringBuilder sb = new StringBuilder("Runtime Errors:\n"); - if (InitializerApp.organization.getErrors().isEmpty()) { + if (this.app.getOrganization().getErrors().isEmpty()) { sb.append("None"); } - for (Error error : InitializerApp.organization.getErrors()) { + for (Error error : this.app.getOrganization().getErrors()) { sb.append(error).append('\n'); } logger.info(sb.toString()); diff --git a/src/main/java/nl/andrewlalis/ui/control/command/executables/ReadStudentsFile.java b/src/main/java/nl/andrewlalis/ui/control/command/executables/ReadStudentsFile.java index 8d14826..cb0b4a0 100644 --- a/src/main/java/nl/andrewlalis/ui/control/command/executables/ReadStudentsFile.java +++ b/src/main/java/nl/andrewlalis/ui/control/command/executables/ReadStudentsFile.java @@ -17,6 +17,15 @@ import java.util.List; */ public class ReadStudentsFile implements Executable { + /** + * A reference to the current application, which contains the model for storing information. + */ + private InitializerApp app; + + public ReadStudentsFile(InitializerApp app) { + this.app = app; + } + @Override public boolean execute(String[] args) { if (args.length < 2) { @@ -28,7 +37,7 @@ public class ReadStudentsFile implements Executable { if (teams == null) { return false; } - InitializerApp.organization.setStudentTeams(teams); + this.app.getOrganization().setStudentTeams(teams); return true; } } diff --git a/src/main/java/nl/andrewlalis/ui/control/command/executables/SetupStudentRepos.java b/src/main/java/nl/andrewlalis/ui/control/command/executables/SetupStudentRepos.java index 140c6e2..411f255 100644 --- a/src/main/java/nl/andrewlalis/ui/control/command/executables/SetupStudentRepos.java +++ b/src/main/java/nl/andrewlalis/ui/control/command/executables/SetupStudentRepos.java @@ -7,13 +7,26 @@ import nl.andrewlalis.ui.view.InitializerApp; import java.util.List; +/** + * This executable, when run, sets up all student repositories. + */ public class SetupStudentRepos extends GithubExecutable { + + /** + * A reference to the current application. + */ + private InitializerApp app; + + public SetupStudentRepos(InitializerApp app) { + this.app = app; + } + @Override protected boolean executeWithManager(GithubManager manager, String[] args) { if (args.length < 1) { return false; } - List taTeams = InitializerApp.organization.getTaTeams(); + List taTeams = this.app.getOrganization().getTaTeams(); for (TATeam team : taTeams) { for (StudentTeam studentTeam : team.getStudentTeams()) { manager.setupStudentRepo(studentTeam, team, args[0]); diff --git a/src/main/java/nl/andrewlalis/ui/view/InitializerApp.java b/src/main/java/nl/andrewlalis/ui/view/InitializerApp.java index 9aeebad..ab856c9 100644 --- a/src/main/java/nl/andrewlalis/ui/view/InitializerApp.java +++ b/src/main/java/nl/andrewlalis/ui/view/InitializerApp.java @@ -42,7 +42,7 @@ public class InitializerApp extends JFrame { /** * The organization object, which contains all important state information. */ - public static Organization organization; + private Organization organization; /** * Constructs a new instance of the main application frame, with both an executor, and organization model. @@ -52,6 +52,7 @@ public class InitializerApp extends JFrame { */ public InitializerApp(CommandExecutor executor) { this.executor = executor; + this.organization = new Organization(); // UI initialization. ImageIcon icon = new ImageIcon(getClass().getResource("/image/icon.png")); @@ -221,6 +222,10 @@ public class InitializerApp extends JFrame { return this.accessTokenField.getText().trim(); } + public Organization getOrganization() { + return this.organization; + } + public void setAccessToken(String accessToken) { this.accessTokenField.setText(accessToken); } diff --git a/src/main/java/nl/andrewlalis/ui/view/dialogs/delegateStudentTeams/DelegateStudentTeamsDialog.java b/src/main/java/nl/andrewlalis/ui/view/dialogs/delegateStudentTeams/DelegateStudentTeamsDialog.java index a4499de..b2c32d8 100644 --- a/src/main/java/nl/andrewlalis/ui/view/dialogs/delegateStudentTeams/DelegateStudentTeamsDialog.java +++ b/src/main/java/nl/andrewlalis/ui/view/dialogs/delegateStudentTeams/DelegateStudentTeamsDialog.java @@ -48,7 +48,7 @@ public class DelegateStudentTeamsDialog extends JDialog { super(parentApp, "Delegate Student Teams", true); this.manager = manager; this.teamSpinners = new ArrayList<>(); - this.totalStudentTeamsCount = InitializerApp.organization.getStudentTeams().size(); + this.totalStudentTeamsCount = parentApp.getOrganization().getStudentTeams().size(); this.setDefaultCloseOperation(DISPOSE_ON_CLOSE); this.initUI(); diff --git a/src/main/java/nl/andrewlalis/util/TeamGenerator.java b/src/main/java/nl/andrewlalis/util/TeamGenerator.java index 6bc00b5..89b8b35 100644 --- a/src/main/java/nl/andrewlalis/util/TeamGenerator.java +++ b/src/main/java/nl/andrewlalis/util/TeamGenerator.java @@ -39,7 +39,6 @@ public class TeamGenerator { logger.fine("Generating teams of size " + teamSize); if (teamSize < 1) { logger.severe("Invalid team size."); - InitializerApp.organization.addError(new Error(Severity.CRITICAL, "Invalid team size while generating teams from CSV.")); throw new IllegalArgumentException("StudentTeam size must be greater than or equal to 1. Got " + teamSize); } logger.fine("Parsing CSV file."); @@ -51,7 +50,6 @@ public class TeamGenerator { studentMap = readAllStudents(records, teamSize); } catch (ArrayIndexOutOfBoundsException e) { logger.severe("StudentTeam size does not match column count in records."); - InitializerApp.organization.addError(new Error(Severity.CRITICAL, "Team size does not match column count in records.")); throw new IllegalArgumentException("StudentTeam size does not match column count in records."); } @@ -156,11 +154,17 @@ public class TeamGenerator { } Student s = new Student(Integer.parseInt(record.get(3)), record.get(2), record.get(1), record.get(4), preferredIds); if (studentMap.containsValue(s)) { - InitializerApp.organization.addError(new Error(Severity.HIGH, "Duplicate entries for student:\n" + s + "\nSince records are in chronological order, this more recent duplicate will override the previous value.")); logger.warning("Duplicate entry found for student: " + s + "\nOverwriting previous value."); } studentMap.put(s.getNumber(), s); } + + // Perform a safety check to ensure all preferred partners are valid students. + for (Map.Entry entry : studentMap.entrySet()) { + // Remove any ids that don't exist in the whole list of students. + entry.getValue().getPreferredPartners().removeIf(partnerId -> !studentMap.containsKey(partnerId)); + } + // At this point, all students are valid, and all preferred partners are valid. logger.fine("Read " + studentMap.size() + " students from records."); return studentMap; }