[astro] Fix returning wrong sun phase name (#14078)

* Add tests and fix very minor bug
* Correct wrong test
* Update tests and fix sorting
* Some checkstyle improvements

Signed-off-by: lsiepel <leosiepel@gmail.com>

bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunCalc.java

@@ -12,7 +12,13 @@
  */
 package org.openhab.binding.astro.internal.calc;
 
+import java.util.ArrayList;
 import java.util.Calendar;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
 
 import org.openhab.binding.astro.internal.model.Eclipse;
@@ -251,9 +257,9 @@ public class SunCalc {
         sun.setSeason(seasonCalc.getSeason(calendar, latitude, useMeteorologicalSeason));
 
         // phase
-        for (Entry<SunPhaseName, Range> rangeEntry : sun.getAllRanges().entrySet()) {
+        for (Entry<SunPhaseName, Range> rangeEntry : sortByValue(sun.getAllRanges()).entrySet()) {
             SunPhaseName entryPhase = rangeEntry.getKey();
-            if (rangeEntry.getValue().matches(Calendar.getInstance())) {
+            if (rangeEntry.getValue().matches(calendar)) {
                 if (entryPhase == SunPhaseName.MORNING_NIGHT || entryPhase == SunPhaseName.EVENING_NIGHT) {
                     sun.getPhase().setName(SunPhaseName.NIGHT);
                 } else {
@@ -336,4 +342,24 @@ public class SunCalc {
     private double getSunriseJulianDate(double jtransit, double jset) {
         return jtransit - (jset - jtransit);
     }
+
+    public static Map<SunPhaseName, Range> sortByValue(Map<SunPhaseName, Range> map) {
+        List<Entry<SunPhaseName, Range>> list = new ArrayList<>(map.entrySet());
+
+        Collections.sort(list, new Comparator<Entry<SunPhaseName, Range>>() {
+            @Override
+            public int compare(Entry<SunPhaseName, Range> p1, Entry<SunPhaseName, Range> p2) {
+                Range p1Range = p1.getValue();
+                Range p2Range = p2.getValue();
+                return p1Range.compareTo(p2Range);
+            }
+        });
+
+        Map<SunPhaseName, Range> result = new LinkedHashMap<>();
+        for (Entry<SunPhaseName, Range> entry : list) {
+            result.put(entry.getKey(), entry.getValue());
+        }
+
+        return result;
+    }
 }

bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/calc/SunZodiacCalc.java

@@ -37,7 +37,6 @@ public class SunZodiacCalc {
      * Returns the zodiac for the specified calendar.
      */
     public Optional<SunZodiac> getZodiac(Calendar calendar) {
-
         int year = calendar.get(Calendar.YEAR);
         List<SunZodiac> zodiacs;
 

bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/job/Job.java

@@ -86,7 +86,6 @@ public interface Job extends SchedulerRunnable, Runnable {
      */
     public static void scheduleEvent(String thingUID, AstroThingHandler astroHandler, Calendar eventAt,
             List<String> events, String channelId, boolean configAlreadyApplied) {
-
         if (events.isEmpty()) {
             return;
         }

bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/MoonPhase.java

@@ -31,7 +31,7 @@ public class MoonPhase {
     private Calendar firstQuarter;
     private Calendar full;
     private Calendar thirdQuarter;
-    private Calendar _new;
+    private Calendar newCalendar;
     private double age;
     private double illumination;
     private double agePercent;
@@ -85,14 +85,14 @@ public class MoonPhase {
      * Returns the date of the new moon.
      */
     public Calendar getNew() {
-        return _new;
+        return newCalendar;
     }
 
     /**
      * Sets the date of the new moon.
      */
-    public void setNew(Calendar _new) {
+    public void setNew(Calendar newCalendar) {
-        this._new = _new;
+        this.newCalendar = newCalendar;
     }
 
     /**

bundles/org.openhab.binding.astro/src/main/java/org/openhab/binding/astro/internal/model/Range.java

@@ -15,6 +15,7 @@ package org.openhab.binding.astro.internal.model;
 import static org.openhab.core.library.unit.MetricPrefix.MILLI;
 
 import java.util.Calendar;
+import java.util.Comparator;
 
 import javax.measure.quantity.Time;
 
@@ -81,4 +82,13 @@ public class Range {
         long matchEnd = end != null ? end.getTimeInMillis() : DateTimeUtils.endOfDayDate(cal).getTimeInMillis();
         return cal.getTimeInMillis() >= matchStart && cal.getTimeInMillis() < matchEnd;
     }
+
+    private static Comparator<Calendar> nullSafeCalendarComparator = Comparator.nullsFirst(Calendar::compareTo);
+
+    private static Comparator<Range> rangeComparator = Comparator.comparing(Range::getStart, nullSafeCalendarComparator)
+            .thenComparing(Range::getEnd, nullSafeCalendarComparator);
+
+    public int compareTo(Range that) {
+        return rangeComparator.compare(this, that);
+    }
 }

bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/calc/SunCalcTest.java

@@ -80,8 +80,8 @@ public class SunCalcTest {
         assertNotNull(sun.getMorningNight());
         assertNotNull(sun.getEveningNight());
 
-        // for an old date the phase is always null
+        // for an old date the phase should also be calculated
-        assertNull(sun.getPhase().getName());
+        assertNotNull(sun.getPhase().getName());
     }
 
     @Test
@@ -274,6 +274,34 @@ public class SunCalcTest {
                 sun.getAllRanges().get(SunPhaseName.EVENING_NIGHT).getStart());
     }
 
+    @Test
+    public void testIssue7642CivilDawnEnd() {
+        TimeZone tZone = TimeZone.getTimeZone("Europe/London");
+        Calendar tDate = SunCalcTest.newCalendar(2020, Calendar.MAY, 13, 5, 12, tZone);
+
+        Sun sun = sunCalc.getSunInfo(tDate, 53.524695, -2.4, 0.0, true);
+        assertEquals(SunPhaseName.CIVIL_DAWN, sun.getPhase().getName());
+    }
+
+    @Test
+    public void testIssue7642SunRiseStart() {
+        // SunCalc.ranges was not sorted, causing unexpected output in corner cases.
+        TimeZone tZone = TimeZone.getTimeZone("Europe/London");
+        Calendar tDate = SunCalcTest.newCalendar(2020, Calendar.MAY, 13, 5, 13, tZone);
+
+        Sun sun = sunCalc.getSunInfo(tDate, 53.524695, -2.4, 0.0, true);
+        assertEquals(SunPhaseName.SUN_RISE, sun.getPhase().getName());
+    }
+
+    @Test
+    public void testIssue7642DaylightStart() {
+        TimeZone tZone = TimeZone.getTimeZone("Europe/London");
+        Calendar tDate = SunCalcTest.newCalendar(2020, Calendar.MAY, 13, 5, 18, tZone);
+
+        Sun sun = sunCalc.getSunInfo(tDate, 53.524695, -2.4, 0.0, true);
+        assertEquals(SunPhaseName.DAYLIGHT, sun.getPhase().getName());
+    }
+
     /***
      * Constructs a <code>GregorianCalendar</code> with the given date and time set
      * for the provided time zone.

bundles/org.openhab.binding.astro/src/test/java/org/openhab/binding/astro/internal/model/SunTest.java

@@ -37,7 +37,7 @@ public class SunTest {
     private Sun sun;
     private AstroChannelConfig config;
 
-    private static ZoneId ZONE = ZoneId.systemDefault();
+    private static final ZoneId ZONE = ZoneId.systemDefault();
 
     @BeforeEach
     public void init() {