quickly copy relevant wind fixes in WindJsonGetServlet to hold lock only shortly; started a test case which currently still fails

java/com.sap.sailing.server.gateway.test/src/com/sap/sailing/server/gateway/impl/WindJsonGetServletRefactoringTest.java

@@ -0,0 +1,141 @@
+package com.sap.sailing.server.gateway.impl;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.argThat;
+import static org.mockito.Matchers.isA;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.stub;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+
+import org.hamcrest.Matcher;
+import org.json.simple.JSONArray;
+import org.json.simple.JSONObject;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.ArgumentMatcher;
+
+import com.sap.sailing.domain.base.RaceDefinition;
+import com.sap.sailing.domain.base.impl.KnotSpeedWithBearingImpl;
+import com.sap.sailing.domain.base.impl.MillisecondsTimePoint;
+import com.sap.sailing.domain.common.Position;
+import com.sap.sailing.domain.common.TimePoint;
+import com.sap.sailing.domain.common.WindSource;
+import com.sap.sailing.domain.common.WindSourceType;
+import com.sap.sailing.domain.common.impl.DegreeBearingImpl;
+import com.sap.sailing.domain.common.impl.DegreePosition;
+import com.sap.sailing.domain.common.impl.WindSourceImpl;
+import com.sap.sailing.domain.tracking.TrackedRace;
+import com.sap.sailing.domain.tracking.Wind;
+import com.sap.sailing.domain.tracking.WindTrack;
+import com.sap.sailing.domain.tracking.impl.CombinedWindTrackImpl;
+import com.sap.sailing.domain.tracking.impl.WindImpl;
+import com.sap.sailing.domain.tracking.impl.WindTrackImpl;
+
+/**
+ * After refactoring the {@link WindJsonGetServlet} to first copy the relevant fixes to hold the read lock only
+ * for a short time, this test uses the old implementation to compute the expected result and compares the
+ * output received from the servlet.
+ * 
+ * @author Axel Uhl (d043530)
+ *
+ */
+public class WindJsonGetServletRefactoringTest {
+    private WindJsonGetServlet servlet;
+    
+    @Before
+    public void setUp() {
+        servlet = new WindJsonGetServlet();
+    }
+    
+    private JSONObject getExpectedResult(String windSourceToRead, TrackedRace trackedRace, TimePoint from, TimePoint to) {
+        JSONObject jsonWindTracks = new JSONObject();
+        List<WindSource> windSources = servlet.getAvailableWindSources(trackedRace);
+        for (WindSource windSource : windSources) {
+            if("ALL".equals(windSourceToRead) || windSource.getType().name().equalsIgnoreCase(windSourceToRead)) {
+                JSONArray jsonWindArray = new JSONArray();
+                WindTrack windTrack = trackedRace.getOrCreateWindTrack(windSource);
+                windTrack.lockForRead();
+                try {
+                    Iterator<Wind> windIter = windTrack.getFixesIterator(from, /* inclusive */true);
+                    while (windIter.hasNext()) {
+                        Wind wind = windIter.next();
+                        if (wind.getTimePoint().compareTo(to) > 0) {
+                            break;
+                        }
+                        JSONObject jsonWind = new JSONObject();
+                        jsonWind.put("truebearingdeg", wind.getBearing().getDegrees());
+                        jsonWind.put("knotspeed", wind.getKnots());
+                        jsonWind.put("meterspersecondspeed", wind.getMetersPerSecond());
+                        if (wind.getTimePoint() != null) {
+                            jsonWind.put("timepoint", wind.getTimePoint().asMillis());
+                            jsonWind.put("dampenedtruebearingdeg",
+                                    windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint())
+                                    .getBearing().getDegrees());
+                            jsonWind.put("dampenedknotspeed",
+                                    windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint()).getKnots());
+                            jsonWind.put("dampenedmeterspersecondspeed",
+                                    windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint())
+                                    .getMetersPerSecond());
+                        }
+                        if (wind.getPosition() != null) {
+                            jsonWind.put("latdeg", wind.getPosition().getLatDeg());
+                            jsonWind.put("lngdeg", wind.getPosition().getLngDeg());
+                        }
+                        jsonWindArray.add(jsonWind);
+                    }
+                } finally {
+                    windTrack.unlockAfterRead();
+                }
+                jsonWindTracks.put(windSource.toString(), jsonWindArray);
+            }
+        }
+        return jsonWindTracks;
+    }
+    
+    @Test
+    public void compareOldAndNewImplementation() {
+        TrackedRace trackedRace = mock(TrackedRace.class);
+        final WindSourceImpl webWindSource = new WindSourceImpl(WindSourceType.WEB);
+        when(trackedRace.getWindSources()).thenReturn(Arrays.asList(new WindSource[] { webWindSource }));
+        RaceDefinition race = mock(RaceDefinition.class);
+        when(race.getName()).thenReturn("Race Name");
+        when(trackedRace.getRace()).thenReturn(race);
+        Set<WindSource> noWindSourcesToExclude = Collections.emptySet();
+        when(trackedRace.getWindSourcesToExclude()).thenReturn(noWindSourcesToExclude);
+        WindTrackImpl webWindTrack = new WindTrackImpl(/* millisecondsOverWhichToAverage */ 30000, /* useSpeed */ true, /* nameForReadWriteLock */ "compareOldAndNewImplementation");
+        when(trackedRace.getOrCreateWindTrack(webWindSource)).thenReturn(webWindTrack);
+        WindSourceImpl combinedWindSource = new WindSourceImpl(WindSourceType.COMBINED);
+        CombinedWindTrackImpl combinedWindTrack = new CombinedWindTrackImpl(trackedRace, /* baseConfidence */ 0.5);
+        when(trackedRace.getOrCreateWindTrack(combinedWindSource)).thenReturn(combinedWindTrack);
+        final TimePoint now = MillisecondsTimePoint.now();
+        final TimePoint earlier = now.minus(10000);
+        final TimePoint later = now.plus(10000);
+        final Matcher<TimePoint> isTooEarlyOrTooLate = new ArgumentMatcher<TimePoint>() {
+            @Override
+            public boolean matches(Object item) {
+                return item instanceof TimePoint &&
+                        ((TimePoint) item).before(earlier);
+            }
+        };
+        final Matcher<TimePoint> isNeitherTooEarlyNorTooLate = new ArgumentMatcher<TimePoint>() {
+            @Override
+            public boolean matches(Object argument) {
+                return !isTooEarlyOrTooLate.matches(argument);
+            }
+        };
+        stub(trackedRace.getWind(isA(Position.class), argThat(isTooEarlyOrTooLate))).toReturn(null);
+        Wind someCombinedWindFix = new WindImpl(new DegreePosition(50, 4), now,
+                new KnotSpeedWithBearingImpl(15, new DegreeBearingImpl(234)));
+        stub(trackedRace.getWind(isA(Position.class), argThat(isNeitherTooEarlyNorTooLate))).toReturn(someCombinedWindFix);
+        webWindTrack.add(new WindImpl(new DegreePosition(49, 3), now,
+                new KnotSpeedWithBearingImpl(12, new DegreeBearingImpl(123))));
+        assertEquals(getExpectedResult(WindJsonGetServlet.ALL, trackedRace, earlier, later),
+                servlet.getResult(WindJsonGetServlet.ALL, trackedRace, earlier, later));
+    }
+}

java/com.sap.sailing.server.gateway/src/com/sap/sailing/server/gateway/impl/WindJsonGetServlet.java

@@ -2,8 +2,10 @@ package com.sap.sailing.server.gateway.impl;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -26,6 +28,8 @@ import com.sap.sailing.server.gateway.AbstractJsonHttpServlet;
 import com.sap.sailing.util.InvalidDateException;
 
 public class WindJsonGetServlet extends AbstractJsonHttpServlet {
+    static final String ALL = "ALL";
+
     private static final long serialVersionUID = -1408004464252437535L;
 
     private static final String PARAM_NAME_WINDSOURCE = "windsource";
@@ -45,7 +49,7 @@ public class WindJsonGetServlet extends AbstractJsonHttpServlet {
                 resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Race not found");
             } else {
                 String windSourceParam = req.getParameter(PARAM_NAME_WINDSOURCE);
-                String windSourceToRead = windSourceParam != null ? windSourceParam : "ALL";         
+                String windSourceToRead = windSourceParam != null ? windSourceParam : ALL;         
 
                 TrackedRace trackedRace = getService().getOrCreateTrackedRegatta(regatta).getTrackedRace(race);
 
@@ -66,54 +70,82 @@ public class WindJsonGetServlet extends AbstractJsonHttpServlet {
                     return;
                 }
 
-                JSONObject jsonWindTracks = new JSONObject();
-                List<WindSource> windSources = getAvailableWindSources(trackedRace);
-                for (WindSource windSource : windSources) {
-                    if("ALL".equals(windSourceToRead) || windSource.getType().name().equalsIgnoreCase(windSourceToRead)) {
-                        JSONArray jsonWindArray = new JSONArray();
-                        WindTrack windTrack = trackedRace.getOrCreateWindTrack(windSource);
-                        windTrack.lockForRead();
-                        try {
-                            Iterator<Wind> windIter = windTrack.getFixesIterator(from, /* inclusive */true);
-                            while (windIter.hasNext()) {
-                                Wind wind = windIter.next();
-                                if (wind.getTimePoint().compareTo(to) > 0) {
-                                    break;
-                                }
-                                JSONObject jsonWind = new JSONObject();
-                                jsonWind.put("truebearingdeg", wind.getBearing().getDegrees());
-                                jsonWind.put("knotspeed", wind.getKnots());
-                                jsonWind.put("meterspersecondspeed", wind.getMetersPerSecond());
-                                if (wind.getTimePoint() != null) {
-                                    jsonWind.put("timepoint", wind.getTimePoint().asMillis());
-                                    jsonWind.put("dampenedtruebearingdeg",
-                                            windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint())
-                                            .getBearing().getDegrees());
-                                    jsonWind.put("dampenedknotspeed",
-                                            windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint()).getKnots());
-                                    jsonWind.put("dampenedmeterspersecondspeed",
-                                            windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint())
-                                            .getMetersPerSecond());
-                                }
-                                if (wind.getPosition() != null) {
-                                    jsonWind.put("latdeg", wind.getPosition().getLatDeg());
-                                    jsonWind.put("lngdeg", wind.getPosition().getLngDeg());
-                                }
-                                jsonWindArray.add(jsonWind);
-                            }
-                        } finally {
-                            windTrack.unlockAfterRead();
-                        }
-                        jsonWindTracks.put(windSource.toString(), jsonWindArray);
-                    }
-                }
+                JSONObject jsonWindTracks = getResult(windSourceToRead, trackedRace, from, to);
                 setJsonResponseHeader(resp);
                 jsonWindTracks.writeJSONString(resp.getWriter());
             }
         }
     }
 
-    private List<WindSource> getAvailableWindSources(TrackedRace trackedRace) {
+    JSONObject getResult(String windSourceToRead, TrackedRace trackedRace, TimePoint from, TimePoint to) {
+        Map<WindSource, List<Wind>> fixes = getRelevantFixes(windSourceToRead, trackedRace, from, to);
+        JSONObject jsonWindTracks = getResultAsJsonObject(trackedRace, fixes);
+        return jsonWindTracks;
+    }
+
+    private Map<WindSource, List<Wind>> getRelevantFixes(String windSourceToRead, TrackedRace trackedRace, TimePoint from,
+            TimePoint to) {
+        // quickly extract relevant fixes to hold locks as shortly as possible; process later
+        List<WindSource> windSources = getAvailableWindSources(trackedRace);
+        Map<WindSource, List<Wind>> fixes = new HashMap<>();
+        for (WindSource windSource : windSources) {
+            if (ALL.equals(windSourceToRead) || windSource.getType().name().equalsIgnoreCase(windSourceToRead)) {
+                ArrayList<Wind> fixesForWindSource = new ArrayList<>();
+                fixes.put(windSource, fixesForWindSource);
+                WindTrack windTrack = trackedRace.getOrCreateWindTrack(windSource);
+                windTrack.lockForRead();
+                try {
+                    Iterator<Wind> windIter = windTrack.getFixesIterator(from, /* inclusive */true);
+                    while (windIter.hasNext()) {
+                        Wind wind = windIter.next();
+                        if (wind.getTimePoint().compareTo(to) > 0) {
+                            break;
+                        } else {
+                            fixesForWindSource.add(wind);
+                        }
+                    }
+                } finally {
+                    windTrack.unlockAfterRead();
+                }
+            }
+        }
+        return fixes;
+    }
+
+    private JSONObject getResultAsJsonObject(TrackedRace trackedRace, Map<WindSource, List<Wind>> fixes) {
+        JSONObject jsonWindTracks = new JSONObject();
+        for (Map.Entry<WindSource, List<Wind>> e : fixes.entrySet()) {
+            WindSource windSource = e.getKey();
+            JSONArray jsonWindArray = new JSONArray();
+            WindTrack windTrack = trackedRace.getOrCreateWindTrack(windSource);
+            for (Wind wind : e.getValue()) {
+                JSONObject jsonWind = new JSONObject();
+                jsonWind.put("truebearingdeg", wind.getBearing().getDegrees());
+                jsonWind.put("knotspeed", wind.getKnots());
+                jsonWind.put("meterspersecondspeed", wind.getMetersPerSecond());
+                if (wind.getTimePoint() != null) {
+                    jsonWind.put("timepoint", wind.getTimePoint().asMillis());
+                    jsonWind.put("dampenedtruebearingdeg",
+                            windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint()).getBearing()
+                                    .getDegrees());
+                    jsonWind.put("dampenedknotspeed",
+                            windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint()).getKnots());
+                    jsonWind.put("dampenedmeterspersecondspeed",
+                            windTrack.getAveragedWind(wind.getPosition(), wind.getTimePoint())
+                                    .getMetersPerSecond());
+                }
+                if (wind.getPosition() != null) {
+                    jsonWind.put("latdeg", wind.getPosition().getLatDeg());
+                    jsonWind.put("lngdeg", wind.getPosition().getLngDeg());
+                }
+                jsonWindArray.add(jsonWind);
+                jsonWindTracks.put(windSource.toString(), jsonWindArray);
+            }
+        }
+        return jsonWindTracks;
+    }
+
+    List<WindSource> getAvailableWindSources(TrackedRace trackedRace) {
         List<WindSource> windSources = new ArrayList<WindSource>();
         for (WindSource windSource : trackedRace.getWindSources()) {
             windSources.add(windSource);