From 8d6d359a78ab22c400ba7f5043330b987d0a6c76 Mon Sep 17 00:00:00 2001 From: fmittag Date: Mon, 28 May 2012 14:28:10 +0200 Subject: [PATCH 1/8] Added a filter for mark positions with a 0,0 lat/lon position Such positions are most probably wrong and can cause strange positioning and drawing on the map --- .../domain/tractracadapter/impl/MarkPositionReceiver.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java b/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java index 88e6149cba5..4701a7e5c8b 100755 --- a/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java +++ b/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java @@ -111,7 +111,12 @@ public class MarkPositionReceiver extends AbstractReceiverWithQueue Date: Mon, 28 May 2012 15:42:08 +0200 Subject: [PATCH 2/8] reverted last fix for filtering marks with 0/0 lat lon --- .../domain/tractracadapter/impl/MarkPositionReceiver.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java b/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java index 4701a7e5c8b..88e6149cba5 100755 --- a/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java +++ b/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/MarkPositionReceiver.java @@ -111,12 +111,7 @@ public class MarkPositionReceiver extends AbstractReceiverWithQueue Date: Mon, 28 May 2012 21:11:18 +0200 Subject: [PATCH 3/8] synchronize access to QuadTree because it's not thread safe --- .../META-INF/MANIFEST.MF | 3 +- .../domain/common/AbstractPosition.java | 9 ++-- .../common/quadtree/impl/QuadTreeNode.java | 8 ++- .../sap/sailing/domain/test/QuadTreeTest.java | 49 +++++++++++++++++++ .../geocoding/impl/ReverseGeocoderImpl.java | 12 +++-- 5 files changed, 69 insertions(+), 12 deletions(-) diff --git a/java/com.sap.sailing.domain.common/META-INF/MANIFEST.MF b/java/com.sap.sailing.domain.common/META-INF/MANIFEST.MF index 83d845f6947..359e07e54bd 100755 --- a/java/com.sap.sailing.domain.common/META-INF/MANIFEST.MF +++ b/java/com.sap.sailing.domain.common/META-INF/MANIFEST.MF @@ -7,4 +7,5 @@ Bundle-Vendor: SAP Bundle-RequiredExecutionEnvironment: JavaSE-1.6 Export-Package: com.sap.sailing.domain.common, com.sap.sailing.domain.common.impl, - com.sap.sailing.domain.common.quadtree + com.sap.sailing.domain.common.quadtree, + com.sap.sailing.domain.common.quadtree.impl;x-friends:="com.sap.sailing.domain.test" diff --git a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/AbstractPosition.java b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/AbstractPosition.java index fe458483bbb..82c85f99dd5 100755 --- a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/AbstractPosition.java +++ b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/AbstractPosition.java @@ -12,9 +12,12 @@ public class AbstractPosition implements Position { } public boolean equals(Object o) { - return o instanceof Position && - getLatRad() == ((Position) o).getLatRad() && - getLngRad() == ((Position) o).getLngRad(); + if (o == null) { + return false; + } else { + return o instanceof Position && getLatRad() == ((Position) o).getLatRad() + && getLngRad() == ((Position) o).getLngRad(); + } } @Override diff --git a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/impl/QuadTreeNode.java b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/impl/QuadTreeNode.java index 1de0b986ff7..ee6229d64a6 100644 --- a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/impl/QuadTreeNode.java +++ b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/impl/QuadTreeNode.java @@ -110,7 +110,7 @@ public class QuadTreeNode implements Serializable { * into the children. */ @SuppressWarnings("unchecked") - protected void split() { + protected void split() { // Make sure we're bigger than the minimum, if we care, if (minSize != NO_MIN_SIZE) { if (Math.abs(bounds.getNorthEast().getLatDeg() - bounds.getSouthWest().getLatDeg()) < minSize @@ -121,7 +121,6 @@ public class QuadTreeNode implements Serializable { double nsHalf = (bounds.getNorthEast().getLatDeg() + bounds.getSouthWest().getLatDeg()) / 2.0; double ewHalf = (bounds.getNorthEast().getLngDeg() + bounds.getSouthWest().getLngDeg()) / 2.0; children = new QuadTreeNode[4]; - children[NORTHWEST] = new QuadTreeNode(new Bounds(new DegreePosition(nsHalf, bounds.getSouthWest().getLngDeg()), new DegreePosition(bounds.getNorthEast().getLatDeg(), ewHalf)), maxItems); children[NORTHEAST] = new QuadTreeNode(new Bounds(new DegreePosition(nsHalf, ewHalf), bounds.getNorthEast()), maxItems); children[SOUTHEAST] = new QuadTreeNode(new Bounds(new DegreePosition(bounds.getSouthWest().getLatDeg(), ewHalf), new DegreePosition(nsHalf, bounds.getNorthEast().getLngDeg())), maxItems); @@ -131,7 +130,6 @@ public class QuadTreeNode implements Serializable { for (Iterator> i=temp.iterator(); i.hasNext(); ) { put(i.next()); } - //items.removeAllElements(); } /** @@ -208,9 +206,9 @@ public class QuadTreeNode implements Serializable { this.allTheSamePoint = false; } } - - if (this.items.size() > maxItems && !this.allTheSamePoint) + if (this.items.size() > maxItems && !this.allTheSamePoint) { split(); + } } else { QuadTreeNode node = getChild(leaf.getPoint()); if (node != null) { diff --git a/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/test/QuadTreeTest.java b/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/test/QuadTreeTest.java index 2577b05f536..d557698c362 100755 --- a/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/test/QuadTreeTest.java +++ b/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/test/QuadTreeTest.java @@ -1,12 +1,14 @@ package com.sap.sailing.domain.test; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import org.junit.Test; import com.sap.sailing.domain.common.Position; import com.sap.sailing.domain.common.impl.DegreePosition; import com.sap.sailing.domain.common.quadtree.QuadTree; +import com.sap.sailing.domain.common.quadtree.impl.QuadTreeNode; public class QuadTreeTest { private class GLatLngQuadTree extends QuadTree { @@ -21,6 +23,53 @@ public class QuadTreeTest { } } + private static class QuadTreeWithPublicGetTop extends QuadTree { + private static final long serialVersionUID = -783622065160380333L; + @Override + public QuadTreeNode getTop() { + return super.getTop(); + } + } + + @Test + public void testNoNPEDuringSecondPutInSameLeaf() { + final QuadTreeWithPublicGetTop qt = new QuadTreeWithPublicGetTop(); + final Position p = new DegreePosition(0, 0); + final NullPointerException[] npe = new NullPointerException[1]; + final boolean[] stop = new boolean[1]; + Runnable r = new Runnable() { + @Override + public void run() { + while (!stop[0]) { + synchronized (qt) { + try { + qt.wait(); + // if the following try/catch is moved outside the synchronized block, occasional NPEs result + try { + qt.put(p, p); + } catch (NullPointerException e) { + npe[0] = e; + } + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + } + } + }; + new Thread(r, "1").start(); + new Thread(r, "2").start(); + new Thread(r, "3").start(); + new Thread(r, "4").start(); + for (int i=0; i<10000000; i++) { + synchronized(qt) { + qt.notifyAll(); + } + assertNull("NullPointerException "+(npe[0]==null?"":npe[0].getMessage())+" during iteration "+i, npe[0]); + } + stop[0] = true; + } + @Test public void testDistance() { GLatLngQuadTree quadtree = new GLatLngQuadTree(new DegreePosition(49.29, diff --git a/java/com.sap.sailing.geocoding/src/com/sap/sailing/geocoding/impl/ReverseGeocoderImpl.java b/java/com.sap.sailing.geocoding/src/com/sap/sailing/geocoding/impl/ReverseGeocoderImpl.java index 4a6351fded5..5ae8488de53 100644 --- a/java/com.sap.sailing.geocoding/src/com/sap/sailing/geocoding/impl/ReverseGeocoderImpl.java +++ b/java/com.sap.sailing.geocoding/src/com/sap/sailing/geocoding/impl/ReverseGeocoderImpl.java @@ -194,7 +194,9 @@ public class ReverseGeocoderImpl implements ReverseGeocoder { private void cachePlacemarks(Position position, Double radius, List placemarks) { Collections.sort(placemarks, new Placemark.ByDistance(position)); if (position != null) { - cache.put(position, new Triple>(position, radius, placemarks)); + synchronized (cache) { + cache.put(position, new Triple>(position, radius, placemarks)); + } } } @@ -211,7 +213,9 @@ public class ReverseGeocoderImpl implements ReverseGeocoder { */ private void updateCachedPlacemarks(Position cachedPoint, Double newRadius, List newPlacemarks) { if (cachedPoint != null) { - cache.replace(cachedPoint, new Triple>(cachedPoint, newRadius, newPlacemarks)); + synchronized (cache) { + cache.replace(cachedPoint, new Triple>(cachedPoint, newRadius, newPlacemarks)); + } } } @@ -223,7 +227,9 @@ public class ReverseGeocoderImpl implements ReverseGeocoder { * {@link ReverseGeocoderImpl#POSITION_CACHE_DISTANCE_LIMIT the distance limit} */ private Triple> checkCache(Position position) { - return cache.get(position, POSITION_CACHE_DISTANCE_LIMIT); + synchronized (cache) { + return cache.get(position, POSITION_CACHE_DISTANCE_LIMIT); + } } private JSONArray callNearestService(Position position) throws MalformedURLException, IOException, ParseException { From d62a42079b3bb6900d50d0597c1a1bc3a6bf7e52 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Mon, 28 May 2012 21:17:32 +0200 Subject: [PATCH 4/8] synchronized all other access to QuadTree, too --- .../impl/DeclinationServiceImpl.java | 13 +++++++--- .../declination/impl/DeclinationStore.java | 24 +++++++++++++++---- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationServiceImpl.java b/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationServiceImpl.java index 31d584ae85b..f367854c858 100755 --- a/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationServiceImpl.java +++ b/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationServiceImpl.java @@ -57,7 +57,10 @@ public class DeclinationServiceImpl implements DeclinationService { int year = cal.get(Calendar.YEAR); QuadTree set; while ((set = getYearStore(year)) != null) { - Declination resultForYear = set.get(position); + Declination resultForYear; + synchronized (set) { + resultForYear = set.get(position); + } Distance spatialDistance = resultForYear.getPosition().getDistance(position); // consider result only if it's closer than maxDistance if (spatialDistance.compareTo(maxDistance) <= 0) { @@ -73,7 +76,9 @@ public class DeclinationServiceImpl implements DeclinationService { if (result == null) { QuadTree importerCacheForYear = importerCache.get(year); if (importerCacheForYear != null) { - result = importerCacheForYear.get(position); + synchronized (importerCacheForYear) { + result = importerCacheForYear.get(position); + } if (result.getPosition().getDistance(position).compareTo(maxDistance) <= 0) { return result; // else it's further away from the requested position as demanded by maxDistance @@ -85,7 +90,9 @@ public class DeclinationServiceImpl implements DeclinationService { importerCacheForYear = new QuadTree(); importerCache.put(year, importerCacheForYear); } - importerCacheForYear.put(result.getPosition(), result); + synchronized (importerCacheForYear) { + importerCacheForYear.put(result.getPosition(), result); + } } } return result; diff --git a/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationStore.java b/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationStore.java index cdf3330c584..c7765a741dc 100755 --- a/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationStore.java +++ b/java/com.sap.sailing.declination/src/com/sap/sailing/declination/impl/DeclinationStore.java @@ -49,7 +49,9 @@ public class DeclinationStore { result = new QuadTree(); BufferedReader in = new BufferedReader(new InputStreamReader(is)); while ((record = readExternal(in)) != null) { - result.put(record.getPosition(), record); + synchronized (result) { + result.put(record.getPosition(), record); + } } } return result; @@ -150,7 +152,10 @@ public class DeclinationStore { System.out.println("Date: " + year + "/" + (month + 1) + ", Latitude: " + lat); for (double lng = 0; lng < 180; lng += grid) { Position point = new DegreePosition(lat, lng); - Declination existingDeclinationRecord = storedDeclinations.get(point); + Declination existingDeclinationRecord; + synchronized (storedDeclinations) { + existingDeclinationRecord = storedDeclinations.get(point); + } if (existingDeclinationRecord == null || DeclinationServiceImpl.timeAndSpaceDistance(existingDeclinationRecord .getPosition().getDistance(point), timePoint, existingDeclinationRecord @@ -161,7 +166,10 @@ public class DeclinationStore { } for (double lng = -grid; lng > -180; lng -= grid) { Position point = new DegreePosition(lat, lng); - Declination existingDeclinationRecord = storedDeclinations.get(point); + Declination existingDeclinationRecord; + synchronized (storedDeclinations) { + existingDeclinationRecord = storedDeclinations.get(point); + } if (existingDeclinationRecord == null || DeclinationServiceImpl.timeAndSpaceDistance(existingDeclinationRecord .getPosition().getDistance(point), timePoint, existingDeclinationRecord @@ -175,7 +183,10 @@ public class DeclinationStore { System.out.println("Date: " + year + "/" + (month + 1) + ", Latitude: " + lat); for (double lng = 0; lng < 180; lng += grid) { Position point = new DegreePosition(lat, lng); - Declination existingDeclinationRecord = storedDeclinations.get(point); + Declination existingDeclinationRecord; + synchronized (storedDeclinations) { + existingDeclinationRecord = storedDeclinations.get(point); + } if (DeclinationServiceImpl.timeAndSpaceDistance(existingDeclinationRecord.getPosition().getDistance(point), timePoint, existingDeclinationRecord.getTimePoint()) > 0.1) { // less than ~6 nautical miles and/or ~.6 months off @@ -184,7 +195,10 @@ public class DeclinationStore { } for (double lng = -grid; lng > -180; lng -= grid) { Position point = new DegreePosition(lat, lng); - Declination existingDeclinationRecord = storedDeclinations.get(point); + Declination existingDeclinationRecord; + synchronized (storedDeclinations) { + existingDeclinationRecord = storedDeclinations.get(point); + } if (DeclinationServiceImpl.timeAndSpaceDistance(existingDeclinationRecord.getPosition().getDistance(point), timePoint, existingDeclinationRecord.getTimePoint()) > 0.1) { // less than ~6 nautical miles and/or ~.6 months off From 2709719005508f95463c86e17d1b7749acd97706 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Mon, 28 May 2012 21:21:57 +0200 Subject: [PATCH 5/8] re-enable additional data during creation of createStrippedLeaderboardDTO --- .../java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java index bf5974ddef3..c1231aea193 100755 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java @@ -1772,7 +1772,7 @@ public class SailingServiceImpl extends RemoteServiceServlet implements SailingS groupDTO.name = leaderboardGroup.getName(); groupDTO.description = leaderboardGroup.getDescription(); for (Leaderboard leaderboard : leaderboardGroup.getLeaderboards()) { - groupDTO.leaderboards.add(createStrippedLeaderboardDTO(leaderboard, false)); + groupDTO.leaderboards.add(createStrippedLeaderboardDTO(leaderboard, true)); } return groupDTO; } From 1a98a8cd6d80f917074475e4051010e141b44b4d Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Mon, 28 May 2012 22:21:46 +0200 Subject: [PATCH 6/8] enhanced comment regarding need for synchronization --- .../sailing/domain/common/quadtree/QuadTree.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/QuadTree.java b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/QuadTree.java index 477b1a2ba40..51f42e5f100 100755 --- a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/QuadTree.java +++ b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/quadtree/QuadTree.java @@ -32,11 +32,16 @@ import com.sap.sailing.domain.common.quadtree.impl.Bounds; import com.sap.sailing.domain.common.quadtree.impl.QuadTreeNode; /** - * The QuadTree lets you organize objects in a grid, that redefines - * itself and focuses more gridding when more objects appear in a - * certain area. + * The QuadTree lets you organize objects in a grid, that redefines itself and focuses more gridding when more objects + * appear in a certain area. + *

* - * @param type of object stored by coordinates + * Note that this class is not thread safe. If multiple threads can access the same instance concurrently, callers have + * to ensure proper synchronization. Concurrent reads are permissible while any write should block all other operations. + * + * @param + * type of object stored by coordinates + * @author Axel Uhl (D043530) */ public class QuadTree implements Serializable { From 15926116801ab82f79748673a475426053ba3433 Mon Sep 17 00:00:00 2001 From: Marcus Kammer Date: Tue, 29 May 2012 10:04:29 +0200 Subject: [PATCH 7/8] Set Tool tips for "Home" and jump-to-leaderboard buttons see bug 672 --- .../gwt/ui/raceboard/GlobalNavigationPanel.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/GlobalNavigationPanel.java b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/GlobalNavigationPanel.java index 608b8055f62..6c196e09185 100644 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/GlobalNavigationPanel.java +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/GlobalNavigationPanel.java @@ -36,9 +36,9 @@ public class GlobalNavigationPanel extends FlowPanel { if(showHomeNavigation) { if (leaderboardGroupName != null && !leaderboardGroupName.isEmpty()) { String leaderBoardGroupLink = spectatorViewLink + "?leaderboardGroupName=" + leaderboardGroupName; - addNavigationLink(leaderboardGroupName, leaderBoardGroupLink, "leaderBoardGroup"); + addNavigationLink(leaderboardGroupName, leaderBoardGroupLink, "leaderBoardGroup", "Go to the Event overview."); } else { - addNavigationLink(stringMessages.home(), homeLink, "home"); + addNavigationLink(stringMessages.home(), homeLink, "home", "Go to the Event overview."); } } @@ -47,11 +47,12 @@ public class GlobalNavigationPanel extends FlowPanel { if (leaderboardGroupName != null && !leaderboardGroupName.isEmpty()) { leaderBoardLink += "&leaderboardGroupName=" + leaderboardGroupName; } - addNavigationLink(leaderboardName, leaderBoardLink, "leaderBoard"); + addNavigationLink(leaderboardName, leaderBoardLink, "leaderBoard", "Go to the overview and see all Races in one Leaderboard"); } } - private void addNavigationLink(String linkName, String linkUrl, String styleNameExtension) { + private void addNavigationLink(String linkName, String linkUrl, String styleNameExtension, String htmlTitle) { + String setHtmlTitle = htmlTitle; String url = linkUrl; if(debugParam != null && !debugParam.isEmpty()) { url += url.contains("?") ? "&" : "?"; @@ -60,6 +61,7 @@ public class GlobalNavigationPanel extends FlowPanel { HTML linkHtml = new HTML(ANCHORTEMPLATE.anchor(URLFactory.INSTANCE.encode(url), linkName)); linkHtml.addStyleName(STYLE_NAME_PREFIX + styleNameExtension); + linkHtml.setTitle(setHtmlTitle); add(linkHtml); } } From 633a4b82d8dfecfe1288b4e1d93418b49fd1313c Mon Sep 17 00:00:00 2001 From: Marcus Kammer Date: Tue, 29 May 2012 10:05:55 +0200 Subject: [PATCH 8/8] remove border from raceBoardNavigation-innerElement --- java/com.sap.sailing.gwt.ui/RaceBoard.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/com.sap.sailing.gwt.ui/RaceBoard.css b/java/com.sap.sailing.gwt.ui/RaceBoard.css index 1c240905378..e5e7d71d2c9 100644 --- a/java/com.sap.sailing.gwt.ui/RaceBoard.css +++ b/java/com.sap.sailing.gwt.ui/RaceBoard.css @@ -230,7 +230,6 @@ input.opencoloumn { .raceBoardNavigation-innerElement { background: url("images/btn-cta-orangeleft.png") repeat-x scroll center 0 transparent; - border: medium none; border-bottom-left-radius: 3px; border-top-left-radius: 3px; color: #FFFFFF; @@ -239,6 +238,7 @@ input.opencoloumn { font-size: 14px; margin: 0 0 0 0; padding: 2px 7px 2px 3px; + height: 19px; } .raceBoardNavigation-innerElement:hover { background: url("images/btn-cta-orangeleft.png") repeat-x scroll center -30px transparent;