From 086118bb498331de19b4d8d55caa59e0efd41402 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Felix=20D=C3=B6rre?= Date: Sat, 5 Jul 2014 19:35:41 +0200 Subject: [PATCH] Implement better result of "Notary.assure" --- .../cacert/gigi/pages/wot/AssuranceForm.java | 12 ++--- src/org/cacert/gigi/pages/wot/AssurePage.java | 7 ++- src/org/cacert/gigi/util/Notary.java | 53 +++++++++++-------- tests/org/cacert/gigi/util/TestNotary.java | 18 ++++--- 4 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/org/cacert/gigi/pages/wot/AssuranceForm.java b/src/org/cacert/gigi/pages/wot/AssuranceForm.java index be7be716..9bb87af5 100644 --- a/src/org/cacert/gigi/pages/wot/AssuranceForm.java +++ b/src/org/cacert/gigi/pages/wot/AssuranceForm.java @@ -17,6 +17,7 @@ import org.cacert.gigi.output.Form; import org.cacert.gigi.output.Template; import org.cacert.gigi.pages.LoginPage; import org.cacert.gigi.util.Notary; +import org.cacert.gigi.util.Notary.AssuranceResult; public class AssuranceForm extends Form { User assuree; @@ -103,15 +104,14 @@ public class AssuranceForm extends Form { return false; } try { - boolean success = Notary.assure(LoginPage.getUser(req), assuree, - Integer.parseInt(req.getParameter("points")), + AssuranceResult success = Notary.assure(LoginPage.getUser(req), + assuree, Integer.parseInt(req.getParameter("points")), req.getParameter("location"), req.getParameter("date")); - if (!success) { - outputError(out, req, - "Assurance failed. Maybe user data changed."); + if (success != AssuranceResult.ASSURANCE_SUCCEDED) { + outputError(out, req, success.getMessage()); } out.println(""); - return success; + return success == AssuranceResult.ASSURANCE_SUCCEDED; } catch (SQLException e) { e.printStackTrace(); } diff --git a/src/org/cacert/gigi/pages/wot/AssurePage.java b/src/org/cacert/gigi/pages/wot/AssurePage.java index 01502960..f553793e 100644 --- a/src/org/cacert/gigi/pages/wot/AssurePage.java +++ b/src/org/cacert/gigi/pages/wot/AssurePage.java @@ -20,6 +20,7 @@ import org.cacert.gigi.output.Form.CSRFError; import org.cacert.gigi.pages.LoginPage; import org.cacert.gigi.pages.Page; import org.cacert.gigi.util.Notary; +import org.cacert.gigi.util.Notary.AssuranceResult; public class AssurePage extends Page { public static final String PATH = "/wot/assure"; @@ -43,8 +44,10 @@ public class AssurePage extends Page { if (pi.length() > 1) { User myself = LoginPage.getUser(req); int mid = Integer.parseInt(pi.substring(1)); - - if (!Notary.checkAssuranceIsPossible(myself, new User(mid), out)) { + AssuranceResult check = Notary.checkAssuranceIsPossible(myself, + new User(mid)); + if (check != AssuranceResult.ASSURANCE_SUCCEDED) { + out.println(translate(req, check.getMessage())); return; } HttpSession hs = req.getSession(); diff --git a/src/org/cacert/gigi/util/Notary.java b/src/org/cacert/gigi/util/Notary.java index f7327eb9..0577fbfc 100644 --- a/src/org/cacert/gigi/util/Notary.java +++ b/src/org/cacert/gigi/util/Notary.java @@ -1,6 +1,5 @@ package org.cacert.gigi.util; -import java.io.PrintWriter; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; @@ -26,13 +25,10 @@ public class Notary { q.execute(); } - public static boolean checkAssuranceIsPossible(User assurer, User target, - PrintWriter errOut) { + public static AssuranceResult checkAssuranceIsPossible(User assurer, + User target) { if (assurer.getId() == target.getId()) { - if (errOut != null) { - errOut.println("Cannot assure myself."); - } - return false; + return AssuranceResult.CANNOT_ASSURE_SELF; } try { PreparedStatement ps = DatabaseConnection @@ -43,37 +39,48 @@ public class Notary { ps.setInt(2, assurer.getId()); ResultSet rs = ps.executeQuery(); if (rs.next()) { - if (errOut != null) { - errOut.println("You already assured this person."); - } rs.close(); - return false; + return AssuranceResult.ALREADY_ASSUREED; } rs.close(); if (!assurer.canAssure()) { - if (errOut != null) { - errOut.println("You cannot assure."); - } - return false; + return AssuranceResult.CANNOT_ASSURE; } } catch (SQLException e) { e.printStackTrace(); } - return true; + return AssuranceResult.ASSURANCE_SUCCEDED; + } + + public enum AssuranceResult { + CANNOT_ASSURE("You cannot assure."), ALREADY_ASSUREED( + "You already assured this person."), CANNOT_ASSURE_SELF( + "Cannot assure myself."), ASSURANCE_SUCCEDED(""), ASSUREE_CHANGED( + "Person details changed. Please start over again."), POINTS_OUT_OF_RANGE( + "Points out of range."); + String message; + private AssuranceResult(String message) { + this.message = message; + } + public String getMessage() { + return message; + } } - public synchronized static boolean assure(User assurer, User target, - int awarded, String location, String date) throws SQLException { - if (!checkAssuranceIsPossible(assurer, target, null)) { - return false; + public synchronized static AssuranceResult assure(User assurer, + User target, int awarded, String location, String date) + throws SQLException { + AssuranceResult can = checkAssuranceIsPossible(assurer, target); + if (can != AssuranceResult.ASSURANCE_SUCCEDED) { + return can; } User u = new User(target.getId()); if (!u.equals(target)) { - return false; + return AssuranceResult.ASSUREE_CHANGED; } System.out.println("Would now assure."); if (awarded > assurer.getMaxAssurePoints() || awarded < 0) { - return false; + return AssuranceResult.POINTS_OUT_OF_RANGE; } PreparedStatement ps = DatabaseConnection @@ -86,6 +93,6 @@ public class Notary { ps.setString(4, location); ps.setString(5, date); ps.execute(); - return true; + return AssuranceResult.ASSURANCE_SUCCEDED; } } diff --git a/tests/org/cacert/gigi/util/TestNotary.java b/tests/org/cacert/gigi/util/TestNotary.java index 750fd938..7b1f5a13 100644 --- a/tests/org/cacert/gigi/util/TestNotary.java +++ b/tests/org/cacert/gigi/util/TestNotary.java @@ -4,6 +4,7 @@ import java.sql.SQLException; import org.cacert.gigi.User; import org.cacert.gigi.testUtils.ManagedTest; +import org.cacert.gigi.util.Notary.AssuranceResult; import org.junit.Test; import static org.junit.Assert.*; @@ -24,16 +25,17 @@ public class TestNotary extends ManagedTest { 35, 35, 35}; System.out.println(result.length); - assertFalse(Notary.assure(assurer, users[0], -1, "test-notary", - "2014-01-01")); + assertNotEquals(AssuranceResult.ASSURANCE_SUCCEDED, Notary.assure( + assurer, users[0], -1, "test-notary", "2014-01-01")); for (int i = 0; i < result.length; i++) { assertEquals(result[i], assurer.getMaxAssurePoints()); - assertFalse(Notary.assure(assurer, users[i], result[i] + 1, - "test-notary", "2014-01-01")); - assertTrue(Notary.assure(assurer, users[i], result[i], - "test-notary", "2014-01-01")); - assertFalse(Notary.assure(assurer, users[i], result[i], - "test-notary", "2014-01-01")); + assertNotEquals(AssuranceResult.ASSURANCE_SUCCEDED, Notary.assure( + assurer, users[i], result[i] + 1, "test-notary", + "2014-01-01")); + assertEquals(AssuranceResult.ASSURANCE_SUCCEDED, Notary.assure( + assurer, users[i], result[i], "test-notary", "2014-01-01")); + assertNotEquals(AssuranceResult.ASSURANCE_SUCCEDED, Notary.assure( + assurer, users[i], result[i], "test-notary", "2014-01-01")); } assertEquals(35, assurer.getMaxAssurePoints()); -- 2.39.2