From 0d279573bb8d7c96d7a4a1dc4b66b2258059dfba Mon Sep 17 00:00:00 2001 From: Ben Companjen Date: Wed, 31 Jul 2024 20:55:11 +0200 Subject: [PATCH] Use JsonUtil.getJsonX more often (#10071) * Update MetricsUtilTest classifiers * Add private MetricsUtil constructor * Make public static field also final This looks like it should have been final * Use Java's standard order of field classifiers * Use empty diamond with generic constructors * Return value directly * Put long notes at top of javadoc * Call JsonUtil.getJsonX to prevent resource leak And add javadoc to the methods * Test StringToJsonX returns null on null --- .../iq/dataverse/metrics/MetricsUtil.java | 70 +++++++++++-------- .../iq/dataverse/metrics/MetricsUtilTest.java | 32 +++++---- 2 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/metrics/MetricsUtil.java b/src/main/java/edu/harvard/iq/dataverse/metrics/MetricsUtil.java index 74bb53e1191..7d968e7e5c1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/metrics/MetricsUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/metrics/MetricsUtil.java @@ -1,7 +1,8 @@ package edu.harvard.iq.dataverse.metrics; import edu.harvard.iq.dataverse.Dataverse; -import java.io.StringReader; +import edu.harvard.iq.dataverse.util.json.JsonUtil; + import java.math.BigDecimal; import java.time.LocalDate; import java.time.YearMonth; @@ -17,28 +18,30 @@ import jakarta.json.JsonArrayBuilder; import jakarta.json.JsonObject; import jakarta.json.JsonObjectBuilder; -import jakarta.json.JsonReader; +import jakarta.json.JsonException; import jakarta.ws.rs.BadRequestException; public class MetricsUtil { private static final Logger logger = Logger.getLogger(MetricsUtil.class.getCanonicalName()); - public final static String CONTENTTYPE = "contenttype"; - public final static String COUNT = "count"; - public final static String CATEGORY = "category"; - public final static String ID = "id"; - public final static String PID = "pid"; - public final static String SUBJECT = "subject"; - public final static String DATE = "date"; - public final static String SIZE = "size"; + public static final String CONTENTTYPE = "contenttype"; + public static final String COUNT = "count"; + public static final String CATEGORY = "category"; + public static final String ID = "id"; + public static final String PID = "pid"; + public static final String SUBJECT = "subject"; + public static final String DATE = "date"; + public static final String SIZE = "size"; - public static String YEAR_AND_MONTH_PATTERN = "yyyy-MM"; + public static final String YEAR_AND_MONTH_PATTERN = "yyyy-MM"; public static final String DATA_LOCATION_LOCAL = "local"; public static final String DATA_LOCATION_REMOTE = "remote"; public static final String DATA_LOCATION_ALL = "all"; + private MetricsUtil() {} + public static JsonObjectBuilder countToJson(long count) { JsonObjectBuilder job = Json.createObjectBuilder(); job.add(COUNT, count); @@ -134,8 +137,8 @@ public static JsonArray timeSeriesToJson(List results, boolean isBigDe public static JsonArray timeSeriesByTypeToJson(List results) { JsonArrayBuilder jab = Json.createArrayBuilder(); - Map totals = new HashMap(); - Map sizes = new HashMap(); + Map totals = new HashMap<>(); + Map sizes = new HashMap<>(); String curDate = (String) results.get(0)[0]; // Get a list of all the monthly dates from the start until now List dates = getDatesFrom(curDate); @@ -169,7 +172,7 @@ public static JsonArray timeSeriesByTypeToJson(List results) { public static JsonArray timeSeriesByPIDToJson(List results) { JsonArrayBuilder jab = Json.createArrayBuilder(); - Map totals = new HashMap(); + Map totals = new HashMap<>(); String curDate = (String) results.get(0)[0]; // Get a list of all the monthly dates from the start until now List dates = getDatesFrom(curDate); @@ -200,8 +203,8 @@ public static JsonArray timeSeriesByPIDToJson(List results) { public static JsonArray timeSeriesByIDAndPIDToJson(List results) { JsonArrayBuilder jab = Json.createArrayBuilder(); - Map totals = new HashMap(); - Map pids = new HashMap(); + Map totals = new HashMap<>(); + Map pids = new HashMap<>(); String curDate = (String) results.get(0)[0]; // Get a list of all the monthly dates from the start until now List dates = getDatesFrom(curDate); @@ -238,11 +241,11 @@ public static JsonArray timeSeriesByIDAndPIDToJson(List results) { /** * - * @param userInput A year and month in YYYY-MM format. - * @return A year and month in YYYY-M * Note that along with sanitization, this checks that the requested month is * not after the current one. This will need to be made more robust if we - * start writing metrics for farther in the future (e.g. the current year) the current year) + * start writing metrics for farther in the future (e.g. the current year) + * @param userInput A year and month in YYYY-MM format. + * @return A year and month in YYYY-M */ public static String sanitizeYearMonthUserInput(String userInput) throws BadRequestException { logger.fine("string from user to sanitize (hopefully YYYY-MM format): " + userInput); @@ -260,8 +263,7 @@ public static String sanitizeYearMonthUserInput(String userInput) throws BadRequ throw new BadRequestException("The requested date is set past the current month."); } - String sanitized = inputLocalDate.format(dateTimeFormatter); - return sanitized; + return inputLocalDate.format(dateTimeFormatter); } public static String validateDataLocationStringType(String dataLocation) throws BadRequestException { @@ -279,30 +281,38 @@ public static String getCurrentMonth() { return LocalDate.now().format(DateTimeFormatter.ofPattern(MetricsUtil.YEAR_AND_MONTH_PATTERN)); } + /** + * Parse a String into a JSON object + * @param str serialized JSON + * @return {@code null} if {@code str} is {@code null}, or the parsed JSON object + * @throws JsonException + * @see JsonUtil#getJsonObject(String) + */ public static JsonObject stringToJsonObject(String str) { if (str == null) { return null; } - JsonReader jsonReader = Json.createReader(new StringReader(str)); - JsonObject jo = jsonReader.readObject(); - jsonReader.close(); - return jo; + return JsonUtil.getJsonObject(str); } + /** + * Parse a String into a JSON array + * @param str serialized JSON + * @return {@code null} if {@code str} is {@code null}, or the parsed JSON array + * @throws JsonException + * @see JsonUtil#getJsonArray(String) + */ public static JsonArray stringToJsonArray(String str) { if (str == null) { return null; } - JsonReader jsonReader = Json.createReader(new StringReader(str)); - JsonArray ja = jsonReader.readArray(); - jsonReader.close(); - return ja; + return JsonUtil.getJsonArray(str); } public static List getDatesFrom(String startMonth) { - List dates = new ArrayList(); + List dates = new ArrayList<>(); LocalDate next = LocalDate.parse(startMonth+ "-01").plusMonths(1); dates.add(startMonth); DateTimeFormatter monthFormat = DateTimeFormatter.ofPattern(YEAR_AND_MONTH_PATTERN); diff --git a/src/test/java/edu/harvard/iq/dataverse/metrics/MetricsUtilTest.java b/src/test/java/edu/harvard/iq/dataverse/metrics/MetricsUtilTest.java index 484ce2ebe47..ca662409a98 100644 --- a/src/test/java/edu/harvard/iq/dataverse/metrics/MetricsUtilTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/metrics/MetricsUtilTest.java @@ -6,38 +6,36 @@ import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.List; -import java.util.Arrays; -import java.util.Collection; import jakarta.json.Json; import jakarta.json.JsonArray; import jakarta.json.JsonArrayBuilder; import jakarta.json.JsonObject; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.fail; -public class MetricsUtilTest { +class MetricsUtilTest { - public static class MetricsUtilNoParamTest { + @Nested + class MetricsUtilNoParamTest { private static final long COUNT = 42l; @Test - public void testCountToJson() { - // This constructor is just here for code coverage. :) - MetricsUtil metricsUtil = new MetricsUtil(); + void testCountToJson() { JsonObject jsonObject = MetricsUtil.countToJson(COUNT).build(); System.out.println(JsonUtil.prettyPrint(jsonObject)); assertEquals(COUNT, jsonObject.getJsonNumber("count").longValue()); } @Test - public void testDataversesByCategoryToJson() { + void testDataversesByCategoryToJson() { List list = new ArrayList<>(); Object[] obj00 = { "RESEARCH_PROJECTS", 791l }; Object[] obj01 = { "RESEARCHERS", 745l }; @@ -66,7 +64,7 @@ public void testDataversesByCategoryToJson() { } @Test - public void testDatasetsBySubjectToJson() { + void testDatasetsBySubjectToJson() { List list = new ArrayList<>(); Object[] obj00 = { "Social Sciences", 24955l }; Object[] obj01 = { "Medicine, Health and Life Sciences", 2262l }; @@ -105,7 +103,7 @@ public void testDatasetsBySubjectToJson() { } @Test - public void testDataversesBySubjectToJson() { + void testDataversesBySubjectToJson() { List list = new ArrayList<>(); Object[] obj00 = { "Social Sciences", 24955l }; Object[] obj01 = { "Medicine, Health and Life Sciences", 2262l }; @@ -164,7 +162,7 @@ void testSanitizeYearMonthUserInputIsAfterCurrentDate() { } @Test - public void testGetCurrentMonth() { + void testGetCurrentMonth() { String expectedMonth = LocalDate.now().format(DateTimeFormatter.ofPattern("yyyy-MM")); String currentMonth = MetricsUtil.getCurrentMonth(); assertEquals(expectedMonth, currentMonth); @@ -173,7 +171,7 @@ public void testGetCurrentMonth() { // Create JsonArray, turn into string and back into array to confirm data // integrity @Test - public void testStringToJsonArrayBuilder() { + void testStringToJsonArrayBuilder() { System.out.println("testStringToJsonArrayBuilder"); List list = new ArrayList<>(); Object[] obj00 = { "Social Sciences", 24955l }; @@ -192,7 +190,7 @@ public void testStringToJsonArrayBuilder() { // Create JsonObject, turn into string and back into array to confirm data // integrity @Test - public void testStringToJsonObjectBuilder() { + void testStringToJsonObjectBuilder() { System.out.println("testStringToJsonObjectBuilder"); JsonObject jsonObjBefore = Json.createObjectBuilder().add("Test", "result").build(); @@ -204,6 +202,12 @@ public void testStringToJsonObjectBuilder() { assertEquals(jsonObjBefore.getString("Test"), jsonObjAfter.getString("Test")); } + @Test + void testStringToJsonWithNull() { + assertNull(MetricsUtil.stringToJsonArray(null)); + assertNull(MetricsUtil.stringToJsonObject(null)); + } + } @ParameterizedTest