Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "is construction finished" quests for highway=construction and building=construction #920

Merged
merged 3 commits into from
Apr 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package de.westnordost.streetcomplete.quests;

import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.util.Log;

import de.westnordost.osmapi.map.data.Element;
import de.westnordost.streetcomplete.data.osm.ElementGeometry;
import de.westnordost.streetcomplete.data.osm.OsmElementQuestType;
import de.westnordost.streetcomplete.data.osm.download.MapDataWithGeometryHandler;


import de.westnordost.osmapi.map.data.BoundingBox;

import static junit.framework.Assert.fail;


public class AssertUtil {
public static void verifyYieldsNoQuest(OsmElementQuestType quest, BoundingBox bbox) {
MapDataWithGeometryHandler verifier = (element, geometry) ->
{
fail("Expected zero elements. Element returned: " +
element.getType().name() + "#" + element.getId());
};
quest.download(bbox, verifier);
}

class ElementCounter implements MapDataWithGeometryHandler{
int count = 0;
@Override
public void handle(@NonNull Element element, @Nullable ElementGeometry geometry) {
count += 1;
}
}

public void verifyYieldsQuest(OsmElementQuestType quest, BoundingBox bbox) {
ElementCounter counter = new ElementCounter();
quest.download(bbox, counter);
if(counter.count == 0) {
fail("Expected nonzero elements. Elements not returned");
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you are unhappy about it. Looks like a good integration test setup. I'd perhaps remove the wtf-log, also you could inline the ElementCounter (easiest trick to circumvent the final limitation of inner classes/lambdas: use an int[] count = {0}; and count up count[0]++ )
If you think the test should terminate after the first element encountered: Well, this is an architectural problem of osmapi/StreetComplete because the parser was written as a push-parser, not a pull-parser and thus has no mechanism to stop. But what you could do is to throw a custom runtime exception within the handle method and then catch it just after (would perhaps need a comment though why you do that).

But honestly, I think it is fine to let the download run through. Tests are not time critical and the test can choose a bounding box that is small enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd perhaps remove the wtf-log

Ops, that is debug code that I deleted. Maybe I failed to push the changes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf-log seems to be a weird cache issue. I pushed change removing it - see 041cfbe

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased and force-pushed what solved the issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you are unhappy about it.

I though that it can be simplified and I am missing it, but if there are no obvious improvements then it improves my opinion about myself :)

Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import de.westnordost.osmapi.map.data.BoundingBox;
import de.westnordost.streetcomplete.data.OsmModule;
import de.westnordost.streetcomplete.data.osm.download.MapDataWithGeometryHandler;
import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataDao;
import de.westnordost.streetcomplete.quests.AssertUtil;
import de.westnordost.streetcomplete.quests.housenumber.AddHousenumber;


Expand Down Expand Up @@ -144,11 +144,6 @@ private void verifyYieldsNoQuest(BoundingBox bbox, String date)
{
OverpassMapDataDao o = OsmModule.overpassOldMapDataDao(OsmModule::overpassMapDataParser, date);
AddHousenumber quest = new AddHousenumber(o);
MapDataWithGeometryHandler verifier = (element, geometry) ->
{
fail("Expected zero elements. Element returned: " +
element.getType().name() + "#" + element.getId());
};
quest.download(bbox, verifier);
AssertUtil.verifyYieldsNoQuest(quest, bbox);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package de.westnordost.streetcomplete.quests.construction;

import junit.framework.TestCase;

import java.text.ParseException;

import de.westnordost.osmapi.map.data.BoundingBox;
import de.westnordost.streetcomplete.data.OsmModule;
import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataDao;
import de.westnordost.streetcomplete.quests.AssertUtil;

public class MarkCompletedBuildingConstructionIntegrationTest extends TestCase {
public void test_matching_candidate_is_accepted() throws ParseException {
//https://www.openstreetmap.org/way/494183785#map=19/50.07671/19.94703
verifyYieldsQuest(
new BoundingBox(50.07664, 19.94671, 50.07672, 19.94700),
"2018-03-01"
);
}

public void test_fresh_construction_is_not_accepted() throws ParseException {
//https://www.openstreetmap.org/way/494183785#map=19/50.07671/19.94703
verifyYieldsNoQuest(
new BoundingBox(50.07664, 19.94671, 50.07672, 19.94700),
"2017-07-30"
);
}

public void test_relations_are_accepted() throws ParseException {
//https://www.openstreetmap.org/relation/7405013
verifyYieldsQuest(
new BoundingBox(55.89375, 37.53794, 55.89441, 37.53857),
"2018-03-01"
);
}

private void verifyYieldsNoQuest(BoundingBox bbox, String date) throws ParseException {
OverpassMapDataDao o = OsmModule.overpassOldMapDataDao(OsmModule::overpassMapDataParser, date);
MarkCompletedBuildingConstructionOldData quest = new MarkCompletedBuildingConstructionOldData(o, date);
AssertUtil.verifyYieldsNoQuest(quest, bbox);
}

private void verifyYieldsQuest(BoundingBox bbox, String date) throws ParseException {
OverpassMapDataDao o = OsmModule.overpassOldMapDataDao(OsmModule::overpassMapDataParser, date);
MarkCompletedBuildingConstructionOldData quest = new MarkCompletedBuildingConstructionOldData(o, date);
new AssertUtil().verifyYieldsQuest(quest, bbox);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package de.westnordost.streetcomplete.quests.construction;

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.Date;

import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataDao;
import de.westnordost.streetcomplete.quests.DateUtil;

public class MarkCompletedBuildingConstructionOldData extends MarkCompletedBuildingConstruction {
private Date date;
MarkCompletedBuildingConstructionOldData(OverpassMapDataDao overpassServer, String dateString) throws ParseException {
super(overpassServer);
date = DateUtil.basicISO8601().parse(dateString);
}

@Override
protected String getCurrentDateString(){
return DateUtil.getOffsetDateStringFromDate(0, date) + "T00:00:00Z";
}

@Override
protected String getOffsetDateString(int offset){
return DateUtil.getOffsetDateStringFromDate(offset, date) + "T00:00:00Z";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package de.westnordost.streetcomplete.quests.construction;

import junit.framework.TestCase;

import java.text.ParseException;

import de.westnordost.osmapi.map.data.BoundingBox;
import de.westnordost.streetcomplete.data.OsmModule;
import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataDao;
import de.westnordost.streetcomplete.quests.AssertUtil;

public class MarkCompletedHighwayConstructionIntegrationTest extends TestCase {
public void test_old_highway_construction_triggers_quest() throws ParseException {
//https://www.openstreetmap.org/way/298656945 edited on 2014-08-18
verifyYieldsQuest(
new BoundingBox(40.01422, -3.02250, 40.01694, -3.02134),
"2018-03-10"
);
}

public void test_new_highway_construction_is_not_triggering_quest() throws ParseException {
//https://www.openstreetmap.org/way/298656945 edited on 2014-08-18
verifyYieldsNoQuest(
new BoundingBox(40.01422, -3.02250, 40.01694, -3.02134),
"2014-08-20"
);
}

public void test_opening_date_tag_used_to_filter_out_active_construction() throws ParseException {
//https://www.openstreetmap.org/way/22462987 - 2017-06-30 not generating, 2017-07-01 generating quest
verifyYieldsNoQuest(
new BoundingBox(47.80952, 12.09730, 47.81005, 12.09801),
"2017-06-30"
);
}
public void test_opening_date_tag_ignored_if_outdated() throws ParseException {
//https://www.openstreetmap.org/way/22462987 - 2017-06-30 not generating, 2017-07-01 generating quest
verifyYieldsQuest(
new BoundingBox(47.80952, 12.09730, 47.81005, 12.09801),
"2017-07-01"
);
}


private void verifyYieldsNoQuest(BoundingBox bbox, String date) throws ParseException {
OverpassMapDataDao o = OsmModule.overpassOldMapDataDao(OsmModule::overpassMapDataParser, date);
MarkCompletedHighwayConstructionOldData quest = new MarkCompletedHighwayConstructionOldData(o, date);
AssertUtil.verifyYieldsNoQuest(quest, bbox);
}

private void verifyYieldsQuest(BoundingBox bbox, String date) throws ParseException {
OverpassMapDataDao o = OsmModule.overpassOldMapDataDao(OsmModule::overpassMapDataParser, date);
MarkCompletedHighwayConstructionOldData quest = new MarkCompletedHighwayConstructionOldData(o, date);
new AssertUtil().verifyYieldsQuest(quest, bbox);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package de.westnordost.streetcomplete.quests.construction;

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.Date;

import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataDao;
import de.westnordost.streetcomplete.quests.DateUtil;

public class MarkCompletedHighwayConstructionOldData extends MarkCompletedHighwayConstruction {
private Date date;
MarkCompletedHighwayConstructionOldData(OverpassMapDataDao overpassServer, String dateString) throws ParseException {
super(overpassServer);
date = DateUtil.basicISO8601().parse(dateString);
}

@Override
protected String getCurrentDateString(){
return DateUtil.getOffsetDateStringFromDate(0, date) + "T00:00:00Z";
}

@Override
protected String getOffsetDateString(int offset){
return DateUtil.getOffsetDateStringFromDate(offset, date) + "T00:00:00Z";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,13 @@ public class OsmTaggings
"unpaved","compacted","gravel","fine_gravel","pebblestone","grass_paver",
"ground","earth","dirt","grass","sand","mud","ice","salt","snow","woodchips"
};

public static final String[] ALL_ROADS = {
"motorway", "motorway_link", "trunk", "trunk_link", "primary", "primary_link",
"secondary", "secondary_link", "tertiary", "tertiary_link",
"unclassified", "residential", "living_street", "pedestrian",
"service", "track", "road",
};

public static final String SURVEY_MARK_KEY = "check_date";
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ public void delete(@NonNull String key)
changes.put(key, new StringMapEntryDelete(key, valueBefore));
}

public void deleteIfExists(@NonNull String key)
{
if(source.get(key) == null)
{
return;
}
delete(key);
}

public void add(@NonNull String key, @NonNull String value)
{
if(source.containsKey(key))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package de.westnordost.streetcomplete.data.osm.download;

import java.time.LocalDate;

import javax.inject.Provider;

import de.westnordost.osmapi.OsmConnection;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package de.westnordost.streetcomplete.quests;

import java.text.SimpleDateFormat;
import java.util.Calendar;
import java.util.Date;

public class DateUtil {
public static SimpleDateFormat basicISO8601(){
return new SimpleDateFormat("yyyy-MM-dd");

}
public static String getOffsetDateStringFromDate(final int offsetInDays, final Date date){
Calendar modifiedCalendar = Calendar.getInstance();
modifiedCalendar.setTime(date);
modifiedCalendar.add(Calendar.DAY_OF_MONTH, offsetInDays);
return basicISO8601().format(modifiedCalendar.getTime());
}

public static String getOffsetDateString(int offsetInDays){
return getOffsetDateStringFromDate(offsetInDays, Calendar.getInstance().getTime());
}

public static String getCurrentDateString() {
return getOffsetDateString(0);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import de.westnordost.streetcomplete.quests.localized_name.AddBusStopName;
import de.westnordost.streetcomplete.quests.bus_stop_shelter.AddBusStopShelter;
import de.westnordost.streetcomplete.quests.car_wash_type.AddCarWashType;
import de.westnordost.streetcomplete.quests.construction.MarkCompletedBuildingConstruction;
import de.westnordost.streetcomplete.quests.construction.MarkCompletedHighwayConstruction;
import de.westnordost.streetcomplete.quests.crossing_type.AddCrossingType;
import de.westnordost.streetcomplete.quests.diet_type.AddVegan;
import de.westnordost.streetcomplete.quests.diet_type.AddVegetarian;
Expand Down Expand Up @@ -68,6 +70,7 @@ public class QuestModule
// ↓ 2. important data that is used by many data consumers
new AddRoadName(o, roadNameSuggestionsDao, putRoadNameSuggestionsHandler),
new AddHousenumber(o),
new MarkCompletedHighwayConstruction(o),
// new AddPlaceName(o), doesn't make sense as long as the app cannot tell the generic name of elements

// ↓ 3. useful data that is used by some data consumers
Expand Down Expand Up @@ -109,6 +112,7 @@ public class QuestModule
new AddWheelChairAccessToilets(o),
new AddReligionToWaysideShrine(o),
new AddBikeParkingType(o),
new MarkCompletedBuildingConstruction(o),

// ↓ 8. defined in the wiki, but not really used by anyone yet. Just collected for
// the sake of mapping it in case it makes sense later
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package de.westnordost.streetcomplete.quests.construction;

import android.os.Bundle;
import android.support.annotation.NonNull;

import java.util.Map;

import javax.inject.Inject;

import de.westnordost.osmapi.map.data.BoundingBox;
import de.westnordost.streetcomplete.R;
import de.westnordost.streetcomplete.data.meta.OsmTaggings;
import de.westnordost.streetcomplete.data.osm.changes.StringMapChangesBuilder;
import de.westnordost.streetcomplete.data.osm.download.MapDataWithGeometryHandler;
import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataDao;
import de.westnordost.streetcomplete.data.osm.tql.OverpassQLUtil;
import de.westnordost.streetcomplete.quests.DateUtil;
import de.westnordost.streetcomplete.quests.YesNoQuestAnswerFragment;

public class MarkCompletedBuildingConstruction extends MarkCompletedConstruction
{
@Inject
public MarkCompletedBuildingConstruction(OverpassMapDataDao overpassServer) {
super(overpassServer);
}

@Override public boolean download(BoundingBox bbox, MapDataWithGeometryHandler handler)
{
return overpassServer.getAndHandleQuota(getOverpassQuery(bbox), handler);
}

/** @return overpass query string to get buildings marked as under construction but excluding ones
* - with tagged opening date that is in future
* - recently edited (includes adding/updating check_date tags)
* . */
private String getOverpassQuery(BoundingBox bbox)
{
String groupName = ".buildings_under_construction";
String wayGroupName = groupName + "_ways";
String relationGroupName = groupName + "_relations";
return OverpassQLUtil.getGlobalOverpassBBox(bbox) +
"way" + getQueryPart("building", wayGroupName, 180) +
"relation" + getQueryPart("building", relationGroupName, 180) +
"(" + wayGroupName + "; " + relationGroupName + ";); out meta geom;";
}

public void applyAnswerTo(Bundle answer, StringMapChangesBuilder changes)
{
if(answer.getBoolean(YesNoQuestAnswerFragment.ANSWER)) {
String constructionValue = changes.getPreviousValue("construction");
if(constructionValue == null) {
constructionValue = "yes";
}
changes.modify("building", constructionValue);
removeTagsDescribingConstruction(changes);
} else {
changes.addOrModify(OsmTaggings.SURVEY_MARK_KEY, DateUtil.getCurrentDateString());
}
}

@Override
public int getIcon() {
return R.drawable.ic_quest_building_construction;
}

@Override
public int getTitle() {
return R.string.quest_construction_building_title;
}

@Override public int getTitle(@NonNull Map<String, String> tags) {
return R.string.quest_construction_building_title;
}
}
Loading