Repository: jmrozanec/cron-utils
Pull request #484 (2021-10-02 20:16:27)

Describe "always ..." for HHMMSS
A hacky solution for #477. 

**Why hacky**: I am not particularly happy with the solution as it introduces even more `instanceof` special case handling in `TimeDescriptionStrategy.java`. It seems needed though because the code assumes generating descriptions is independent for each field. It's not and `TimeDescriptionStrategy.describe` is currently the only place where this lack of independence is taken into account.

**Bugs**: I think this discovered two bugs in the other, non-related tests.


Patch:
diff --git a/src/main/java/com/cronutils/descriptor/TimeDescriptionStrategy.java b/src/main/java/com/cronutils/descriptor/TimeDescriptionStrategy.java
index c7ece48..9b68824 100755
--- a/src/main/java/com/cronutils/descriptor/TimeDescriptionStrategy.java
+++ b/src/main/java/com/cronutils/descriptor/TimeDescriptionStrategy.java
@@ -91,17 +91,23 @@ class TimeDescriptionStrategy extends DescriptionStrategy {
 		}
 		String secondsDesc = "";
 		String minutesDesc = "";
-		final String hoursDesc = addTimeExpressions(describe(hours), bundle.getString(HOUR),
-				bundle.getString("hours"));
-		if (!(seconds instanceof On && isDefault((On) seconds))) {
-			secondsDesc = addTimeExpressions(describe(seconds), bundle.getString(SECOND), bundle.getString("seconds"));
+		String hoursDesc = "";
+		if (!(hours instanceof Always)) {
+			hoursDesc = addTimeExpressions(describe(hours), bundle.getString(HOUR), bundle.getString("hours"));
 		}
-		if (!(minutes instanceof On && isDefault((On) minutes))) {
+		if (!(minutes instanceof On && isDefault((On) minutes)) && !((minutes instanceof Always) && (hours instanceof Always))) {
 			minutesDesc = addTimeExpressions(describe(minutes), bundle.getString(MINUTE), bundle.getString("minutes"));
 		}
+		if (!(seconds instanceof On && isDefault((On) seconds))) {
+			secondsDesc = addTimeExpressions(describe(seconds), bundle.getString(SECOND), bundle.getString("seconds"));
+		}
 		return String.format("%s %s %s", secondsDesc, minutesDesc, hoursDesc);
 	}
 
+	protected String describe(final Always always, final boolean and) {
+		return describe(new Every(new IntegerFieldValue(1)), and);
+	}
+
 	private String addTimeExpressions(final String description, final String singular, final String plural) {
 		return description.replaceAll("%s", singular).replaceAll("replace_plural", plural);
 	}


Test patch:
diff --git a/src/test/java/com/cronutils/utils/descriptor/CronDescriptorQuartzIntegrationTest.java b/src/test/java/com/cronutils/utils/descriptor/CronDescriptorQuartzIntegrationTest.java
index 93d1929..52af0b7 100755
--- a/src/test/java/com/cronutils/utils/descriptor/CronDescriptorQuartzIntegrationTest.java
+++ b/src/test/java/com/cronutils/utils/descriptor/CronDescriptorQuartzIntegrationTest.java
@@ -87,7 +87,7 @@ public class CronDescriptorQuartzIntegrationTest {
 
     @Test
     public void testEveryMinuteBetween14and15EveryDay() {
-        assertExpression("0 * 14 * * ?", "at 14 hour");
+        assertExpression("0 * 14 * * ?", "every minute at 14 hour");
     }
 
     @Test
@@ -113,7 +113,7 @@ public class CronDescriptorQuartzIntegrationTest {
      */
     @Test
     public void testDescriptionDayOfWeek() {
-        assertExpression("* 0/1 * ? * TUE", "every minute at Tuesday day");
+        assertExpression("* 0/1 * ? * TUE", "every second every minute at Tuesday day");
     }
 
     /**