diff --git a/app/build.gradle b/app/build.gradle index 7c9de4d441..f913a27b1c 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -81,13 +81,13 @@ dependencies { // tests testImplementation 'junit:junit:4.12' - testImplementation 'org.mockito:mockito-core:2.23.4' + testImplementation 'org.mockito:mockito-core:2.28.2' testImplementation 'org.assertj:assertj-core:2.8.0' + testImplementation 'de.westnordost:osmapi:3.4' androidTestImplementation 'androidx.test:runner:1.2.0' androidTestImplementation 'androidx.test:rules:1.2.0' - // KotlinX Jetifier can't cope with bytebuddy-1.9 which is a dependency of mockito-android 2.23.+ - androidTestImplementation 'org.mockito:mockito-android:2.22.0' + androidTestImplementation 'org.mockito:mockito-android:2.28.2' androidTestImplementation 'org.assertj:assertj-core:2.8.0' // dependency injection @@ -122,7 +122,7 @@ dependencies { // talking with OSM Api implementation 'oauth.signpost:signpost-core:1.2.1.2' - implementation ('de.westnordost:osmapi:3.3') { + implementation ('de.westnordost:osmapi:3.4') { // it's already included in Android exclude group: 'net.sf.kxml', module: 'kxml2' } diff --git a/app/src/androidTest/java/de/westnordost/streetcomplete/data/ApplicationDbTestCase.java b/app/src/androidTest/java/de/westnordost/streetcomplete/data/ApplicationDbTestCase.java index 1425af7c4c..9f7e8694e5 100644 --- a/app/src/androidTest/java/de/westnordost/streetcomplete/data/ApplicationDbTestCase.java +++ b/app/src/androidTest/java/de/westnordost/streetcomplete/data/ApplicationDbTestCase.java @@ -6,9 +6,6 @@ import org.junit.Before; import org.junit.Test; -import de.westnordost.streetcomplete.ApplicationConstants; -import de.westnordost.streetcomplete.quests.localized_name.data.RoadNamesTablesHelper; -import de.westnordost.streetcomplete.quests.oneway.data.WayTrafficFlowTablesHelper; import de.westnordost.streetcomplete.util.KryoSerializer; import de.westnordost.streetcomplete.util.Serializer; diff --git a/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayDaoTest.kt b/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayDaoTest.kt new file mode 100644 index 0000000000..9661373f0b --- /dev/null +++ b/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayDaoTest.kt @@ -0,0 +1,98 @@ +package de.westnordost.streetcomplete.data.osm.persist + +import de.westnordost.osmapi.map.data.OsmLatLon +import de.westnordost.streetcomplete.data.ApplicationDbTestCase +import de.westnordost.streetcomplete.data.QuestTypeRegistry +import de.westnordost.streetcomplete.data.osm.OsmQuestSplitWay +import de.westnordost.streetcomplete.data.osm.changes.SplitAtLinePosition +import de.westnordost.streetcomplete.data.osm.changes.SplitAtPoint +import de.westnordost.streetcomplete.data.osm.persist.test.* +import org.junit.Test + +import org.junit.Assert.* +import org.junit.Before + +class OsmQuestSplitWayDaoTest : ApplicationDbTestCase() { + + private val questType = TestQuestType() + private lateinit var dao: OsmQuestSplitWayDao + + @Before fun createDao() { + dao = OsmQuestSplitWayDao(dbHelper, serializer, QuestTypeRegistry(listOf(questType))) + } + + @Test fun getButNothingIsThere() { + assertNull(dao.get(1L)) + } + + @Test fun getAllButNothingIsThere() { + assertEquals(listOf(), dao.getAll()) + } + + @Test fun putAndGet() { + val id = 1L + val input = createOsmQuestSplitWay(id) + dao.put(input) + val output = dao.get(id)!! + + assertEquals(input.questType, output.questType) + assertEquals(input.wayId, output.wayId) + assertEquals(input.questId, output.questId) + assertEquals(input.splits.size, output.splits.size) + val it = input.splits.listIterator() + val ot = output.splits.listIterator() + while(it.hasNext()) { + val iSplit = it.next() + val oSplit = ot.next() + assertEquals(iSplit, oSplit) + } + } + + @Test fun delete() { + val id = 1L + val input = createOsmQuestSplitWay(id) + dao.put(input) + dao.delete(id) + assertNull(dao.get(id)) + } + + @Test fun putReplaces() { + val id = 1L + dao.put(createOsmQuestSplitWay(id, 1L)) + dao.put(createOsmQuestSplitWay(id, 123L)) + assertEquals(123L, dao.get(id)?.wayId) + assertEquals(1, dao.getAll().size) + } + + @Test fun getAll() { + dao.put(createOsmQuestSplitWay(1L, 1L)) + dao.put(createOsmQuestSplitWay(2L, 2L)) + assertEquals(2, dao.getAll().size) + } + + @Test fun getCount0() { + assertEquals(0, dao.getCount()) + } + + @Test fun getCount1() { + dao.put(createOsmQuestSplitWay(1L, 1L)) + assertEquals(1, dao.getCount()) + } + + @Test fun getCount2() { + dao.put(createOsmQuestSplitWay(1L, 1L)) + dao.put(createOsmQuestSplitWay(2L, 2L)) + assertEquals(2, dao.getCount()) + } + + private fun createOsmQuestSplitWay(id: Long, wayId: Long = 1L): OsmQuestSplitWay { + val pos1 = OsmLatLon(0.0, 0.0) + val pos2 = OsmLatLon(1.0, 0.0) + return OsmQuestSplitWay(id, questType, wayId, "test", listOf( + SplitAtLinePosition(pos1, pos2, 0.3), + SplitAtPoint(pos2)) + ) + } +} + + diff --git a/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDaoTest.kt b/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDaoTest.kt new file mode 100644 index 0000000000..b50f9a2cbe --- /dev/null +++ b/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDaoTest.kt @@ -0,0 +1,71 @@ +package de.westnordost.streetcomplete.data.osm.persist + +import de.westnordost.osmapi.map.data.Element +import de.westnordost.osmapi.map.data.OsmLatLon +import de.westnordost.streetcomplete.data.ApplicationDbTestCase +import de.westnordost.streetcomplete.data.QuestTypeRegistry +import de.westnordost.streetcomplete.data.osm.ElementGeometry +import de.westnordost.streetcomplete.data.osm.UndoOsmQuest +import de.westnordost.streetcomplete.data.osm.changes.StringMapChanges +import de.westnordost.streetcomplete.data.osm.changes.StringMapEntryAdd +import de.westnordost.streetcomplete.data.osm.persist.test.* +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test + +class UndoOsmQuestDaoTest : ApplicationDbTestCase() { + + private val questType = TestQuestType() + private lateinit var geometryDao: ElementGeometryDao + private lateinit var dao: UndoOsmQuestDao + + @Before fun createDaos() { + geometryDao = ElementGeometryDao(dbHelper, serializer) + dao = UndoOsmQuestDao(dbHelper, serializer, QuestTypeRegistry(listOf(questType))) + } + + @Test fun getButNothingIsThere() { + assertNull(dao.get(1L)) + } + + @Test fun getAllButNothingIsThere() { + assertEquals(listOf(), dao.getAll()) + } + + @Test fun addAndGet() { + val id = 1L + val input = addUndoQuest(id) + val output = dao.get(id)!! + + assertEquals(input.id, output.id) + assertEquals(input.type, output.type) + assertEquals(input.geometry, output.geometry) + assertEquals(input.changesSource, output.changesSource) + assertEquals(input.changes, output.changes) + assertEquals(input.elementType, output.elementType) + assertEquals(input.elementId, output.elementId) + } + + @Test fun delete() { + val id = 1L + addUndoQuest(id) + dao.delete(id) + assertNull(dao.get(id)) + } + + @Test fun getAll() { + addUndoQuest(1L, 1L) + addUndoQuest(2L, 2L) + assertEquals(2, dao.getAll().size) + } + + private fun addUndoQuest(id: Long, elementId: Long = 1L): UndoOsmQuest { + val geometry = ElementGeometry(OsmLatLon(1.0, 2.0)) + val elementType = Element.Type.NODE + val changes = StringMapChanges(listOf(StringMapEntryAdd("foo", "bar"))) + val quest = UndoOsmQuest(id, questType, elementType, elementId, changes, "test", geometry) + geometryDao.put(elementType, elementId, geometry) + dao.add(quest) + return quest + } +} diff --git a/app/src/androidTest/java/de/westnordost/streetcomplete/quests/QuestAnswerComponentTest.kt b/app/src/androidTest/java/de/westnordost/streetcomplete/quests/QuestAnswerComponentTest.kt index 41d05bf246..7a87b1923b 100644 --- a/app/src/androidTest/java/de/westnordost/streetcomplete/quests/QuestAnswerComponentTest.kt +++ b/app/src/androidTest/java/de/westnordost/streetcomplete/quests/QuestAnswerComponentTest.kt @@ -26,11 +26,9 @@ class QuestAnswerComponentTest { val c1 = QuestAnswerComponent() val expectQuestId = 3 - val expectGroup = QuestGroup.OSM_NOTE - val expectNote = "test" + val expectGroup = QuestGroup.OSM val expectQuestTitle = "What?" val expectObject = "jo" - val expectImagePaths = listOf("dings","dongs") c1.onAttach(object : OsmQuestAnswerListener { override fun onAnsweredQuest(questId: Long, group: QuestGroup, answer: Any) { @@ -45,12 +43,8 @@ class QuestAnswerComponentTest { assertEquals(expectQuestTitle, questTitle) } - override fun onLeaveNote(questId: Long, group: QuestGroup, questTitle: String, note: String, imagePaths: List?) { - assertEquals(expectQuestId.toLong(), questId) - assertEquals(expectGroup, group) - assertEquals(expectNote, note) - assertEquals(expectQuestTitle, questTitle) - assertEquals(expectImagePaths, imagePaths) + override fun onSplitWay(osmQuestId: Long) { + assertEquals(expectQuestId.toLong(), osmQuestId) } override fun onSkippedQuest(questId: Long, group: QuestGroup) { @@ -61,8 +55,8 @@ class QuestAnswerComponentTest { c1.onCreate(QuestAnswerComponent.createArguments(expectQuestId.toLong(), expectGroup)) c1.onComposeNote(expectQuestTitle) - c1.onLeaveNote(expectQuestTitle, expectNote, expectImagePaths) - c1.onAnswerQuest(expectObject) + c1.onSplitWay() + c1.onAnsweredQuest(expectObject) c1.onSkippedQuest() } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/ApplicationComponent.java b/app/src/main/java/de/westnordost/streetcomplete/ApplicationComponent.java index 62535fe919..cd9ff29058 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/ApplicationComponent.java +++ b/app/src/main/java/de/westnordost/streetcomplete/ApplicationComponent.java @@ -2,6 +2,7 @@ import javax.inject.Singleton; +import androidx.annotation.NonNull; import dagger.Component; import de.westnordost.streetcomplete.data.DbModule; import de.westnordost.streetcomplete.data.OsmModule; @@ -13,6 +14,7 @@ import de.westnordost.streetcomplete.oauth.OsmOAuthDialogFragment; import de.westnordost.streetcomplete.quests.AbstractQuestAnswerFragment; import de.westnordost.streetcomplete.quests.QuestModule; +import de.westnordost.streetcomplete.quests.SplitWayFragment; import de.westnordost.streetcomplete.quests.building_levels.AddBuildingLevelsForm; import de.westnordost.streetcomplete.quests.localized_name.AAddLocalizedNameForm; import de.westnordost.streetcomplete.quests.oneway.AddOnewayForm; @@ -68,4 +70,5 @@ public interface ApplicationComponent void inject(AddBuildingLevelsForm fragment); void inject(ChangesetAutoCloserWorker worker); + void inject(@NonNull SplitWayFragment splitWayFragment); } diff --git a/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.java b/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.java index bc41bb3985..a2375c1b47 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.java +++ b/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.java @@ -7,25 +7,10 @@ import android.content.res.Resources; import android.preference.PreferenceManager; -import java.util.List; - -import javax.inject.Provider; import javax.inject.Singleton; import dagger.Module; import dagger.Provides; -import de.westnordost.streetcomplete.data.QuestController; -import de.westnordost.streetcomplete.data.QuestType; -import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao; -import de.westnordost.streetcomplete.data.download.MobileDataAutoDownloadStrategy; -import de.westnordost.streetcomplete.data.download.WifiAutoDownloadStrategy; -import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; -import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; -import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; -import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestDao; -import de.westnordost.streetcomplete.data.osmnotes.CreateNoteDao; -import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestDao; -import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; import de.westnordost.streetcomplete.location.LocationRequestFragment; import de.westnordost.streetcomplete.oauth.OsmOAuthDialogFragment; import de.westnordost.streetcomplete.sound.SoundFx; @@ -66,31 +51,6 @@ public ApplicationModule(Application application) return application.getResources(); } - @Provides public QuestController questController( - OsmQuestDao osmQuestDB, UndoOsmQuestDao undoOsmQuestDB, MergedElementDao osmElementDB, - ElementGeometryDao geometryDB, OsmNoteQuestDao osmNoteQuestDB, - CreateNoteDao createNoteDB, OpenChangesetsDao manageChangesetsDB, - Provider> questTypesProvider) - { - return new QuestController( - osmQuestDB, undoOsmQuestDB, osmElementDB, geometryDB, osmNoteQuestDB, createNoteDB, - manageChangesetsDB, questTypesProvider, appContext()); - } - - @Provides public static MobileDataAutoDownloadStrategy mobileDataAutoDownloadStrategy( - OsmQuestDao osmQuestDB, DownloadedTilesDao downloadedTilesDao, - Provider> questTypesProvider) - { - return new MobileDataAutoDownloadStrategy(osmQuestDB, downloadedTilesDao, questTypesProvider); - } - - @Provides public static WifiAutoDownloadStrategy wifiAutoDownloadStrategy( - OsmQuestDao osmQuestDB, DownloadedTilesDao downloadedTilesDao, - Provider> questTypesProvider) - { - return new WifiAutoDownloadStrategy(osmQuestDB, downloadedTilesDao, questTypesProvider); - } - @Provides public static LocationRequestFragment locationRequestComponent() { return new LocationRequestFragment(); diff --git a/app/src/main/java/de/westnordost/streetcomplete/MainActivity.java b/app/src/main/java/de/westnordost/streetcomplete/MainActivity.java index d5a71c4671..9074b9c692 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/MainActivity.java +++ b/app/src/main/java/de/westnordost/streetcomplete/MainActivity.java @@ -5,6 +5,7 @@ import android.graphics.Point; import android.graphics.PointF; import androidx.annotation.DrawableRes; +import androidx.annotation.NonNull; import androidx.fragment.app.Fragment; import android.content.BroadcastReceiver; import android.content.ComponentName; @@ -68,17 +69,18 @@ import de.westnordost.osmapi.map.data.Element; import de.westnordost.osmapi.map.data.LatLon; import de.westnordost.osmapi.map.data.OsmElement; +import de.westnordost.osmapi.map.data.Way; import de.westnordost.osmfeatures.FeatureDictionary; import de.westnordost.streetcomplete.about.AboutFragment; import de.westnordost.streetcomplete.data.Quest; import de.westnordost.streetcomplete.data.QuestAutoSyncer; -import de.westnordost.streetcomplete.data.osmnotes.CreateNoteListener; import de.westnordost.streetcomplete.data.QuestController; import de.westnordost.streetcomplete.data.QuestGroup; import de.westnordost.streetcomplete.data.VisibleQuestListener; import de.westnordost.streetcomplete.data.download.QuestDownloadProgressListener; import de.westnordost.streetcomplete.data.download.QuestDownloadService; import de.westnordost.streetcomplete.data.osm.OsmQuest; +import de.westnordost.streetcomplete.data.osm.changes.SplitPolylineAtPosition; import de.westnordost.streetcomplete.data.upload.QuestChangesUploadProgressListener; import de.westnordost.streetcomplete.data.upload.QuestChangesUploadService; import de.westnordost.streetcomplete.data.upload.VersionBannedException; @@ -87,12 +89,14 @@ import de.westnordost.streetcomplete.location.LocationState; import de.westnordost.streetcomplete.location.LocationUtil; import de.westnordost.streetcomplete.oauth.OAuthPrefs; -import de.westnordost.streetcomplete.quests.AbstractBottomSheetFragment; import de.westnordost.streetcomplete.quests.AbstractQuestAnswerFragment; +import de.westnordost.streetcomplete.quests.IsCloseableBottomSheet; +import de.westnordost.streetcomplete.quests.IsShowingQuestDetails; import de.westnordost.streetcomplete.quests.LeaveNoteInsteadFragment; import de.westnordost.streetcomplete.quests.OsmQuestAnswerListener; import de.westnordost.streetcomplete.quests.QuestAnswerComponent; import de.westnordost.streetcomplete.quests.QuestUtilKt; +import de.westnordost.streetcomplete.quests.SplitWayFragment; import de.westnordost.streetcomplete.settings.SettingsActivity; import de.westnordost.streetcomplete.sound.SoundFx; import de.westnordost.streetcomplete.statistics.AnswersCounter; @@ -109,8 +113,9 @@ import static de.westnordost.streetcomplete.ApplicationConstants.MANUAL_DOWNLOAD_QUEST_TYPE_COUNT; public class MainActivity extends AppCompatActivity implements - OsmQuestAnswerListener, CreateNoteListener, VisibleQuestListener, - QuestsMapFragment.Listener, MapFragment.Listener, MapControlsFragment.Listener + OsmQuestAnswerListener, CreateNoteFragment.Listener, VisibleQuestListener, + QuestsMapFragment.Listener, MapFragment.Listener, MapControlsFragment.Listener, + SplitWayFragment.Listener, LeaveNoteInsteadFragment.Listener { @Inject CrashReportExceptionHandler crashReportExceptionHandler; @@ -264,6 +269,7 @@ public void onServiceDisconnected(ComponentName className) Uri data = intent.getData(); if (Intent.ACTION_VIEW.equals(intent.getAction()) && data != null) { + // TODO: geo intent parsing and creating should go into own utility class if ("geo".equals(data.getScheme())) { String geoUriRegex = "geo:(-?[0-9]*\\.?[0-9]+),(-?[0-9]*\\.?[0-9]+).*?(?:\\?z=([0-9]*\\.?[0-9]+))?"; @@ -499,8 +505,7 @@ private void confirmUndo(final OsmQuest quest) { if(dontShowRequestAuthorizationAgain) return; - View inner = LayoutInflater.from(this).inflate( - R.layout.dialog_authorize_now, null, false); + View inner = LayoutInflater.from(this).inflate(R.layout.dialog_authorize_now, null, false); final CheckBox checkBox = inner.findViewById(R.id.checkBoxDontShowAgain); new AlertDialog.Builder(this) @@ -567,6 +572,21 @@ private void downloadAreaConfirmed(BoundingBox bbox) questController.download(bbox, MANUAL_DOWNLOAD_QUEST_TYPE_COUNT, true); } + private void triggerAutoUploadByUserInteraction() + { + if(questAutoSyncer.isAllowedByPreference()) + { + if (!oAuth.isAuthorized()) { + // new users should not be immediately pestered to login after each change (#1446) + if(answersCounter.waitingForUpload() > 5) { + requestOAuthorized(); + } + } + else { + questAutoSyncer.triggerAutoUpload(); + } + } + } /* ------------------------------ Upload progress listener ---------------------------------- */ @@ -733,33 +753,9 @@ else if(e instanceof OsmAuthorizationException) } }; - /* ------------ Managing bottom sheet (quest details) and interaction with map ------------- */ + /* --------------------------------- OsmQuestAnswerListener --------------------------------- */ - private final static String BOTTOM_SHEET = "bottom_sheet"; - - @Override public void onBackPressed() - { - AbstractBottomSheetFragment f = getBottomSheetFragment(); - if(f != null) - { - f.onClickClose(() -> - { - mapFragment.removeQuestGeometry(); - mapFragment.setIsFollowingPosition(isFollowingPosition); - mapFragment.setCompassMode(isCompassMode); - mapFragment.showMapControls(); - MainActivity.super.onBackPressed(); - }); - } - else - { - super.onBackPressed(); - } - } - - /* ------------- OsmQuestAnswerListener ------------- */ - - @Override public void onAnsweredQuest(long questId, QuestGroup group, Object answer) + @Override public void onAnsweredQuest(long questId, @NonNull QuestGroup group, @NonNull Object answer) { questSource.findSource(questId, group, mapFragment.getDisplayedLocation(), source -> { @@ -773,26 +769,33 @@ else if(e instanceof OsmAuthorizationException) }); } - @Override public void onComposeNote(long questId, QuestGroup group, String questTitle) + @Override public void onComposeNote(long questId, @NonNull QuestGroup group, @NonNull String questTitle) { - LeaveNoteInsteadFragment f = new LeaveNoteInsteadFragment(); - Bundle args = QuestAnswerComponent.Companion.createArguments(questId, group); - args.putString(LeaveNoteInsteadFragment.ARG_QUEST_TITLE, questTitle); - f.setArguments(args); + showInBottomSheet(LeaveNoteInsteadFragment.create(questId, group, questTitle)); + } - getSupportFragmentManager().popBackStack(BOTTOM_SHEET, FragmentManager.POP_BACK_STACK_INCLUSIVE); - FragmentTransaction ft = getSupportFragmentManager().beginTransaction(); - ft.setCustomAnimations( - 0, R.animator.quest_answer_form_disappear, - 0, R.animator.quest_answer_form_disappear); - ft.add(R.id.map_bottom_sheet_container, f, BOTTOM_SHEET); - ft.addToBackStack(BOTTOM_SHEET); - ft.commit(); + @Override public void onSplitWay(long osmQuestId) + { + Quest quest = questController.get(osmQuestId, QuestGroup.OSM); + if (quest == null) return; + OsmElement element = questController.getOsmElement((OsmQuest) quest); + if (!(element instanceof Way)) return; + showInBottomSheet(SplitWayFragment.create(osmQuestId, (Way) element, quest.getGeometry())); } - @Override public void onLeaveNote(long questId, QuestGroup group, String questTitle, String note, @Nullable List imagePaths) + @Override public void onSkippedQuest(long questId, @NonNull QuestGroup group) { - closeBottomSheet(); + closeQuestDetailsFor(questId, group); + questController.hide(questId, group); + } + + /* --------------------------- LeaveNoteInsteadFragment.Listener ---------------------------- */ + + @Override public void onCreatedNoteInstead( + long questId, @NonNull QuestGroup group, @NonNull String questTitle, @NonNull String note, + @Nullable List imagePaths) + { + closeQuestDetailsFor(questId, group); // the quest is deleted from DB on creating a note, so need to fetch quest before Quest quest = questController.get(questId, group); if(questController.createNote(questId, questTitle, note, imagePaths)) @@ -802,26 +805,58 @@ else if(e instanceof OsmAuthorizationException) triggerAutoUploadByUserInteraction(); } - private void flingQuestMarkerTo(View quest, View target, Runnable onFinished) + /* ------------------------------ CreateNoteFragment.Listener ------------------------------- */ + + + @Override public void onCreatedNote( + @NonNull String note, @Nullable List imagePaths, @NonNull Point screenPosition) { - int[] targetPos = new int[2]; - target.getLocationOnScreen(targetPos); + showMarkerSolvedAnimation(R.drawable.ic_quest_create_note, new PointF(screenPosition), null); + closeBottomSheet(); - quest.animate() - .scaleX(1.6f).scaleY(1.6f) - .setInterpolator(new OvershootInterpolator(8f)) - .setDuration(250) - .withEndAction(() -> { - quest.animate() - .scaleX(0.2f).scaleY(0.2f) - .alpha(0.8f) - .x(targetPos[0]).y(targetPos[1]) - .setDuration(250) - .setInterpolator(new AccelerateInterpolator()) - .withEndAction(onFinished); + int[] mapPosition = new int[2]; + View mapView = mapFragment.getView(); + if(mapView == null) return; + + mapView.getLocationInWindow(mapPosition); + + PointF notePosition = new PointF(screenPosition); + notePosition.offset(-mapPosition[0], -mapPosition[1]); + + LngLat position = mapFragment.getPositionAt(notePosition); + if(position == null) throw new NullPointerException(); + questController.createNote(note, imagePaths, TangramConst.toLatLon(position)); + triggerAutoUploadByUserInteraction(); + } + + /* ------------------------------- SplitWayFragment.Listener -------------------------------- */ + + @Override public void onSplittedWay(long osmQuestId, @NonNull List splits) + { + questSource.findSource(osmQuestId, QuestGroup.OSM, mapFragment.getDisplayedLocation(), source -> + { + Quest quest = questController.get(osmQuestId, QuestGroup.OSM); + closeQuestDetailsFor(osmQuestId, QuestGroup.OSM); + if(questController.splitWay(osmQuestId, splits, source)) + { + showQuestSolvedAnimation(quest, source); + } + triggerAutoUploadByUserInteraction(); }); } + @Override public void onAddSplit(@NonNull LatLon point) + { + mapFragment.putMarkerForCurrentQuest(point); + } + + @Override public void onRemoveSplit(@NonNull LatLon point) + { + mapFragment.deleteMarkerForCurrentQuest(point); + } + + /* ------------------------------------------------------------------------------------------ */ + private void showQuestSolvedAnimation(Quest quest, String source) { if(quest == null) return; @@ -852,21 +887,27 @@ private void showMarkerSolvedAnimation(@DrawableRes int iconResId, PointF startS }); } - @Override public void onSkippedQuest(long questId, QuestGroup group) + private void flingQuestMarkerTo(View quest, View target, Runnable onFinished) { - closeQuestDetailsFor(questId, group); - questController.hide(questId, group); - } + int[] targetPos = new int[2]; + target.getLocationOnScreen(targetPos); - private void closeQuestDetailsFor(long questId, QuestGroup group) - { - if (isQuestDetailsCurrentlyDisplayedFor(questId, group)) - { - closeBottomSheet(); - } + quest.animate() + .scaleX(1.6f).scaleY(1.6f) + .setInterpolator(new OvershootInterpolator(8f)) + .setDuration(250) + .withEndAction(() -> { + quest.animate() + .scaleX(0.2f).scaleY(0.2f) + .alpha(0.8f) + .x(targetPos[0]).y(targetPos[1]) + .setDuration(250) + .setInterpolator(new AccelerateInterpolator()) + .withEndAction(onFinished); + }); } - /* ------------- creating notes ------------- */ + /* ------------------------------ MapControlsFragment.Listener ------------------------------ */ @Override public void onClickCreateNote() { @@ -876,60 +917,28 @@ private void closeQuestDetailsFor(long questId, QuestGroup group) return; } - AbstractBottomSheetFragment f = getBottomSheetFragment(); - if (f != null) f.onClickClose(this::composeNote); - else composeNote(); + Fragment f = getBottomSheetFragment(); + if (f instanceof IsCloseableBottomSheet) + ((IsCloseableBottomSheet)f).onClickClose(this::composeNote); + else + composeNote(); } private void composeNote() { + freezeMap(); showInBottomSheet(new CreateNoteFragment()); } - @Override public void onLeaveNote(String note, @Nullable List imagePaths, Point screenPosition) - { - showMarkerSolvedAnimation(R.drawable.ic_quest_create_note, new PointF(screenPosition), null); - closeBottomSheet(); - - int[] mapPosition = new int[2]; - View mapView = mapFragment.getView(); - if(mapView == null) return; - - mapView.getLocationInWindow(mapPosition); - - PointF notePosition = new PointF(screenPosition); - notePosition.offset(-mapPosition[0], -mapPosition[1]); - - LngLat position = mapFragment.getPositionAt(notePosition); - if(position == null) throw new NullPointerException(); - questController.createNote(note, imagePaths, TangramConst.toLatLon(position)); - triggerAutoUploadByUserInteraction(); - } - - private void triggerAutoUploadByUserInteraction() - { - if(questAutoSyncer.isAllowedByPreference()) - { - if (!oAuth.isAuthorized()) { - // new users should not be immediately pestered to login after each change (#1446) - if(answersCounter.waitingForUpload() > 5) { - requestOAuthorized(); - } - } - else { - questAutoSyncer.triggerAutoUpload(); - } - } - } - - /* ------------- VisibleQuestListener ------------- */ + /* ---------------------------------- VisibleQuestListener ---------------------------------- */ @AnyThread @Override public void onQuestsCreated(final Collection quests, final QuestGroup group) { runOnUiThread(() -> mapFragment.addQuests(quests, group)); // to recreate element geometry of selected quest (if any) after recreation of activity - if(getQuestDetailsFragment() != null) + Fragment f = getBottomSheetFragment(); + if(f instanceof IsShowingQuestDetails) { for (Quest q : quests) { @@ -952,17 +961,36 @@ public synchronized void onQuestsRemoved(Collection questIds, QuestGroup g for(long questId : questIds) { - if (!isQuestDetailsCurrentlyDisplayedFor(questId, group)) continue; + runOnUiThread(() -> { closeQuestDetailsFor(questId, group); }); + } + } + + /* ------------ Managing bottom sheet (quest details) and interaction with map ------------- */ + + private final static String BOTTOM_SHEET = "bottom_sheet"; - runOnUiThread(this::closeBottomSheet); - // disabled this feature (for now), it does not feel good - /*Quest quest = questController.getNextAt(questId, group); - if(quest != null) + @Override public void onBackPressed() + { + Fragment f = getBottomSheetFragment(); + if(f instanceof IsCloseableBottomSheet) + { + ((IsCloseableBottomSheet)f).onClickClose(() -> { - runOnUiThread(() -> showQuestDetails(quest, group)); - }*/ + getSupportFragmentManager().popBackStackImmediate(BOTTOM_SHEET, FragmentManager.POP_BACK_STACK_INCLUSIVE); + unfreezeMap(); + }); + } + else + { + super.onBackPressed(); + } + } - break; + private void closeQuestDetailsFor(long questId, QuestGroup group) + { + if (isQuestDetailsCurrentlyDisplayedFor(questId, group)) + { + closeBottomSheet(); } } @@ -986,25 +1014,19 @@ public synchronized void onQuestsRemoved(Collection questIds, QuestGroup g } getSupportFragmentManager().popBackStackImmediate(BOTTOM_SHEET, FragmentManager.POP_BACK_STACK_INCLUSIVE); - - mapFragment.setIsFollowingPosition(isFollowingPosition); - mapFragment.setCompassMode(isCompassMode); - mapFragment.removeQuestGeometry(); - mapFragment.showMapControls(); + unfreezeMap(); } private boolean isQuestDetailsCurrentlyDisplayedFor(long questId, QuestGroup group) { - AbstractQuestAnswerFragment currentFragment = getQuestDetailsFragment(); - return currentFragment != null - && currentFragment.getQuestId() == questId - && currentFragment.getQuestGroup() == group; + Fragment f = getBottomSheetFragment(); + return f instanceof IsShowingQuestDetails + && ((IsShowingQuestDetails)f).getQuestId() == questId + && ((IsShowingQuestDetails)f).getQuestGroup() == group; } @UiThread private void showQuestDetails(Quest quest, QuestGroup group) { - mapFragment.addQuestGeometry(quest.getGeometry()); - if(isQuestDetailsCurrentlyDisplayedFor(quest.getId(), group)) return; if(getBottomSheetFragment() != null) @@ -1027,49 +1049,56 @@ private boolean isQuestDetailsCurrentlyDisplayedFor(long questId, QuestGroup gro args.putFloat(AbstractQuestAnswerFragment.ARG_MAP_TILT, mapTilt); f.setArguments(args); + freezeMap(); showInBottomSheet(f); } - private void showInBottomSheet(Fragment f) - { - isFollowingPosition = mapFragment.isFollowingPosition(); - isCompassMode = mapFragment.isCompassMode(); - mapFragment.setIsFollowingPosition(false); - mapFragment.setCompassMode(false); - mapFragment.hideMapControls(); - + private void showInBottomSheet(Fragment f) { + int appearAnim = getBottomSheetFragment() == null ? R.animator.quest_answer_form_appear : 0; + int disappearAnim = R.animator.quest_answer_form_disappear; FragmentTransaction ft = getSupportFragmentManager().beginTransaction(); - ft.setCustomAnimations( - R.animator.quest_answer_form_appear, R.animator.quest_answer_form_disappear, - R.animator.quest_answer_form_appear, R.animator.quest_answer_form_disappear); - ft.add(R.id.map_bottom_sheet_container, f, BOTTOM_SHEET); + ft.setCustomAnimations(appearAnim, disappearAnim, appearAnim, disappearAnim); + ft.replace(R.id.map_bottom_sheet_container, f, BOTTOM_SHEET); ft.addToBackStack(BOTTOM_SHEET); ft.commit(); } - private AbstractBottomSheetFragment getBottomSheetFragment() + private Fragment getBottomSheetFragment() { - return (AbstractBottomSheetFragment) getSupportFragmentManager().findFragmentByTag(BOTTOM_SHEET); + return getSupportFragmentManager().findFragmentByTag(BOTTOM_SHEET); } - private AbstractQuestAnswerFragment getQuestDetailsFragment() + private void freezeMap() { - AbstractBottomSheetFragment f = getBottomSheetFragment(); + isFollowingPosition = mapFragment.isFollowingPosition(); + isCompassMode = mapFragment.isCompassMode(); + mapFragment.setIsFollowingPosition(false); + mapFragment.setCompassMode(false); + mapFragment.hideMapControls(); + } - return f instanceof AbstractQuestAnswerFragment ? (AbstractQuestAnswerFragment) f : null ; + private void unfreezeMap() + { + mapFragment.setIsFollowingPosition(isFollowingPosition); + mapFragment.setCompassMode(isCompassMode); + mapFragment.removeQuestGeometry(); + mapFragment.showMapControls(); } + /* ---------------------------------- MapFragment.Listener ---------------------------------- */ + @AnyThread @Override public void onMapOrientation(float rotation, float tilt) { mapRotation = rotation; mapTilt = tilt; - AbstractQuestAnswerFragment f = getQuestDetailsFragment(); - if (f != null) + Fragment f = getBottomSheetFragment(); + if (f instanceof AbstractQuestAnswerFragment) { - f.onMapOrientation(rotation, tilt); + ((AbstractQuestAnswerFragment)f).onMapOrientation(rotation, tilt); } } - /* ---------- QuestsMapFragment.Listener ---------- */ + + /* ------------------------------- QuestsMapFragment.Listener ------------------------------- */ @Override public void onFirstInView(BoundingBox bbox) { @@ -1086,21 +1115,25 @@ private AbstractQuestAnswerFragment getQuestDetailsFragment() if(quest != null) showQuestDetails(quest, questGroup); }; - AbstractBottomSheetFragment f = getBottomSheetFragment(); - if (f != null) f.onClickClose(retrieveQuest); - else retrieveQuest.run(); + Fragment f = getBottomSheetFragment(); + if (f instanceof IsCloseableBottomSheet) + ((IsCloseableBottomSheet)f).onClickClose(retrieveQuest); + else + retrieveQuest.run(); } - @Override public void onClickedMapAt(@Nullable LatLon position) + @Override public void onClickedMapAt(@NonNull LatLon position, double clickAreaSizeInMeters) { - AbstractBottomSheetFragment f = getBottomSheetFragment(); - if(f != null) + Fragment f = getBottomSheetFragment(); + if (f instanceof IsCloseableBottomSheet) { - f.onClickClose(this::closeBottomSheet); + IsCloseableBottomSheet g = (IsCloseableBottomSheet) f; + if(!g.onClickMapAt(position, clickAreaSizeInMeters)) + g.onClickClose(this::closeBottomSheet); } } - /* ---------- Location listener ---------- */ + /* ------------------------------------ Location listener ----------------------------------- */ private void updateLocationAvailability() { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/DbModule.java b/app/src/main/java/de/westnordost/streetcomplete/data/DbModule.java index ec90eaac1f..a952f84b43 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/DbModule.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/DbModule.java @@ -4,10 +4,6 @@ import android.content.SharedPreferences; import android.database.sqlite.SQLiteOpenHelper; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; - import javax.inject.Singleton; import dagger.Module; @@ -80,23 +76,4 @@ public static SQLiteOpenHelper sqliteOpenHelper(Context ctx, String databaseName { return new QuestTypeOrderList(prefs, questTypeRegistry); } - - @Provides public static List visibleQuestTypes( - QuestTypeRegistry questTypeRegistry, VisibleQuestTypeDao visibleQuestTypeDao, - QuestTypeOrderList questTypeOrderList) - { - List questTypes = new ArrayList<>(questTypeRegistry.getAll()); - Iterator it = questTypes.listIterator(); - while(it.hasNext()) - { - QuestType questType = it.next(); - if(!visibleQuestTypeDao.isVisible(questType)) - { - it.remove(); - } - } - questTypeOrderList.sort(questTypes); - - return questTypes; - } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/OsmModule.java b/app/src/main/java/de/westnordost/streetcomplete/data/OsmModule.java index eb60598f4a..aea16bfacf 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/OsmModule.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/OsmModule.java @@ -5,8 +5,6 @@ import java.io.File; -import javax.inject.Inject; -import javax.inject.Named; import javax.inject.Provider; import javax.inject.Singleton; @@ -22,9 +20,17 @@ import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayDao; import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestDao; -import de.westnordost.streetcomplete.data.osm.upload.OsmQuestChangeUpload; +import de.westnordost.streetcomplete.data.osm.upload.OpenQuestChangesetsManager; +import de.westnordost.streetcomplete.data.osm.upload.OsmQuestsUpload; +import de.westnordost.streetcomplete.data.osm.upload.SingleOsmElementTagChangesUpload; +import de.westnordost.streetcomplete.data.osm.upload.SplitSingleWayUpload; +import de.westnordost.streetcomplete.data.osm.upload.SplitWaysUpload; +import de.westnordost.streetcomplete.data.osm.upload.UndoOsmQuestsUpload; import de.westnordost.streetcomplete.data.osmnotes.OsmAvatarsDownload; +import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao; +import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; import de.westnordost.streetcomplete.oauth.OAuthPrefs; import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataDao; import de.westnordost.streetcomplete.data.osm.download.OverpassMapDataParser; @@ -118,22 +124,33 @@ public static OsmConnection osmConnection(OAuthConsumer consumer) return new MapDataDao(osm); } - @Provides public static OsmQuestChangeUpload osmQuestChangeUpload( - MapDataDao osmDao, OsmQuestDao questDB, MergedElementDao elementDB, - ElementGeometryDao elementGeometryDB, OsmApiWayGeometrySource wayGeometrySource, - OsmQuestGiver questGiver) - { - return new OsmQuestChangeUpload(osmDao, questDB, elementDB, elementGeometryDB, - new ElementGeometryCreator(wayGeometrySource), questGiver); - } - - @Provides @Named("undo") public static OsmQuestChangeUpload undoOsmQuestChangeUpload( - MapDataDao osmDao, UndoOsmQuestDao questDB, MergedElementDao elementDB, - ElementGeometryDao elementGeometryDB, OsmApiWayGeometrySource wayGeometrySource, - OsmQuestGiver questGiver) - { - return new OsmQuestChangeUpload(osmDao, questDB, elementDB, elementGeometryDB, - new ElementGeometryCreator(wayGeometrySource), questGiver); + @Provides public static OsmQuestsUpload osmQuestsUpload( + MergedElementDao elementDB, ElementGeometryDao elementGeometryDB, + OpenQuestChangesetsManager changesetManager, OsmQuestGiver questGiver, + QuestStatisticsDao statisticsDB, OsmApiWayGeometrySource wayGeometrySource, + OsmQuestDao questDB, SingleOsmElementTagChangesUpload singleChangeUpload, + DownloadedTilesDao downloadedTilesDao) { + return new OsmQuestsUpload(elementDB, elementGeometryDB, changesetManager, questGiver, + statisticsDB, new ElementGeometryCreator(wayGeometrySource), questDB, + singleChangeUpload, downloadedTilesDao); + } + + @Provides public static UndoOsmQuestsUpload undoOsmQuestsUpload( + MergedElementDao elementDB, ElementGeometryDao elementGeometryDB, + OpenQuestChangesetsManager changesetManager, OsmQuestGiver questGiver, + QuestStatisticsDao statisticsDB, OsmApiWayGeometrySource wayGeometrySource, + UndoOsmQuestDao questDB, SingleOsmElementTagChangesUpload singleChangeUpload) { + return new UndoOsmQuestsUpload(elementDB, elementGeometryDB, changesetManager, questGiver, + statisticsDB, new ElementGeometryCreator(wayGeometrySource), questDB, singleChangeUpload); + } + + @Provides public static SplitWaysUpload splitWaysUpload( + MergedElementDao elementDB, ElementGeometryDao elementGeometryDB, + OpenQuestChangesetsManager changesetManager, OsmQuestGiver questGiver, + QuestStatisticsDao statisticsDB, OsmApiWayGeometrySource wayGeometrySource, + OsmQuestSplitWayDao questDB, SplitSingleWayUpload singleUpload) { + return new SplitWaysUpload(elementDB, elementGeometryDB, changesetManager, questGiver, + statisticsDB, new ElementGeometryCreator(wayGeometrySource), questDB, singleUpload); } @Provides public static OsmAvatarsDownload avatarsDownload(UserDao userDao, Context context) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/QuestController.java b/app/src/main/java/de/westnordost/streetcomplete/data/QuestController.java index 16e6c7b697..0db503479d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/QuestController.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/QuestController.java @@ -8,6 +8,8 @@ import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; + +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import android.util.Log; @@ -16,7 +18,6 @@ import java.util.List; import javax.inject.Inject; -import javax.inject.Provider; import de.westnordost.osmapi.map.data.LatLon; import de.westnordost.osmapi.map.data.OsmElement; @@ -24,17 +25,22 @@ import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao; import de.westnordost.streetcomplete.data.download.QuestDownloadService; import de.westnordost.streetcomplete.data.osm.OsmQuest; +import de.westnordost.streetcomplete.data.osm.OsmQuestSplitWay; +import de.westnordost.streetcomplete.data.osm.UndoOsmQuest; +import de.westnordost.streetcomplete.data.osm.changes.SplitPolylineAtPosition; import de.westnordost.streetcomplete.data.osm.changes.StringMapChanges; import de.westnordost.streetcomplete.data.osm.changes.StringMapChangesBuilder; import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayDao; import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestDao; import de.westnordost.streetcomplete.data.osmnotes.CreateNote; import de.westnordost.streetcomplete.data.osmnotes.CreateNoteDao; import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuest; import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestDao; import de.westnordost.streetcomplete.data.upload.QuestChangesUploadService; +import de.westnordost.streetcomplete.data.visiblequests.OrderedVisibleQuestTypesProvider; import de.westnordost.streetcomplete.quests.note_discussion.NoteAnswer; import de.westnordost.streetcomplete.util.SlippyMapMath; import de.westnordost.osmapi.map.data.BoundingBox; @@ -51,11 +57,12 @@ public class QuestController private final MergedElementDao osmElementDB; private final ElementGeometryDao geometryDB; private final OsmNoteQuestDao osmNoteQuestDB; + private final OsmQuestSplitWayDao splitWayDB; private final CreateNoteDao createNoteDB; private final OpenChangesetsDao openChangesetsDao; private final Context context; private final VisibleQuestRelay relay; - private final Provider> questTypesProvider; + private final OrderedVisibleQuestTypesProvider questTypesProvider; private boolean downloadServiceIsBound; private QuestDownloadService.Interface downloadService; @@ -93,15 +100,16 @@ public void onServiceDisconnected(ComponentName className) @Inject public QuestController(OsmQuestDao osmQuestDB, UndoOsmQuestDao undoOsmQuestDB, MergedElementDao osmElementDB, ElementGeometryDao geometryDB, - OsmNoteQuestDao osmNoteQuestDB, CreateNoteDao createNoteDB, - OpenChangesetsDao openChangesetsDao, - Provider> questTypesProvider, Context context) + OsmNoteQuestDao osmNoteQuestDB, OsmQuestSplitWayDao splitWayDB, + CreateNoteDao createNoteDB, OpenChangesetsDao openChangesetsDao, + OrderedVisibleQuestTypesProvider questTypesProvider, Context context) { this.osmQuestDB = osmQuestDB; this.undoOsmQuestDB = undoOsmQuestDB; this.osmElementDB = osmElementDB; this.geometryDB = geometryDB; this.osmNoteQuestDB = osmNoteQuestDB; + this.splitWayDB = splitWayDB; this.createNoteDB = createNoteDB; this.openChangesetsDao = openChangesetsDao; this.questTypesProvider = questTypesProvider; @@ -165,8 +173,24 @@ public boolean createNote(long osmQuestId, String questTitle, String text, @Null creation for other users, so those quests should be removed from the user's own display as well. As soon as the note is resolved, the quests will be re- created next time they are downloaded */ + removeQuestsForElement(q.getElementType(), q.getElementId()); + return true; + } + + public void createNote(String text, @Nullable List imagePaths, LatLon position) + { + CreateNote createNote = new CreateNote(); + createNote.position = position; + createNote.text = text; + createNote.imagePaths = imagePaths; + createNoteDB.add(createNote); + } + + private void removeQuestsForElement(Element.Type elementType, long elementId) { + + // TODO actually there should be a method in OsmQuestDB: deleteAllForElement or similar List questsForThisOsmElement = osmQuestDB.getAll(null, QuestStatus.NEW, null, - q.getElementType(), q.getElementId()); + elementType, elementId); List questIdsForThisOsmElement = new ArrayList<>(questsForThisOsmElement.size()); for(OsmQuest quest : questsForThisOsmElement) { @@ -178,16 +202,26 @@ public boolean createNote(long osmQuestId, String questTitle, String text, @Null osmElementDB.deleteUnreferenced(); geometryDB.deleteUnreferenced(); - return true; } - public void createNote(String text, @Nullable List imagePaths, LatLon position) + /** Split a way for the given OSM Quest. The quest will turn invisible. + * @return true if successful */ + public boolean splitWay(long osmQuestId, @NonNull List splits, @NonNull String source) { - CreateNote createNote = new CreateNote(); - createNote.position = position; - createNote.text = text; - createNote.imagePaths = imagePaths; - createNoteDB.add(createNote); + OsmQuest q = osmQuestDB.get(osmQuestId); + // race condition: another thread may have removed the element already (#288) + if(q == null || q.getStatus() != QuestStatus.NEW) return false; + + splitWayDB.put(new OsmQuestSplitWay( + osmQuestId, + q.getOsmElementQuestType(), + q.getElementId(), + source, + splits + )); + + removeQuestsForElement(q.getElementType(), q.getElementId()); + return true; } /** Apply the user's answer to the given quest. (The quest will turn invisible.) @@ -245,15 +279,7 @@ else if(quest.getStatus() == QuestStatus.CLOSED) { quest.setStatus(QuestStatus.REVERT); osmQuestDB.update(quest); - - OsmQuest reversedQuest = new OsmQuest( - quest.getOsmElementQuestType(), - quest.getElementType(), - quest.getElementId(), - quest.getGeometry()); - reversedQuest.setChanges(quest.getChanges().reversed(), quest.getChangesSource()); - reversedQuest.setStatus(QuestStatus.ANSWERED); - undoOsmQuestDB.add(reversedQuest); + undoOsmQuestDB.add(new UndoOsmQuest(quest)); } else { @@ -370,7 +396,7 @@ public void retrieve(BoundingBox bbox) private List getQuestTypeNames() { - List questTypes = questTypesProvider.get(); + List> questTypes = questTypesProvider.get(); List questTypeNames = new ArrayList<>(questTypes.size()); for (QuestType questType : questTypes) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/StreetCompleteOpenHelper.java b/app/src/main/java/de/westnordost/streetcomplete/data/StreetCompleteOpenHelper.java index ba8bfa25d4..24c4712965 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/StreetCompleteOpenHelper.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/StreetCompleteOpenHelper.java @@ -12,6 +12,8 @@ import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryTable; import de.westnordost.streetcomplete.data.osm.persist.NodeTable; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestTable; +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayTable; +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable; import de.westnordost.streetcomplete.data.osmnotes.CreateNoteTable; import de.westnordost.streetcomplete.data.osmnotes.NoteTable; import de.westnordost.streetcomplete.data.osm.persist.RelationTable; @@ -25,36 +27,38 @@ @Singleton public class StreetCompleteOpenHelper extends SQLiteOpenHelper { - public static final int DB_VERSION = 11; - - private static final String OSM_QUESTS_CREATE_PARAMS = " (" + - OsmQuestTable.Columns.QUEST_ID + " INTEGER PRIMARY KEY, " + - OsmQuestTable.Columns.QUEST_TYPE + " varchar(255) NOT NULL, " + - OsmQuestTable.Columns.QUEST_STATUS + " varchar(255) NOT NULL, " + - OsmQuestTable.Columns.TAG_CHANGES + " blob, " + // null if no changes - OsmQuestTable.Columns.CHANGES_SOURCE + " varchar(255), " + - OsmQuestTable.Columns.LAST_UPDATE + " int NOT NULL, " + - OsmQuestTable.Columns.ELEMENT_ID + " int NOT NULL, " + - OsmQuestTable.Columns.ELEMENT_TYPE + " varchar(255) NOT NULL, " + + public static final int DB_VERSION = 12; + + private static final String OSM_QUESTS_TABLE_CREATE_DB_VERSION_3 = + "CREATE TABLE " + OsmQuestTable.NAME + " (" + + OsmQuestTable.Columns.QUEST_ID + " INTEGER PRIMARY KEY, " + + OsmQuestTable.Columns.QUEST_TYPE + " varchar(255) NOT NULL, " + + OsmQuestTable.Columns.QUEST_STATUS + " varchar(255) NOT NULL, " + + OsmQuestTable.Columns.TAG_CHANGES + " blob, " + // null if no changes + OsmQuestTable.Columns.LAST_UPDATE + " int NOT NULL, " + + OsmQuestTable.Columns.ELEMENT_ID + " int NOT NULL, " + + OsmQuestTable.Columns.ELEMENT_TYPE + " varchar(255) NOT NULL, " + "CONSTRAINT same_osm_quest UNIQUE (" + - OsmQuestTable.Columns.QUEST_TYPE + ", " + - OsmQuestTable.Columns.ELEMENT_ID + ", " + - OsmQuestTable.Columns.ELEMENT_TYPE + + OsmQuestTable.Columns.QUEST_TYPE + ", " + + OsmQuestTable.Columns.ELEMENT_ID + ", " + + OsmQuestTable.Columns.ELEMENT_TYPE + "), " + "CONSTRAINT element_key FOREIGN KEY (" + - OsmQuestTable.Columns.ELEMENT_TYPE + ", " + OsmQuestTable.Columns.ELEMENT_ID + + OsmQuestTable.Columns.ELEMENT_TYPE + ", " + OsmQuestTable.Columns.ELEMENT_ID + ") REFERENCES " + ElementGeometryTable.NAME + " (" + - ElementGeometryTable.Columns.ELEMENT_TYPE + ", " + - ElementGeometryTable.Columns.ELEMENT_ID + + ElementGeometryTable.Columns.ELEMENT_TYPE + ", " + + ElementGeometryTable.Columns.ELEMENT_ID + ")" + ");"; - private static final String OSM_QUESTS_TABLE_CREATE_DB_VERSION_3 = + + private static final String OSM_QUESTS_TABLE_CREATE = "CREATE TABLE " + OsmQuestTable.NAME + " (" + OsmQuestTable.Columns.QUEST_ID + " INTEGER PRIMARY KEY, " + OsmQuestTable.Columns.QUEST_TYPE + " varchar(255) NOT NULL, " + OsmQuestTable.Columns.QUEST_STATUS + " varchar(255) NOT NULL, " + OsmQuestTable.Columns.TAG_CHANGES + " blob, " + // null if no changes + OsmQuestTable.Columns.CHANGES_SOURCE + " varchar(255), " + OsmQuestTable.Columns.LAST_UPDATE + " int NOT NULL, " + OsmQuestTable.Columns.ELEMENT_ID + " int NOT NULL, " + OsmQuestTable.Columns.ELEMENT_TYPE + " varchar(255) NOT NULL, " + @@ -71,12 +75,26 @@ public class StreetCompleteOpenHelper extends SQLiteOpenHelper ")" + ");"; - - private static final String OSM_QUESTS_TABLE_CREATE = - "CREATE TABLE " + OsmQuestTable.NAME + OSM_QUESTS_CREATE_PARAMS; - private static final String UNDO_OSM_QUESTS_TABLE_CREATE = - "CREATE TABLE " + OsmQuestTable.NAME_UNDO + OSM_QUESTS_CREATE_PARAMS; + "CREATE TABLE " + UndoOsmQuestTable.NAME + " (" + + UndoOsmQuestTable.Columns.QUEST_ID + " INTEGER PRIMARY KEY, " + + UndoOsmQuestTable.Columns.QUEST_TYPE + " varchar(255) NOT NULL, " + + UndoOsmQuestTable.Columns.TAG_CHANGES + " blob NOT NULL, " + + UndoOsmQuestTable.Columns.CHANGES_SOURCE + " varchar(255) NOT NULL, " + + UndoOsmQuestTable.Columns.ELEMENT_ID + " int NOT NULL, " + + UndoOsmQuestTable.Columns.ELEMENT_TYPE + " varchar(255) NOT NULL, " + + "CONSTRAINT same_osm_quest UNIQUE (" + + UndoOsmQuestTable.Columns.QUEST_TYPE + ", " + + UndoOsmQuestTable.Columns.ELEMENT_ID + ", " + + UndoOsmQuestTable.Columns.ELEMENT_TYPE + + "), " + + "CONSTRAINT element_key FOREIGN KEY (" + + UndoOsmQuestTable.Columns.ELEMENT_TYPE + ", " + UndoOsmQuestTable.Columns.ELEMENT_ID + + ") REFERENCES " + ElementGeometryTable.NAME + " (" + + ElementGeometryTable.Columns.ELEMENT_TYPE + ", " + + ElementGeometryTable.Columns.ELEMENT_ID + + ")" + + ");"; private static final String ELEMENTS_GEOMETRY_TABLE_CREATE = "CREATE TABLE " + ElementGeometryTable.NAME + @@ -101,13 +119,13 @@ public class StreetCompleteOpenHelper extends SQLiteOpenHelper ElementGeometryTable.Columns.ELEMENT_ID + ");"; - private static final String OSM_UNDO_QUESTS_VIEW_CREATE = - "CREATE VIEW " + OsmQuestTable.NAME_UNDO_MERGED_VIEW + " AS " + - "SELECT * FROM " + OsmQuestTable.NAME_UNDO + " " + - "INNER JOIN " + ElementGeometryTable.NAME + " USING (" + + private static final String UNDO_OSM_QUESTS_VIEW_CREATE = + "CREATE VIEW " + UndoOsmQuestTable.NAME_MERGED_VIEW + " AS " + + "SELECT * FROM " + UndoOsmQuestTable.NAME + " " + + "INNER JOIN " + ElementGeometryTable.NAME + " USING (" + ElementGeometryTable.Columns.ELEMENT_TYPE + ", " + ElementGeometryTable.Columns.ELEMENT_ID + - ");"; + ");"; private static final String OSM_NOTES_QUESTS_TABLE_CREATE = "CREATE TABLE " + OsmNoteQuestTable.NAME + @@ -204,8 +222,8 @@ public class StreetCompleteOpenHelper extends SQLiteOpenHelper "CREATE TABLE " + OpenChangesetsTable.NAME + " (" + OpenChangesetsTable.Columns.QUEST_TYPE + " varchar(255), " + - OpenChangesetsTable.Columns.SOURCE + " varchar(255), " + - OpenChangesetsTable.Columns.CHANGESET_ID + " int NOT NULL, " + + OpenChangesetsTable.Columns.SOURCE + " varchar(255), " + + OpenChangesetsTable.Columns.CHANGESET_ID + " int NOT NULL, " + "CONSTRAINT primary_key PRIMARY KEY (" + OpenChangesetsTable.Columns.QUEST_TYPE + ", " + OpenChangesetsTable.Columns.SOURCE + @@ -219,6 +237,16 @@ public class StreetCompleteOpenHelper extends SQLiteOpenHelper QuestVisibilityTable.Columns.VISIBILITY + " int NOT NULL " + ");"; + private static final String SPLIT_WAYS_TABLE_CREATE = + "CREATE TABLE " + OsmQuestSplitWayTable.NAME + + " (" + + OsmQuestSplitWayTable.Columns.QUEST_ID + " int PRIMARY KEY, " + + OsmQuestSplitWayTable.Columns.QUEST_TYPE + " varchar(255) NOT NULL, " + + OsmQuestSplitWayTable.Columns.WAY_ID + " int NOT NULL, " + + OsmQuestSplitWayTable.Columns.SPLITS + " blob NOT NULL, " + + OsmQuestSplitWayTable.Columns.SOURCE + " varchar(255) NOT NULL " + + ");"; + private final TablesHelper[] extensions; public StreetCompleteOpenHelper(Context context, String dbName, TablesHelper[] extensions) @@ -247,13 +275,15 @@ public void onCreate(SQLiteDatabase db) db.execSQL(DOWNLOADED_TILES_TABLE_CREATE); db.execSQL(OSM_QUESTS_VIEW_CREATE); - db.execSQL(OSM_UNDO_QUESTS_VIEW_CREATE); + db.execSQL(UNDO_OSM_QUESTS_VIEW_CREATE); db.execSQL(OSM_NOTES_VIEW_CREATE); db.execSQL(OPEN_CHANGESETS_TABLE_CREATE); db.execSQL(QUEST_VISIBILITY_TABLE_CREATE); + db.execSQL(SPLIT_WAYS_TABLE_CREATE); + for (TablesHelper extension : extensions) { extension.onCreate(db); @@ -311,7 +341,7 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) if(oldVersion < 7 && newVersion >= 7) { db.execSQL(UNDO_OSM_QUESTS_TABLE_CREATE); - db.execSQL(OSM_UNDO_QUESTS_VIEW_CREATE); + db.execSQL(UNDO_OSM_QUESTS_VIEW_CREATE); } if(oldVersion < 8 && newVersion >= 8) @@ -333,9 +363,18 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) String where = OsmQuestTable.Columns.QUEST_TYPE + " = ?"; String[] args = {AddOneway.class.getSimpleName()}; db.delete(OsmQuestTable.NAME, where, args); - db.delete(OsmQuestTable.NAME_UNDO, where, args); + db.delete(UndoOsmQuestTable.NAME, where, args); } + if(oldVersion < 12 && newVersion >= 12) + { + db.execSQL(SPLIT_WAYS_TABLE_CREATE); + // slightly different structure for undo osm quest table. Isn't worth converting + db.execSQL("DROP TABLE " + UndoOsmQuestTable.NAME); + db.execSQL("DROP VIEW " + UndoOsmQuestTable.NAME_MERGED_VIEW); + db.execSQL(UNDO_OSM_QUESTS_TABLE_CREATE); + db.execSQL(UNDO_OSM_QUESTS_VIEW_CREATE); + } // for later changes to the DB // ... @@ -345,20 +384,13 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) } } - private static boolean tableHasColumn(SQLiteDatabase db, String tableName, String columnName) { - try (Cursor cursor = db.rawQuery("PRAGMA table_info(" + tableName + ")", null)) { - if (cursor.moveToFirst()) - { - while (!cursor.isAfterLast()) - { - String name = cursor.getString(cursor.getColumnIndexOrThrow("name")); - if (columnName.equals(name)) return true; - cursor.moveToNext(); - } + while(cursor.moveToNext()) { + String name = cursor.getString(cursor.getColumnIndexOrThrow("name")); + if (columnName.equals(name)) return true; } } return false; diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/download/AActiveRadiusStrategy.java b/app/src/main/java/de/westnordost/streetcomplete/data/download/AActiveRadiusStrategy.java index ca24f52586..df8611daf0 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/download/AActiveRadiusStrategy.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/download/AActiveRadiusStrategy.java @@ -6,13 +6,12 @@ import java.util.ArrayList; import java.util.List; -import javax.inject.Provider; - import de.westnordost.streetcomplete.ApplicationConstants; import de.westnordost.streetcomplete.data.QuestStatus; import de.westnordost.streetcomplete.data.QuestType; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; +import de.westnordost.streetcomplete.data.visiblequests.OrderedVisibleQuestTypesProvider; import de.westnordost.streetcomplete.util.SlippyMapMath; import de.westnordost.streetcomplete.util.SphericalEarthMath; import de.westnordost.osmapi.map.data.BoundingBox; @@ -26,11 +25,11 @@ public abstract class AActiveRadiusStrategy implements QuestAutoDownloadStrategy private final OsmQuestDao osmQuestDB; private final DownloadedTilesDao downloadedTilesDao; - private final Provider> questTypesProvider; + private final OrderedVisibleQuestTypesProvider questTypesProvider; public AActiveRadiusStrategy( OsmQuestDao osmQuestDB, DownloadedTilesDao downloadedTilesDao, - Provider> questTypesProvider) + OrderedVisibleQuestTypesProvider questTypesProvider) { this.osmQuestDB = osmQuestDB; this.downloadedTilesDao = downloadedTilesDao; @@ -68,7 +67,7 @@ private boolean mayDownloadHere(LatLon pos, int radius, List questTypeNa private List getQuestTypeNames() { - List questTypes = questTypesProvider.get(); + List> questTypes = questTypesProvider.get(); List questTypeNames = new ArrayList<>(questTypes.size()); for (QuestType questType : questTypes) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/download/MobileDataAutoDownloadStrategy.java b/app/src/main/java/de/westnordost/streetcomplete/data/download/MobileDataAutoDownloadStrategy.java index ed75fd3724..f3535c3d1d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/download/MobileDataAutoDownloadStrategy.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/download/MobileDataAutoDownloadStrategy.java @@ -1,19 +1,15 @@ package de.westnordost.streetcomplete.data.download; -import java.util.List; - import javax.inject.Inject; -import javax.inject.Provider; - -import de.westnordost.streetcomplete.data.QuestType; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; +import de.westnordost.streetcomplete.data.visiblequests.OrderedVisibleQuestTypesProvider; public class MobileDataAutoDownloadStrategy extends AActiveRadiusStrategy { @Inject public MobileDataAutoDownloadStrategy(OsmQuestDao osmQuestDB, DownloadedTilesDao downloadedTilesDao, - Provider> questTypesProvider) + OrderedVisibleQuestTypesProvider questTypesProvider) { super(osmQuestDB, downloadedTilesDao, questTypesProvider); } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/download/QuestDownload.java b/app/src/main/java/de/westnordost/streetcomplete/data/download/QuestDownload.java index 000c55157b..b3a3eabf33 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/download/QuestDownload.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/download/QuestDownload.java @@ -25,6 +25,7 @@ import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestType; import de.westnordost.streetcomplete.data.osmnotes.OsmNotesDownload; import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; +import de.westnordost.streetcomplete.data.visiblequests.OrderedVisibleQuestTypesProvider; import de.westnordost.streetcomplete.util.SlippyMapMath; import de.westnordost.osmapi.map.data.BoundingBox; import de.westnordost.osmapi.map.data.LatLon; @@ -36,7 +37,7 @@ public class QuestDownload private final Provider notesDownloadProvider; private final Provider questDownloadProvider; private final QuestTypeRegistry questTypeRegistry; - private final Provider> questTypesProvider; + private final OrderedVisibleQuestTypesProvider questTypesProvider; private final SharedPreferences prefs; private final DownloadedTilesDao downloadedTilesDao; private final OsmNoteQuestDao osmNoteQuestDb; @@ -60,7 +61,7 @@ public class QuestDownload DownloadedTilesDao downloadedTilesDao, OsmNoteQuestDao osmNoteQuestDb, QuestTypeRegistry questTypeRegistry, SharedPreferences prefs, - Provider> questTypesProvider) + OrderedVisibleQuestTypesProvider questTypesProvider) { this.notesDownloadProvider = notesDownloadProvider; this.questDownloadProvider = questDownloadProvider; diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/download/WifiAutoDownloadStrategy.java b/app/src/main/java/de/westnordost/streetcomplete/data/download/WifiAutoDownloadStrategy.java index efea417168..0da7d03fb4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/download/WifiAutoDownloadStrategy.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/download/WifiAutoDownloadStrategy.java @@ -1,19 +1,17 @@ package de.westnordost.streetcomplete.data.download; -import java.util.List; import javax.inject.Inject; -import javax.inject.Provider; -import de.westnordost.streetcomplete.data.QuestType; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; +import de.westnordost.streetcomplete.data.visiblequests.OrderedVisibleQuestTypesProvider; public class WifiAutoDownloadStrategy extends AActiveRadiusStrategy { @Inject public WifiAutoDownloadStrategy( OsmQuestDao osmQuestDB, DownloadedTilesDao downloadedTilesDao, - Provider> questTypes) + OrderedVisibleQuestTypesProvider questTypes) { super(osmQuestDB, downloadedTilesDao, questTypes); } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmElementKey.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/ElementKey.java similarity index 69% rename from app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmElementKey.java rename to app/src/main/java/de/westnordost/streetcomplete/data/osm/ElementKey.java index d287ead358..89dbfb2b23 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmElementKey.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/ElementKey.java @@ -1,13 +1,13 @@ -package de.westnordost.streetcomplete.data.osm.persist; +package de.westnordost.streetcomplete.data.osm; import de.westnordost.osmapi.map.data.Element; -public class OsmElementKey +public class ElementKey { private Element.Type elementType; private long elementId; - public OsmElementKey(Element.Type elementType, long elementId) + public ElementKey(Element.Type elementType, long elementId) { this.elementType = elementType; this.elementId = elementId; @@ -26,9 +26,9 @@ public long getElementId() @Override public boolean equals(Object o) { if (this == o) return true; - if (o == null || !(o instanceof OsmElementKey)) return false; + if (o == null || !(o instanceof ElementKey)) return false; - OsmElementKey that = (OsmElementKey) o; + ElementKey that = (ElementKey) o; return elementType == that.elementType && diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmElementQuestType.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmElementQuestType.kt index fc08259de4..25667ad753 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmElementQuestType.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmElementQuestType.kt @@ -25,6 +25,9 @@ interface OsmElementQuestType : QuestType { /** returns whether the markers should be at the ends instead of the center */ val hasMarkersAtEnds: Boolean get() = false + /** returns whether the user should be able to split the way instead */ + val isSplitWayEnabled: Boolean get() = false + /** returns title resource for when the element has the specified [tags]. The tags are unmodifiable */ fun getTitle(tags: Map): Int diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuest.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuest.java index 616ec95e0a..5ab4f30fe2 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuest.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuest.java @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.data.osm; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import java.util.Date; @@ -11,10 +12,12 @@ import de.westnordost.streetcomplete.data.QuestType; import de.westnordost.osmapi.map.data.Element; import de.westnordost.osmapi.map.data.LatLon; +import de.westnordost.streetcomplete.data.osm.upload.HasElementTagChanges; +import de.westnordost.streetcomplete.data.osm.upload.UploadableInChangeset; import de.westnordost.streetcomplete.util.SphericalEarthMath; /** Represents one task for the user to complete/correct the data based on one OSM element */ -public class OsmQuest implements Quest +public class OsmQuest implements Quest, UploadableInChangeset, HasElementTagChanges { public OsmQuest(OsmElementQuestType type, Element.Type elementType, long elementId, ElementGeometry geometry) @@ -22,7 +25,7 @@ public OsmQuest(OsmElementQuestType type, Element.Type elementType, long element this(null, type, elementType, elementId, QuestStatus.NEW, null, null, new Date(), geometry); } - public OsmQuest(Long id, OsmElementQuestType type, Element.Type elementType, long elementId, + public OsmQuest(@Nullable Long id, OsmElementQuestType type, Element.Type elementType, long elementId, QuestStatus status, @Nullable StringMapChanges changes, @Nullable String changesSource, Date lastUpdate, ElementGeometry geometry) { @@ -37,7 +40,7 @@ public OsmQuest(Long id, OsmElementQuestType type, Element.Type elementType, lon this.lastUpdate = lastUpdate; } - private Long id; + @Nullable private Long id; private final OsmElementQuestType type; private QuestStatus status; private final ElementGeometry geometry; @@ -48,12 +51,12 @@ public OsmQuest(Long id, OsmElementQuestType type, Element.Type elementType, lon // and the changes to the tags (in the future, streetcomplete should probably be able to edit more // than just tags -> osmchange?) - private StringMapChanges changes; - private String changesSource; + @Nullable private StringMapChanges changes; + @Nullable private String changesSource; private Date lastUpdate; - @Override public Long getId() + @Override @Nullable public Long getId() { return id; } @@ -105,12 +108,12 @@ public OsmElementQuestType getOsmElementQuestType() return geometry; } - public StringMapChanges getChanges() + @Nullable public StringMapChanges getChanges() { return changes; } - public String getChangesSource() { return changesSource; } + @Nullable public String getChangesSource() { return changesSource; } public void setChanges(StringMapChanges changes, String source) { @@ -137,4 +140,12 @@ public void setChanges(StringMapChanges changes, String source) { this.id = id; } + + public Boolean isApplicableTo(@NonNull Element element) { + return type.isApplicableTo(element); + } + + /* --------------------------- UploadableInChangeset --------------------------- */ + + @Override public String getSource() { return changesSource; } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestGiver.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestGiver.java deleted file mode 100644 index 8addb1ae6f..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestGiver.java +++ /dev/null @@ -1,136 +0,0 @@ -package de.westnordost.streetcomplete.data.osm; - -import android.text.TextUtils; -import android.util.Log; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import javax.inject.Inject; -import javax.inject.Provider; - -import de.westnordost.osmapi.map.data.BoundingBox; -import de.westnordost.osmapi.map.data.Element; -import de.westnordost.osmapi.map.data.LatLon; -import de.westnordost.streetcomplete.data.QuestStatus; -import de.westnordost.streetcomplete.data.QuestType; -import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; -import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; -import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestDao; -import de.westnordost.streetcomplete.util.SphericalEarthMath; - -/** Manages creating new quests and removing quests that are no longer applicable for an OSM - * element locally */ -public class OsmQuestGiver -{ - private static final String TAG = "OsmQuestGiver"; - - private final OsmNoteQuestDao osmNoteQuestDb; - private final OsmQuestDao questDB; - private final ElementGeometryDao elementGeometryDB; - private final Provider> questTypesProvider; - - @Inject public OsmQuestGiver( - OsmNoteQuestDao osmNoteQuestDb, OsmQuestDao questDB, - ElementGeometryDao elementGeometryDB, Provider> questTypesProvider) - { - this.osmNoteQuestDb = osmNoteQuestDb; - this.questDB = questDB; - this.elementGeometryDB = elementGeometryDB; - this.questTypesProvider = questTypesProvider; - } - - public static class QuestUpdates - { - public final List createdQuests = new ArrayList<>(); - public final List removedQuestIds = new ArrayList<>(); - } - - public QuestUpdates updateQuests(Element element) - { - QuestUpdates result = new QuestUpdates(); - - ElementGeometry geometry = elementGeometryDB.get(element.getType(), element.getId()); - if(geometry == null) return result; - - boolean hasNote = hasNoteAt(geometry.center); - - Map currentQuests = getCurrentQuests(element); - List createdQuestsLog = new ArrayList<>(); - List removedQuestsLog = new ArrayList<>(); - - for(QuestType questType : questTypesProvider.get()) - { - if(!(questType instanceof OsmElementQuestType)) continue; - OsmElementQuestType osmQuestType = (OsmElementQuestType)questType; - - Boolean appliesToElement = osmQuestType.isApplicableTo(element); - if(appliesToElement == null) continue; - - boolean hasQuest = currentQuests.containsKey(osmQuestType); - if(appliesToElement && !hasQuest && !hasNote) - { - OsmQuest quest = new OsmQuest(osmQuestType, element.getType(), element.getId(), geometry); - result.createdQuests.add(quest); - createdQuestsLog.add(osmQuestType.getClass().getSimpleName()); - } - if(!appliesToElement && hasQuest) - { - OsmQuest quest = currentQuests.get(osmQuestType); - // only remove "fresh" unanswered quests because answered/closed quests by definition - // do not apply to the element anymore. E.g. after adding the name to the street, - // there shan't be any AddRoadName quest for that street anymore - if(quest.getStatus() == QuestStatus.NEW) - { - result.removedQuestIds.add(quest.getId()); - removedQuestsLog.add(osmQuestType.getClass().getSimpleName()); - } - } - } - - if(!result.createdQuests.isEmpty()) - { - // Before new quests are unlocked, all reverted quests need to be removed for - // this element so that they can be created anew as the case may be - questDB.deleteAllReverted(element.getType(), element.getId()); - - questDB.addAll(result.createdQuests); - - Log.d(TAG, "Created new quests for " + element.getType().name() + "#" + element.getId() + ": " + - TextUtils.join(", ", createdQuestsLog) - ); - } - if(!result.removedQuestIds.isEmpty()) - { - questDB.deleteAll(result.removedQuestIds); - - Log.d(TAG, "Removed quests no longer applicable for " + element.getType().name() + "#" + element.getId() + ": " + - TextUtils.join(", ", removedQuestsLog) - ); - } - - return result; - } - - private boolean hasNoteAt(LatLon pos) - { - // note about one meter around the center of an element still count as at this point as to - // deal with imprecision of the center calculation of geometry (see #1089) - BoundingBox bbox = SphericalEarthMath.enclosingBoundingBox(pos, 1); - return !osmNoteQuestDb.getAllPositions(bbox).isEmpty(); - } - - private Map getCurrentQuests(Element element) - { - List quests = questDB.getAll(null, null, null, element.getType(), element.getId()); - Map result = new HashMap<>(quests.size()); - for (OsmQuest quest : quests) - { - if(quest.getStatus() == QuestStatus.REVERT) continue; - result.put(quest.getType(), quest); - } - return result; - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestGiver.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestGiver.kt new file mode 100644 index 0000000000..020838103f --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestGiver.kt @@ -0,0 +1,110 @@ +package de.westnordost.streetcomplete.data.osm + +import android.util.Log + + +import javax.inject.Inject + +import de.westnordost.osmapi.map.data.Element +import de.westnordost.osmapi.map.data.LatLon +import de.westnordost.streetcomplete.data.QuestStatus +import de.westnordost.streetcomplete.data.QuestType +import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao +import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestDao +import de.westnordost.streetcomplete.data.visiblequests.OrderedVisibleQuestTypesProvider +import de.westnordost.streetcomplete.util.SphericalEarthMath +import javax.inject.Named +import javax.inject.Provider + +/** Manages creating new quests and removing quests that are no longer applicable for an OSM + * element locally */ +class OsmQuestGiver @Inject constructor( + private val osmNoteQuestDb: OsmNoteQuestDao, + private val questDB: OsmQuestDao, + private val elementGeometryDB: ElementGeometryDao, + private val questTypesProvider: OrderedVisibleQuestTypesProvider +) { + + private val TAG = "OsmQuestGiver" + + class QuestUpdates { + val createdQuests: MutableList = ArrayList() + val removedQuestIds: MutableList = ArrayList() + } + + fun updateQuests(element: Element): QuestUpdates { + val result = QuestUpdates() + + val geometry = elementGeometryDB.get(element.type, element.id) ?: return result + + val hasNote = hasNoteAt(geometry.center) + + val currentQuests = getCurrentQuests(element) + val createdQuestsLog = ArrayList() + val removedQuestsLog = ArrayList() + + for (questType in questTypesProvider.get()) { + if (questType !is OsmElementQuestType<*>) continue + + val appliesToElement = questType.isApplicableTo(element) ?: continue + + val hasQuest = currentQuests.containsKey(questType) + if (appliesToElement && !hasQuest && !hasNote) { + val quest = OsmQuest(questType, element.type, element.id, geometry) + result.createdQuests.add(quest) + createdQuestsLog.add(questType.javaClass.simpleName) + } + if (!appliesToElement && hasQuest) { + val quest = currentQuests.getValue(questType) + // only remove "fresh" unanswered quests because answered/closed quests by definition + // do not apply to the element anymore. E.g. after adding the name to the street, + // there shan't be any AddRoadName quest for that street anymore + if (quest.status == QuestStatus.NEW) { + result.removedQuestIds.add(quest.id!!) + removedQuestsLog.add(questType.javaClass.simpleName) + } + } + } + + if (result.createdQuests.isNotEmpty()) { + // Before new quests are unlocked, all reverted quests need to be removed for + // this element so that they can be created anew as the case may be + questDB.deleteAllReverted(element.type, element.id) + + questDB.addAll(result.createdQuests) + Log.d(TAG, "Created new quests for ${element.type.name}#${element.id}: ${createdQuestsLog.joinToString()}") + } + if (result.removedQuestIds.isNotEmpty()) { + questDB.deleteAll(result.removedQuestIds) + Log.d(TAG, "Removed quests no longer applicable for ${element.type.name}#${element.id}: ${removedQuestsLog.joinToString()}") + } + + return result + } + + fun deleteQuests(elementType: Element.Type, elementId: Long): List { + val ids = questDB.getAllIds(elementType, elementId) + questDB.deleteAll(ids) + + Log.d(TAG, "Removed all quests for deleted element " + elementType.name + "#" + elementId) + return ids + } + + private fun hasNoteAt(pos: LatLon): Boolean { + // note about one meter around the center of an element still count as at this point as to + // deal with imprecision of the center calculation of geometry (see #1089) + val bbox = SphericalEarthMath.enclosingBoundingBox(pos, 1.0) + return osmNoteQuestDb.getAllPositions(bbox).isNotEmpty() + } + + private fun getCurrentQuests(element: Element): Map, OsmQuest> { + val quests = questDB.getAll(null, null, null, element.type, element.id) + val result = HashMap, OsmQuest>(quests.size) + for (quest in quests) { + if (quest.status == QuestStatus.REVERT) continue + result[quest.type] = quest + } + return result + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestSplitWay.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestSplitWay.kt new file mode 100644 index 0000000000..71acbf819e --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/OsmQuestSplitWay.kt @@ -0,0 +1,17 @@ +package de.westnordost.streetcomplete.data.osm + +import de.westnordost.osmapi.map.data.Element +import de.westnordost.streetcomplete.data.osm.changes.SplitPolylineAtPosition +import de.westnordost.streetcomplete.data.osm.upload.UploadableInChangeset + +data class OsmQuestSplitWay( + val questId: Long, + val questType: OsmElementQuestType<*>, + val wayId: Long, + override val source: String, + val splits: List) : UploadableInChangeset { + + override val osmElementQuestType get() = questType + override val elementType get() = Element.Type.WAY + override val elementId get() = wayId +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/UndoOsmQuest.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/UndoOsmQuest.kt new file mode 100644 index 0000000000..ccacb7b184 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/UndoOsmQuest.kt @@ -0,0 +1,29 @@ +package de.westnordost.streetcomplete.data.osm + +import de.westnordost.osmapi.map.data.Element +import de.westnordost.streetcomplete.data.osm.changes.StringMapChanges +import de.westnordost.streetcomplete.data.osm.upload.HasElementTagChanges +import de.westnordost.streetcomplete.data.osm.upload.UploadableInChangeset + +class UndoOsmQuest( + val id: Long?, + val type: OsmElementQuestType<*>, + override val elementType: Element.Type, + override val elementId: Long, + override val changes: StringMapChanges, + val changesSource: String, + val geometry: ElementGeometry +) : UploadableInChangeset, HasElementTagChanges { + + constructor(quest: OsmQuest) : this( + null, quest.osmElementQuestType, quest.elementType, quest.elementId, + quest.changes!!.reversed(), quest.changesSource!!, quest.geometry) + + /* can't ask the quest here if it is applicable to the element or not, because the change + of the revert is exactly the opposite of what the quest would normally change and the + element ergo has the changes already applied that a normal quest would add */ + override fun isApplicableTo(element: Element) = true + + override val source get() = changesSource + override val osmElementQuestType get() = type +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/changes/SplitPolylineAtPosition.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/changes/SplitPolylineAtPosition.kt new file mode 100644 index 0000000000..d3a9945835 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/changes/SplitPolylineAtPosition.kt @@ -0,0 +1,23 @@ +package de.westnordost.streetcomplete.data.osm.changes + +import de.westnordost.osmapi.map.data.LatLon +import de.westnordost.osmapi.map.data.OsmLatLon +import de.westnordost.streetcomplete.util.SphericalEarthMath.distance +import de.westnordost.streetcomplete.util.SphericalEarthMath.pointOnPolylineFromStart + +sealed class SplitPolylineAtPosition { + abstract val pos: LatLon +} + +data class SplitAtPoint(override val pos: OsmLatLon) : SplitPolylineAtPosition() + +data class SplitAtLinePosition(val pos1: OsmLatLon, val pos2: OsmLatLon, val delta: Double) : SplitPolylineAtPosition() { + override val pos: LatLon get() { + val line = listOf(pos1, pos2) + return pointOnPolylineFromStart(line, distance(line) * delta)!! + } + init { + if(delta <= 0 || delta >= 1) + throw IllegalArgumentException("Delta must be between 0 and 1 (both exclusive)") + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/changes/StringMapChanges.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/changes/StringMapChanges.java index 5444c17240..94d2524aa7 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/changes/StringMapChanges.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/changes/StringMapChanges.java @@ -13,7 +13,7 @@ * it. A StringMapChanges is immutable. */ public class StringMapChanges { - private final List changes; + private final ArrayList changes; public List getChanges() { @@ -22,7 +22,7 @@ public List getChanges() public StringMapChanges(@NonNull List changes) { - this.changes = changes; + this.changes = new ArrayList<>(changes); } /** @return a StringMapChanges that exactly reverses this StringMapChanges */ diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/download/OsmQuestDownload.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/download/OsmQuestDownload.java index f1a80527c3..01615b8342 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/download/OsmQuestDownload.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/download/OsmQuestDownload.java @@ -28,7 +28,7 @@ import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; -import de.westnordost.streetcomplete.data.osm.persist.OsmElementKey; +import de.westnordost.streetcomplete.data.osm.ElementKey; import de.westnordost.osmapi.map.data.BoundingBox; import de.westnordost.osmapi.map.data.Element; import de.westnordost.osmapi.map.data.LatLon; @@ -76,9 +76,9 @@ public boolean download(final OsmElementQuestType questType, BoundingBox bbox, Log.i(TAG, getQuestTypeName(questType) + ": Starting"); final ArrayList geometryRows = new ArrayList<>(); - final Map elements = new HashMap<>(); + final Map elements = new HashMap<>(); final ArrayList quests = new ArrayList<>(); - final Map previousQuests = getPreviousQuestsIdsByElementKey(questType, bbox); + final Map previousQuests = getPreviousQuestsIdsByElementKey(questType, bbox); final HashSet truncatedBlacklistedPositions = new HashSet<>(); for (LatLon blacklistedPosition : blacklistedPositions) @@ -99,7 +99,7 @@ public boolean download(final OsmElementQuestType questType, BoundingBox bbox, geometryRows.add(new ElementGeometryDao.Row( elementType, elementId, quest.getGeometry())); quests.add(quest); - OsmElementKey elementKey = new OsmElementKey(elementType, elementId); + ElementKey elementKey = new ElementKey(elementType, elementId); elements.put(elementKey, element); previousQuests.remove(elementKey); } @@ -149,14 +149,14 @@ public boolean download(final OsmElementQuestType questType, BoundingBox bbox, return true; } - private Map getPreviousQuestsIdsByElementKey( + private Map getPreviousQuestsIdsByElementKey( OsmElementQuestType questType, BoundingBox bbox) { String questTypeName = questType.getClass().getSimpleName(); - Map result = new HashMap<>(); + Map result = new HashMap<>(); for(OsmQuest quest : osmQuestDB.getAll(bbox, null, questTypeName, null, null)) { - result.put(new OsmElementKey(quest.getElementType(), quest.getElementId()), quest.getId()); + result.put(new ElementKey(quest.getElementType(), quest.getElementId()), quest.getId()); } return result; } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/AOsmElementDao.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/AOsmElementDao.java index 23ac19e139..5e736d0581 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/AOsmElementDao.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/AOsmElementDao.java @@ -10,7 +10,7 @@ public abstract class AOsmElementDao { - private final SQLiteOpenHelper dbHelper; + protected final SQLiteOpenHelper dbHelper; public AOsmElementDao(SQLiteOpenHelper dbHelper) { @@ -62,16 +62,16 @@ public T get(long id) public void deleteUnreferenced() { SQLiteDatabase db = dbHelper.getWritableDatabase(); - String where = NodeTable.Columns.ID + " NOT IN ( " + + String where = getIdColumnName() + " NOT IN ( " + getSelectAllElementIdsIn(OsmQuestTable.NAME) + " UNION " + - getSelectAllElementIdsIn(OsmQuestTable.NAME_UNDO) + + getSelectAllElementIdsIn(UndoOsmQuestTable.NAME) + ")"; db.delete(getTableName(), where, null); } - private String getSelectAllElementIdsIn(String table) + protected String getSelectAllElementIdsIn(String table) { return "SELECT " + OsmQuestTable.Columns.ELEMENT_ID + " AS " + getIdColumnName() + " FROM " + table + diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/ElementGeometryDao.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/ElementGeometryDao.java index 44785120f8..2817e66d2d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/ElementGeometryDao.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/ElementGeometryDao.java @@ -162,7 +162,7 @@ public int deleteUnreferenced() ") NOT IN ( " + getSelectAllElementsIn(OsmQuestTable.NAME) + " UNION " + - getSelectAllElementsIn(OsmQuestTable.NAME_UNDO) + + getSelectAllElementsIn(UndoOsmQuestTable.NAME) + ")"; return db.delete(ElementGeometryTable.NAME, where, null); diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayDao.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayDao.kt new file mode 100644 index 0000000000..23a5859fab --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayDao.kt @@ -0,0 +1,66 @@ +package de.westnordost.streetcomplete.data.osm.persist + +import android.content.ContentValues +import android.database.Cursor +import android.database.sqlite.SQLiteDatabase.CONFLICT_REPLACE +import android.database.sqlite.SQLiteOpenHelper +import de.westnordost.streetcomplete.data.QuestTypeRegistry +import de.westnordost.streetcomplete.data.osm.OsmElementQuestType +import de.westnordost.streetcomplete.data.osm.OsmQuestSplitWay +import de.westnordost.streetcomplete.data.osm.changes.SplitPolylineAtPosition +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayTable.NAME +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayTable.Columns.QUEST_ID +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayTable.Columns.QUEST_TYPE +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayTable.Columns.SOURCE +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayTable.Columns.SPLITS +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayTable.Columns.WAY_ID +import de.westnordost.streetcomplete.ktx.* +import de.westnordost.streetcomplete.util.Serializer +import javax.inject.Inject +import kotlin.collections.ArrayList + +class OsmQuestSplitWayDao @Inject constructor( + private val dbHelper: SQLiteOpenHelper, + private val serializer: Serializer, + private val questTypeList: QuestTypeRegistry +) { + private val db get() = dbHelper.writableDatabase + + fun getAll(): List { + return db.query(NAME) { it.createOsmQuestSplitWay() } + } + + fun get(questId: Long): OsmQuestSplitWay? { + val selection = "$QUEST_ID = ?" + val args = arrayOf(questId.toString()) + return db.queryOne(NAME, null, selection, args) { it.createOsmQuestSplitWay() } + } + + fun getCount(): Int { + return db.queryOne(NAME, arrayOf("COUNT(*)")) { it.getInt(0) } ?: 0 + } + + fun put(quest: OsmQuestSplitWay) { + db.insertWithOnConflict(NAME, null, quest.createContentValues(), CONFLICT_REPLACE) + } + + fun delete(questId: Long) { + db.delete(NAME, "$QUEST_ID = $questId", null) + } + + private fun OsmQuestSplitWay.createContentValues() = ContentValues().also { v -> + v.put(QUEST_ID, questId) + v.put(QUEST_TYPE, questType.javaClass.simpleName) + v.put(WAY_ID, wayId) + v.put(SOURCE, source) + v.put(SPLITS, serializer.toBytes(ArrayList(splits))) + } + + private fun Cursor.createOsmQuestSplitWay() = OsmQuestSplitWay( + getLong(QUEST_ID), + questTypeList.getByName(getString(QUEST_TYPE)) as OsmElementQuestType<*>, + getLong(WAY_ID), + getString(SOURCE), + (serializer.toObject(getBlob(SPLITS)) as ArrayList) + ) +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayTable.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayTable.kt new file mode 100644 index 0000000000..48d036218f --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestSplitWayTable.kt @@ -0,0 +1,13 @@ +package de.westnordost.streetcomplete.data.osm.persist + +object OsmQuestSplitWayTable { + const val NAME = "osm_split_ways" + + object Columns { + const val QUEST_ID = "quest_id" + const val QUEST_TYPE = "quest_type" + const val WAY_ID = "way_id" + const val SPLITS = "splits" + const val SOURCE = "source" + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestTable.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestTable.java index f31010aeb5..66b316c00e 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestTable.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/OsmQuestTable.java @@ -9,10 +9,6 @@ public class OsmQuestTable public static final String NAME = "osm_quests"; public static final String NAME_MERGED_VIEW = "osm_quests_full"; - public static final String NAME_UNDO = "osm_quests_undo"; - public static final String NAME_UNDO_MERGED_VIEW = "osm_quests_full_undo"; - - public static class Columns { public static final String diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDao.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDao.java deleted file mode 100644 index 33becc3bb1..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDao.java +++ /dev/null @@ -1,21 +0,0 @@ -package de.westnordost.streetcomplete.data.osm.persist; - -import android.database.sqlite.SQLiteOpenHelper; - -import javax.inject.Inject; - -import de.westnordost.streetcomplete.data.QuestTypeRegistry; -import de.westnordost.streetcomplete.util.Serializer; - -/** Same as OsmQuestDao, only operates on a different table to have a clear cut between - * "real" quests and reversed quests used to revert changes made by another quest */ -public class UndoOsmQuestDao extends AOsmQuestDao -{ - @Inject public UndoOsmQuestDao(SQLiteOpenHelper dbHelper, Serializer serializer, QuestTypeRegistry questTypeList) - { - super(dbHelper, serializer, questTypeList); - } - - @Override protected String getTableName() { return OsmQuestTable.NAME_UNDO; } - @Override protected String getMergedViewName() { return OsmQuestTable.NAME_UNDO_MERGED_VIEW; } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDao.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDao.kt new file mode 100644 index 0000000000..5ede98196f --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestDao.kt @@ -0,0 +1,67 @@ +package de.westnordost.streetcomplete.data.osm.persist + +import android.content.ContentValues +import android.database.Cursor +import android.database.sqlite.SQLiteOpenHelper +import de.westnordost.osmapi.map.data.Element + +import javax.inject.Inject + +import de.westnordost.streetcomplete.data.QuestTypeRegistry +import de.westnordost.streetcomplete.data.osm.OsmElementQuestType +import de.westnordost.streetcomplete.data.osm.UndoOsmQuest +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.Columns.QUEST_ID +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.Columns.QUEST_TYPE +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.Columns.ELEMENT_ID +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.Columns.ELEMENT_TYPE +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.Columns.TAG_CHANGES +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.Columns.CHANGES_SOURCE +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.NAME +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestTable.NAME_MERGED_VIEW +import de.westnordost.streetcomplete.ktx.* +import de.westnordost.streetcomplete.util.Serializer + +class UndoOsmQuestDao @Inject constructor( + private val dbHelper: SQLiteOpenHelper, + private val serializer: Serializer, + private val questTypeList: QuestTypeRegistry +) { + private val db get() = dbHelper.writableDatabase + + fun getAll(): List { + return db.query(NAME_MERGED_VIEW) { it.createUndo() } + } + + fun get(questId: Long): UndoOsmQuest? { + val selection = "$QUEST_ID = ?" + val args = arrayOf(questId.toString()) + return db.queryOne(NAME_MERGED_VIEW, null, selection, args) { it.createUndo() } + } + + fun delete(questId: Long) { + db.delete(NAME, "$QUEST_ID = $questId", null) + } + + fun add(quest: UndoOsmQuest) { + db.insert(NAME, null, quest.createContentValues()) + } + + private fun UndoOsmQuest.createContentValues() = ContentValues().also { v -> + v.put(QUEST_ID, id) + v.put(QUEST_TYPE, type.javaClass.simpleName) + v.put(TAG_CHANGES, serializer.toBytes(changes)) + v.put(CHANGES_SOURCE, changesSource) + v.put(ELEMENT_TYPE, elementType.name) + v.put(ELEMENT_ID, elementId) + } + + private fun Cursor.createUndo() = UndoOsmQuest( + getLong(QUEST_ID), + questTypeList.getByName(getString(QUEST_TYPE)) as OsmElementQuestType<*>, + Element.Type.valueOf(getString(ELEMENT_TYPE)), + getLong(ELEMENT_ID), + serializer.toObject(getBlob(TAG_CHANGES)), + getString(CHANGES_SOURCE), + ElementGeometryDao.createObjectFrom(serializer, this) + ) +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestTable.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestTable.kt new file mode 100644 index 0000000000..38decd3285 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/UndoOsmQuestTable.kt @@ -0,0 +1,16 @@ +package de.westnordost.streetcomplete.data.osm.persist + + +object UndoOsmQuestTable { + const val NAME = "osm_quests_undo" + const val NAME_MERGED_VIEW = "osm_quests_full_undo" + + object Columns { + const val QUEST_ID = "quest_id" + const val QUEST_TYPE = "quest_type" + const val ELEMENT_ID = "element_id" + const val ELEMENT_TYPE = "element_type" + const val TAG_CHANGES = "tag_changes" + const val CHANGES_SOURCE = "changes_source" + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/WayDao.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/WayDao.java index 7597d3e772..1fd420df47 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/WayDao.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/persist/WayDao.java @@ -88,4 +88,19 @@ public class WayDao extends AOsmElementDao return new OsmWay(id, version, nodeIds, tags); } + + /** Cleans up element entries that are not referenced by any quest anymore. */ + @Override public void deleteUnreferenced() + { + SQLiteDatabase db = dbHelper.getWritableDatabase(); + String where = getIdColumnName() + " NOT IN ( " + + getSelectAllElementIdsIn(OsmQuestTable.NAME) + + " UNION " + + getSelectAllElementIdsIn(UndoOsmQuestTable.NAME) + + " UNION " + + " SELECT " + OsmQuestSplitWayTable.Columns.WAY_ID + " AS " + getIdColumnName() + " FROM " + OsmQuestSplitWayTable.NAME + + ")"; + + db.delete(getTableName(), where, null); + } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/AOsmQuestChangesetsUpload.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/AOsmQuestChangesetsUpload.java deleted file mode 100644 index a88c391bdc..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/AOsmQuestChangesetsUpload.java +++ /dev/null @@ -1,277 +0,0 @@ -package de.westnordost.streetcomplete.data.osm.upload; - -import android.graphics.Point; -import android.util.Log; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; - -import javax.inject.Provider; - -import de.westnordost.osmapi.common.errors.OsmConflictException; -import de.westnordost.osmapi.map.MapDataDao; -import de.westnordost.osmapi.map.data.LatLon; -import de.westnordost.streetcomplete.ApplicationConstants; -import de.westnordost.streetcomplete.data.QuestGroup; -import de.westnordost.streetcomplete.data.QuestStatus; -import de.westnordost.streetcomplete.data.VisibleQuestListener; -import de.westnordost.streetcomplete.data.changesets.OpenChangesetInfo; -import de.westnordost.streetcomplete.data.changesets.OpenChangesetKey; -import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao; -import de.westnordost.streetcomplete.data.osm.OsmElementQuestType; -import de.westnordost.streetcomplete.data.osm.OsmQuest; -import de.westnordost.streetcomplete.data.osm.persist.AOsmQuestDao; -import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; -import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; -import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao; -import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; -import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener; -import de.westnordost.streetcomplete.util.SlippyMapMath; - -import static de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao.CLOSE_CHANGESETS_AFTER_INACTIVITY_OF; - -public abstract class AOsmQuestChangesetsUpload -{ - private final String TAG = getLogTag(); - - private final MapDataDao osmDao; - private final AOsmQuestDao questDB; - private final MergedElementDao elementDB; - private final ElementGeometryDao elementGeometryDB; - private final QuestStatisticsDao statisticsDB; - private final OpenChangesetsDao openChangesetsDB; - private final DownloadedTilesDao downloadedTilesDao; - private final Provider osmQuestChangeUploadProvider; - private final ChangesetAutoCloser changesetAutoCloser; - - private final List createdQuests; - private final List removedQuestIds; - private VisibleQuestListener visibleQuestListener; - private OnUploadedChangeListener uploadedChangeListener; - - // The cache is just here so that uploading 500 quests of same quest type does not result in 500 DB requests. - private Map changesetIdsCache = new HashMap<>(); - - public AOsmQuestChangesetsUpload( - MapDataDao osmDao, AOsmQuestDao questDB, MergedElementDao elementDB, - ElementGeometryDao elementGeometryDB, QuestStatisticsDao statisticsDB, - OpenChangesetsDao openChangesetsDB, DownloadedTilesDao downloadedTilesDao, - Provider osmQuestChangeUploadProvider, - ChangesetAutoCloser changesetAutoCloser) - { - this.osmDao = osmDao; - this.questDB = questDB; - this.elementDB = elementDB; - this.statisticsDB = statisticsDB; - this.elementGeometryDB = elementGeometryDB; - this.openChangesetsDB = openChangesetsDB; - this.downloadedTilesDao = downloadedTilesDao; - this.osmQuestChangeUploadProvider = osmQuestChangeUploadProvider; - this.changesetAutoCloser = changesetAutoCloser; - createdQuests = new ArrayList<>(); - removedQuestIds = new ArrayList<>(); - } - - public synchronized void setProgressListener(OnUploadedChangeListener uploadedChangeListener) - { - this.uploadedChangeListener = uploadedChangeListener; - } - - public synchronized void setVisibleQuestListener(VisibleQuestListener visibleQuestListener) - { - this.visibleQuestListener = visibleQuestListener; - } - - public synchronized void upload(AtomicBoolean cancelState) - { - int commits = 0, obsolete = 0; - changesetIdsCache = new HashMap<>(); - createdQuests.clear(); - removedQuestIds.clear(); - - HashSet uploadedQuestTypes = new HashSet<>(); - - for(OsmQuest quest : questDB.getAll(null, QuestStatus.ANSWERED)) - { - if(cancelState.get()) break; // break so that the unreferenced stuff is deleted still - - // was deleted while trying to upload another quest - if(removedQuestIds.contains(quest.getId())) continue; - - long changesetId = getChangesetIdOrCreate(quest.getOsmElementQuestType(), quest.getChangesSource()); - OsmQuestChangeUpload.UploadResult uploadResult = uploadAndHandleChangesetConflict(changesetId, quest); - createdQuests.addAll(uploadResult.createdQuests); - removedQuestIds.addAll(uploadResult.removedQuestIds); - if (uploadResult.success) - { - uploadedQuestTypes.add(quest.getOsmElementQuestType()); - if(uploadedChangeListener != null) uploadedChangeListener.onUploaded(); - statisticsDB.addOne(quest.getType().getClass().getSimpleName()); - commits++; - } - else - { - if(uploadedChangeListener != null) uploadedChangeListener.onDiscarded(); - obsolete++; - invalidateAreaAroundQuest(quest); - } - } - - cleanUp(uploadedQuestTypes); - - String logMsg = "Committed " + commits + " changes"; - if(obsolete > 0) - { - logMsg += " but dropped " + obsolete + " changes because there were conflicts"; - } - - Log.i(TAG, logMsg); - - if(!createdQuests.isEmpty()) - { - int createdQuestsCount = createdQuests.size(); - if(visibleQuestListener != null) - { - visibleQuestListener.onQuestsCreated(createdQuests, QuestGroup.OSM); - } - Log.i(TAG, "Created " + createdQuestsCount + " new quests"); - } - if(!removedQuestIds.isEmpty()) - { - int removedQuestsCount = removedQuestIds.size(); - if(visibleQuestListener != null) - { - visibleQuestListener.onQuestsRemoved(removedQuestIds, QuestGroup.OSM); - } - Log.i(TAG, "Removed " + removedQuestsCount + " quests which are no longer applicable"); - } - - closeOpenChangesets(); - - if(commits > 0) - { - changesetAutoCloser.enqueue(); - } - } - - private OsmQuestChangeUpload.UploadResult uploadAndHandleChangesetConflict(long changesetId, OsmQuest quest) - { - try - { - return osmQuestChangeUploadProvider.get().upload(changesetId, quest, shouldCheckForQuestApplicability()); - } - catch (OsmConflictException e) - { - OsmElementQuestType questType = quest.getOsmElementQuestType(); - - long newChangesetId = createChangeset(questType, quest.getChangesSource()); - OpenChangesetKey key = new OpenChangesetKey(questType.getClass().getSimpleName(), quest.getChangesSource()); - changesetIdsCache.put(key, newChangesetId); - - // try again with new created changeset. If this still throws an exception, it is - // likely a programming error in this code - return osmQuestChangeUploadProvider.get().upload(newChangesetId, quest, shouldCheckForQuestApplicability()); - } - } - - protected abstract String getLogTag(); - - private void cleanUp(Set questTypes) - { - long timestamp = System.currentTimeMillis() - ApplicationConstants.MAX_QUEST_UNDO_HISTORY_AGE; - int deletedQuests = questDB.deleteAllClosed(timestamp); - if(deletedQuests > 0) - { - elementGeometryDB.deleteUnreferenced(); - elementDB.deleteUnreferenced(); - // must be after unreferenced elements have been deleted - for (OsmElementQuestType questType : questTypes) - { - questType.cleanMetadata(); - } - } - } - - public synchronized void closeOpenChangesets() - { - long timePassed = System.currentTimeMillis() - openChangesetsDB.getLastQuestSolvedTime(); - if(timePassed < CLOSE_CHANGESETS_AFTER_INACTIVITY_OF) return; - - for (OpenChangesetInfo info : openChangesetsDB.getAll()) - { - try - { - osmDao.closeChangeset(info.changesetId); - Log.i(TAG, "Closed changeset #" + info.changesetId + "."); - } - catch (OsmConflictException e) - { - Log.w(TAG, "Couldn't close changeset #" + info.changesetId + " because it has already been closed."); - } - finally - { - // done! - openChangesetsDB.delete(info.key); - } - } - } - - private long getChangesetIdOrCreate(OsmElementQuestType questType, String source) - { - String questTypeName = questType.getClass().getSimpleName(); - - OpenChangesetKey key = new OpenChangesetKey(questTypeName, source); - Long cachedChangesetId = changesetIdsCache.get(key); - if(cachedChangesetId != null) return cachedChangesetId; - - OpenChangesetInfo changesetInfo = openChangesetsDB.get(key); - long result; - if (changesetInfo != null && changesetInfo.changesetId != null) - { - result = changesetInfo.changesetId; - } - else - { - result = createChangeset(questType, source); - } - - changesetIdsCache.put(key, result); - return result; - } - - private long createChangeset(OsmElementQuestType questType, String source) - { - OpenChangesetKey key = new OpenChangesetKey(questType.getClass().getSimpleName(), source); - long changesetId = osmDao.openChangeset(createChangesetTags(questType, source)); - openChangesetsDB.replace(key, changesetId); - return changesetId; - } - - private void invalidateAreaAroundQuest(OsmQuest quest) - { - // called after a conflict. If there is a conflict, the user is not the only one in that - // area, so best invalidate all downloaded quests here and redownload on next occasion - LatLon questPosition = quest.getGeometry().center; - Point tile = SlippyMapMath.enclosingTile(questPosition, ApplicationConstants.QUEST_TILE_ZOOM); - downloadedTilesDao.remove(tile); - } - - protected abstract boolean shouldCheckForQuestApplicability(); - - private Map createChangesetTags(OsmElementQuestType questType, String source) - { - Map changesetTags = new HashMap<>(); - String commitMessage = questType.getCommitMessage(); - changesetTags.put("comment", commitMessage); - changesetTags.put("created_by", ApplicationConstants.USER_AGENT); - String questTypeName = questType.getClass().getSimpleName(); - changesetTags.put(ApplicationConstants.QUESTTYPE_TAG_KEY, questTypeName); - changesetTags.put("source", source); - return changesetTags; - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/ChangesetAutoCloserWorker.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/ChangesetAutoCloserWorker.java index 92aa070418..6c6542464a 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/ChangesetAutoCloserWorker.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/ChangesetAutoCloserWorker.java @@ -14,7 +14,7 @@ public class ChangesetAutoCloserWorker extends Worker { - @Inject OsmQuestChangesetsUpload osmQuestChangesUpload; + @Inject OpenQuestChangesetsManager openQuestChangesetsManager; public ChangesetAutoCloserWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) { @@ -26,7 +26,7 @@ public ChangesetAutoCloserWorker(@NonNull Context context, @NonNull WorkerParame { try { - osmQuestChangesUpload.closeOpenChangesets(); + openQuestChangesetsManager.closeOldChangesets(); } catch(OsmConnectionException e) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/ConflictExceptions.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/ConflictExceptions.kt new file mode 100644 index 0000000000..3c2943bd7c --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/ConflictExceptions.kt @@ -0,0 +1,17 @@ +package de.westnordost.streetcomplete.data.osm.upload + +open class ConflictException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : RuntimeException(message, cause) + +class ChangesetConflictException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : ConflictException(message, cause) + +open class ElementConflictException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : ConflictException(message, cause) + +/** Element conflict that concern all quests that have something to do with this element */ +open class ElementIncompatibleException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : ElementConflictException(message, cause) + +class ElementDeletedException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : ElementIncompatibleException(message, cause) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/HasElementTagChanges.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/HasElementTagChanges.kt new file mode 100644 index 0000000000..f00efd9465 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/HasElementTagChanges.kt @@ -0,0 +1,9 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import de.westnordost.osmapi.map.data.Element +import de.westnordost.streetcomplete.data.osm.changes.StringMapChanges + +interface HasElementTagChanges { + val changes: StringMapChanges? + fun isApplicableTo(element: Element): Boolean? +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OpenQuestChangesetsManager.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OpenQuestChangesetsManager.kt new file mode 100644 index 0000000000..f2dd185d63 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OpenQuestChangesetsManager.kt @@ -0,0 +1,70 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import android.util.Log + +import de.westnordost.osmapi.common.errors.OsmConflictException +import de.westnordost.osmapi.map.MapDataDao +import de.westnordost.streetcomplete.ApplicationConstants.QUESTTYPE_TAG_KEY +import de.westnordost.streetcomplete.ApplicationConstants.USER_AGENT +import de.westnordost.streetcomplete.data.QuestType +import de.westnordost.streetcomplete.data.changesets.OpenChangesetKey +import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao +import de.westnordost.streetcomplete.data.osm.OsmElementQuestType + +import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao.CLOSE_CHANGESETS_AFTER_INACTIVITY_OF +import javax.inject.Inject + +/** Manages the creation and reusage of quest-related changesets */ +class OpenQuestChangesetsManager @Inject constructor( + private val osmDao: MapDataDao, + private val openChangesetsDB: OpenChangesetsDao, + private val changesetAutoCloser: ChangesetAutoCloser +) { + private val TAG = "ChangesetManager" + + fun getOrCreateChangeset(questType: OsmElementQuestType<*>, source: String): Long { + closeOldChangesets() + val key = OpenChangesetKey(questType.name, source) + val changesetInfo = openChangesetsDB.get(key) + return if (changesetInfo?.changesetId != null) { + changesetInfo.changesetId + } else { + createChangeset(questType, source) + } + } + + fun createChangeset(questType: OsmElementQuestType<*>, source: String): Long { + val key = OpenChangesetKey(questType.name, source) + val changesetId = osmDao.openChangeset(createChangesetTags(questType, source)) + openChangesetsDB.replace(key, changesetId) + changesetAutoCloser.enqueue() + Log.i(TAG, "Created changeset #$changesetId") + return changesetId + } + + @Synchronized fun closeOldChangesets() { + val timePassed = System.currentTimeMillis() - openChangesetsDB.lastQuestSolvedTime + if (timePassed < CLOSE_CHANGESETS_AFTER_INACTIVITY_OF) return + + for (info in openChangesetsDB.all) { + try { + osmDao.closeChangeset(info.changesetId) + Log.i(TAG, "Closed changeset #${info.changesetId}") + } catch (e: OsmConflictException) { + Log.w(TAG, "Couldn't close changeset #${info.changesetId} because it has already been closed") + } finally { + openChangesetsDB.delete(info.key) + } + } + } + + private fun createChangesetTags(questType: OsmElementQuestType<*>, source: String) = + mapOf( + "comment" to questType.commitMessage, + "created_by" to USER_AGENT, + QUESTTYPE_TAG_KEY to questType.name, + "source" to source + ) +} + +private val QuestType<*>.name get() = javaClass.simpleName diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmInChangesetsUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmInChangesetsUpload.kt new file mode 100644 index 0000000000..c67050fd57 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmInChangesetsUpload.kt @@ -0,0 +1,116 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import androidx.annotation.CallSuper +import de.westnordost.osmapi.map.data.Element +import de.westnordost.streetcomplete.data.QuestGroup +import de.westnordost.streetcomplete.data.QuestType +import de.westnordost.streetcomplete.data.VisibleQuestListener +import de.westnordost.streetcomplete.data.osm.OsmElementQuestType +import de.westnordost.streetcomplete.data.osm.OsmQuest +import de.westnordost.streetcomplete.data.osm.OsmQuestGiver +import de.westnordost.streetcomplete.data.osm.download.ElementGeometryCreator +import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao +import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao +import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao +import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener + +import java.util.concurrent.atomic.AtomicBoolean + +abstract class OsmInChangesetsUpload( + private val elementDB: MergedElementDao, + private val elementGeometryDB: ElementGeometryDao, + private val changesetManager: OpenQuestChangesetsManager, + private val questGiver: OsmQuestGiver, + private val statisticsDB: QuestStatisticsDao, + private val elementGeometryCreator: ElementGeometryCreator + ) { + + var visibleQuestListener: VisibleQuestListener? = null + var uploadedChangeListener: OnUploadedChangeListener? = null + + @Synchronized @CallSuper open fun upload(cancelled: AtomicBoolean) { + if (cancelled.get()) return + + val uploadedQuestTypes = mutableSetOf>() + val createdOsmQuests = mutableListOf() + val removedOsmQuestIds = mutableListOf() + + for (quest in getAll()) { + if (cancelled.get()) break + + try { + val uploadedElements = uploadSingle(quest) + for (element in uploadedElements) { + val questUpdates = updateElement(element) + createdOsmQuests.addAll(questUpdates.createdQuests) + removedOsmQuestIds.addAll(questUpdates.removedQuestIds) + } + uploadedQuestTypes.add(quest.osmElementQuestType) + onUploadSuccessful(quest) + uploadedChangeListener?.onUploaded() + statisticsDB.addOne(quest.osmElementQuestType.name) + } catch (e: ElementIncompatibleException) { + val questIds = deleteElement(quest.elementType, quest.elementId) + removedOsmQuestIds.addAll(questIds) + onUploadFailed(quest, e) + uploadedChangeListener?.onDiscarded() + } catch (e: ElementConflictException) { + onUploadFailed(quest, e) + uploadedChangeListener?.onDiscarded() + } + } + cleanUp(uploadedQuestTypes) + + if (createdOsmQuests.isNotEmpty()) { + visibleQuestListener?.onQuestsCreated(createdOsmQuests, QuestGroup.OSM) + } + if (removedOsmQuestIds.isNotEmpty()) { + visibleQuestListener?.onQuestsRemoved(removedOsmQuestIds, QuestGroup.OSM) + } + } + + private fun uploadSingle(quest: T) : List { + val element = elementDB.get(quest.elementType, quest.elementId) + ?: throw ElementDeletedException("Element deleted") + + return try { + val changesetId = changesetManager.getOrCreateChangeset(quest.osmElementQuestType, quest.source) + uploadSingle(changesetId, quest, element) + } catch (e: ChangesetConflictException) { + val changesetId = changesetManager.createChangeset(quest.osmElementQuestType, quest.source) + uploadSingle(changesetId, quest, element) + } + } + + private fun updateElement(newElement: Element): OsmQuestGiver.QuestUpdates { + elementDB.put(newElement) + elementGeometryDB.put(newElement.type, newElement.id, elementGeometryCreator.create(newElement)) + return questGiver.updateQuests(newElement) + } + + private fun deleteElement(elementType: Element.Type, elementId: Long): List { + elementDB.delete(elementType, elementId) + elementGeometryDB.delete(elementType, elementId) + return questGiver.deleteQuests(elementType, elementId) + } + + @CallSuper protected open fun cleanUp(questTypes: Set>) { + elementGeometryDB.deleteUnreferenced() + elementDB.deleteUnreferenced() + // must be after unreferenced elements have been deleted + for (questType in questTypes) { + questType.cleanMetadata() + } + } + + protected abstract fun getAll() : Collection + /** Upload the changes for a single quest and element. + * Returns the updated element(s) for which it should be checked whether they are eligible + * for new quests (or not eligible anymore for existing quests) */ + protected abstract fun uploadSingle(changesetId: Long, quest: T, element: Element): List + + protected abstract fun onUploadSuccessful(quest: T) + protected abstract fun onUploadFailed(quest: T, e: Throwable) +} + +private val QuestType<*>.name get() = javaClass.simpleName diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestChangeUpload.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestChangeUpload.java deleted file mode 100644 index cf58b72270..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestChangeUpload.java +++ /dev/null @@ -1,370 +0,0 @@ -package de.westnordost.streetcomplete.data.osm.upload; - -import android.util.Log; - -import java.net.HttpURLConnection; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; - -import javax.inject.Inject; - -import androidx.annotation.NonNull; -import de.westnordost.osmapi.common.errors.OsmConflictException; -import de.westnordost.osmapi.map.MapDataDao; -import de.westnordost.osmapi.map.data.Element; -import de.westnordost.osmapi.map.data.Node; -import de.westnordost.osmapi.map.data.OsmNode; -import de.westnordost.osmapi.map.data.OsmRelation; -import de.westnordost.osmapi.map.data.OsmWay; -import de.westnordost.osmapi.map.data.Relation; -import de.westnordost.osmapi.map.data.Way; -import de.westnordost.streetcomplete.data.QuestStatus; -import de.westnordost.streetcomplete.data.osm.OsmQuest; -import de.westnordost.streetcomplete.data.osm.OsmQuestGiver; -import de.westnordost.streetcomplete.data.osm.changes.StringMapChanges; -import de.westnordost.streetcomplete.data.osm.download.ElementGeometryCreator; -import de.westnordost.streetcomplete.data.osm.persist.AOsmQuestDao; -import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; -import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; -import de.westnordost.streetcomplete.util.SphericalEarthMath; - -import static java.net.HttpURLConnection.HTTP_CONFLICT; - -public class OsmQuestChangeUpload -{ - private final String TAG = "OsmQuestUpload"; - - private final MapDataDao osmDao; - private final AOsmQuestDao questDB; - private final MergedElementDao elementDB; - private final ElementGeometryDao elementGeometryDB; - private final ElementGeometryCreator elementGeometryCreator; - private final OsmQuestGiver questGiver; - - private boolean called = false; - private OsmQuest quest; - private long changesetId; - private boolean shouldCheckForQuestApplicability; - - private List createdQuests = new ArrayList<>(); - private List removedQuestIds = new ArrayList<>(); - private boolean alreadyHandlingElementConflict; - - @Inject public OsmQuestChangeUpload( - MapDataDao osmDao, AOsmQuestDao questDB, MergedElementDao elementDB, - ElementGeometryDao elementGeometryDB, ElementGeometryCreator elementGeometryCreator, - OsmQuestGiver questGiver) - { - this.osmDao = osmDao; - this.questDB = questDB; - this.elementDB = elementDB; - this.elementGeometryDB = elementGeometryDB; - this.questGiver = questGiver; - this.elementGeometryCreator = elementGeometryCreator; - } - - public static class UploadResult - { - public final boolean success; - public final List createdQuests; - public final List removedQuestIds; - - public UploadResult(boolean success, List createdQuests, List removedQuestIds) - { - this.success = success; - this.createdQuests = createdQuests; - this.removedQuestIds = removedQuestIds; - } - } - - public synchronized UploadResult upload(long changesetId, OsmQuest quest) - { - return upload(changesetId, quest, true); - } - - public synchronized UploadResult upload( - long changesetId, OsmQuest quest, boolean shouldCheckForQuestApplicability) - { - if(called) throw new IllegalStateException("This is a single-use object"); - called = true; - - this.quest = quest; - this.changesetId = changesetId; - this.shouldCheckForQuestApplicability = shouldCheckForQuestApplicability; - Element element = elementDB.get(quest.getElementType(), quest.getElementId()); - - boolean success = uploadQuestChange(element); - if(success) - { - quest.setStatus(QuestStatus.CLOSED); - questDB.update(quest); - - } else { - // #812 conflicting quests may not reside in the database, otherwise they would wrongfully - // be candidates for an undo - even though nothing was changed - questDB.delete(quest.getId()); - } - - return new UploadResult(success, createdQuests, removedQuestIds); - } - - private boolean uploadQuestChange(Element element) - { - Element elementWithChangesApplied = changesApplied(element); - if(elementWithChangesApplied == null) - { - return false; - } - - int[] newVersion = {element.getVersion()}; - try - { - // only necessary because of #1408: Elements where the version is invalid need to be treated - // as element conflicts so that the element can be updated from server, otherwise all users - // who already downloaded those incomplete elements are stuck and can no longer upload anything - // see also https://github.com/openstreetmap/operations/issues/303 - if(element.getVersion() < 0) - throw new OsmConflictException(HTTP_CONFLICT, "Conflict", "Negative version is invalid"); - - osmDao.uploadChanges(changesetId, Collections.singleton(elementWithChangesApplied), diffElement -> - { - if(diffElement.clientId == elementWithChangesApplied.getId()) - { - newVersion[0] = diffElement.serverVersion; - /* It is not necessary (yet) to handle updating the element's id because - StreetComplete does not add or delete elements */ - } - }); - } - catch(OsmConflictException e) - { - return handleConflict(element, e); - } - Element updatedElement = copyElement(elementWithChangesApplied, newVersion[0]); - - // save with new version when persisting to DB - updateElement(updatedElement); - - return true; - } - - private Element changesApplied(Element element) - { - // The element can be null if it has been deleted in the meantime (outside this app usually) - if(element == null) - { - Log.d(TAG, "Dropping quest " + getQuestStringForLog() + - " because the associated element has already been deleted"); - return null; - } - - Element copy = copyElement(element, element.getVersion()); - - StringMapChanges changes = quest.getChanges(); - try - { - changes.applyTo(copy.getTags()); - } - catch (IllegalStateException e) - { - Log.d(TAG, "Dropping quest " + getQuestStringForLog() + - " because there has been a conflict while applying the changes"); - return null; - } - catch (IllegalArgumentException e) - { - /* There is a max key/value length limit of 255 characters in OSM. If we reach this - point, it means the UI did permit an input of more than that. So, we have to catch - this here latest. - This is a warning because the UI should prevent this in the first place, at least - for free-text input. For structured input, like opening hours, it is another matter - because it's awkward to explain to a non-technical user this technical limitation - - See also https://github.com/openstreetmap/openstreetmap-website/issues/2025 - */ - Log.w(TAG, "Dropping quest " + getQuestStringForLog() + - " because a key or value is too long for OSM", e); - return null; - } - - return copy; - } - - private static Element copyElement(Element e, int newVersion) - { - if(e == null) return null; - Map tagsCopy = new HashMap<>(); - if(e.getTags() != null) tagsCopy.putAll(e.getTags()); - - if(e instanceof Node) - { - return new OsmNode(e.getId(), newVersion, ((Node)e).getPosition(), tagsCopy); - } - if(e instanceof Way) - { - return new OsmWay(e.getId(), newVersion, - new ArrayList<>(((Way)e).getNodeIds()), tagsCopy); - } - if(e instanceof Relation) - { - return new OsmRelation(e.getId(), newVersion, - new ArrayList<>(((Relation)e).getMembers()), tagsCopy); - } - return null; - } - - private boolean handleConflict(Element element, OsmConflictException e) - { - // Conflict can either happen because of the changeset or because of the element(s) - // uploaded. Let's find out. - Element newElement = getElementFromServer(element.getType(), element.getId()); - if (newElement != null && newElement.getVersion() == element.getVersion()) - { - // a changeset conflict cannot be handled here: throw it along - throw e; - } - - // safeguard against stack overflow in case of programming error - if(alreadyHandlingElementConflict) - { - throw new RuntimeException("OSM server continues to report an element " + - "conflict on uploading the changes for the quest " + getQuestStringForLog() + - ". The local version is " + element.getVersion(), e); - } - alreadyHandlingElementConflict = true; - - if (newElement != null) - { - if (isGeometrySubstantiallyDifferent(element, newElement)) - { - // TODO NOTE: when implementing splitting up - what about undo? - Log.d(TAG, "Dropping quest " + getQuestStringForLog() + - " and all other quests for the same element because the element's geometry" + - " changed substantially"); - replaceElement(newElement); - return false; - } - - updateElement(newElement); - - // if after updating to the new version of the element, the quest is not applicable to the - // element anymore, drop it (#720) - if (shouldCheckForQuestApplicability && !questIsApplicableToElement(newElement)) - { - Log.d(TAG, "Dropping quest " + getQuestStringForLog() + - " because the quest is no longer applicable to the element"); - return false; - } - } - else - { - deleteElement(quest.getElementType(), quest.getElementId()); - } - return uploadQuestChange(newElement); - - } - - private boolean questIsApplicableToElement(Element element) - { - Boolean questIsApplicableToElement = quest.getOsmElementQuestType().isApplicableTo(element); - return questIsApplicableToElement == null || questIsApplicableToElement; - } - - private boolean isGeometrySubstantiallyDifferent(Element element, Element newElement) - { - if(element instanceof Node) - return isNodeGeometrySubstantiallyDifferent((Node) element, (Node) newElement); - if(element instanceof Way) - return isWayGeometrySubstantiallyDifferent((Way) element, (Way) newElement); - if(element instanceof Relation) - return isRelationGeometrySubstantiallyDifferent((Relation) element, (Relation) newElement); - return false; - } - - private boolean isNodeGeometrySubstantiallyDifferent(Node node, Node newNode) - { - /* Moving the node a distance beyond what would pass as adjusting the position within a - building counts as substantial change. Also, the maximum distance should be not (much) - bigger than the usual GPS inaccuracy in the city. */ - double distance = SphericalEarthMath.distance(node.getPosition(), newNode.getPosition()); - return distance > 20; - } - - private boolean isWayGeometrySubstantiallyDifferent(Way way, Way newWay) - { - /* if the first or last node is different, it means that the way has either been extended or - shortened at one end, which is counted as being substantial: - If for example the surveyor has been asked to determine something for a certain way - and this way is now longer, his answer does not apply to the whole way anymore, so that - is an unsolvable conflict. */ - List nodeIds = way.getNodeIds(); - List newNodeIds = newWay.getNodeIds(); - - if(newNodeIds.isEmpty()) return true; - - if(((long) nodeIds.get(0)) != newNodeIds.get(0)) return true; - - int lastIndex = nodeIds.size()-1; - int lastIndexNew = newNodeIds.size()-1; - if((long) nodeIds.get(lastIndex) != newNodeIds.get(lastIndexNew)) return true; - - return false; - } - - private boolean isRelationGeometrySubstantiallyDifferent(Relation relation, Relation newRelation) - { - /* a relation is counted as substantially different, if any member changed, even if just - the order changed because for some relations, the order has an important meaning */ - return !relation.getMembers().equals(newRelation.getMembers()); - } - - private void updateElement(@NonNull Element newElement) - { - elementDB.put(newElement); - OsmQuestGiver.QuestUpdates questUpdates = questGiver.updateQuests(newElement); - createdQuests.addAll(questUpdates.createdQuests); - removedQuestIds.addAll(questUpdates.removedQuestIds); - } - - private void deleteElement(Element.Type type, long id) - { - elementDB.delete(type, id); - elementGeometryDB.delete(type, id); - removeQuestsForElement(type, id); - } - - private void replaceElement(Element element) - { - removeQuestsForElement(element.getType(), element.getId()); - updateElement(element); - elementGeometryDB.put(element.getType(), element.getId(), elementGeometryCreator.create(element)); - } - - private void removeQuestsForElement(Element.Type type, long id) - { - List ids = questDB.getAllIds(type, id); - questDB.deleteAll(ids); - removedQuestIds.addAll(ids); - } - - private Element getElementFromServer(Element.Type elementType, long id) - { - switch(elementType) - { - case NODE: return osmDao.getNode(id); - case WAY: return osmDao.getWay(id); - case RELATION: return osmDao.getRelation(id); - } - return null; - } - - private String getQuestStringForLog() - { - return quest.getType().getClass().getSimpleName() + " for " + - quest.getElementType().name().toLowerCase(Locale.US) + " #" + quest.getElementId(); - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestChangesetsUpload.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestChangesetsUpload.java deleted file mode 100644 index 7af78bdb7e..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestChangesetsUpload.java +++ /dev/null @@ -1,36 +0,0 @@ -package de.westnordost.streetcomplete.data.osm.upload; - -import javax.inject.Inject; -import javax.inject.Provider; - -import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao; -import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; -import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; -import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; -import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao; -import de.westnordost.osmapi.map.MapDataDao; -import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; - -public class OsmQuestChangesetsUpload extends AOsmQuestChangesetsUpload -{ - @Inject public OsmQuestChangesetsUpload( - MapDataDao osmDao, OsmQuestDao questDB, MergedElementDao elementDB, - ElementGeometryDao elementGeometryDB, QuestStatisticsDao statisticsDB, - OpenChangesetsDao openChangesetsDB, DownloadedTilesDao downloadedTilesDao, - Provider osmQuestChangeUploadProvider, - ChangesetAutoCloser changesetAutoCloser) - { - super(osmDao, questDB, elementDB, elementGeometryDB, statisticsDB, openChangesetsDB, - downloadedTilesDao, osmQuestChangeUploadProvider, changesetAutoCloser); - } - - @Override protected String getLogTag() - { - return "OsmQuestChangesetsUpload"; - } - - @Override protected boolean shouldCheckForQuestApplicability() - { - return true; - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestsUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestsUpload.kt new file mode 100644 index 0000000000..0d4d2c9ebc --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/OsmQuestsUpload.kt @@ -0,0 +1,78 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import android.util.Log +import de.westnordost.osmapi.map.data.Element +import de.westnordost.streetcomplete.ApplicationConstants +import de.westnordost.streetcomplete.ApplicationConstants.MAX_QUEST_UNDO_HISTORY_AGE +import javax.inject.Inject + +import de.westnordost.streetcomplete.data.QuestStatus +import de.westnordost.streetcomplete.data.osm.OsmElementQuestType +import de.westnordost.streetcomplete.data.osm.OsmQuest +import de.westnordost.streetcomplete.data.osm.OsmQuestGiver +import de.westnordost.streetcomplete.data.osm.download.ElementGeometryCreator +import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao +import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao +import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao +import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao +import de.westnordost.streetcomplete.util.SlippyMapMath +import java.util.* +import java.util.concurrent.atomic.AtomicBoolean + +/** Gets all answered osm quests from local DB and uploads them via the OSM API */ +class OsmQuestsUpload @Inject constructor( + elementDB: MergedElementDao, + elementGeometryDB: ElementGeometryDao, + changesetManager: OpenQuestChangesetsManager, + questGiver: OsmQuestGiver, + statisticsDB: QuestStatisticsDao, + elementGeometryCreator: ElementGeometryCreator, + private val questDB: OsmQuestDao, + private val singleChangeUpload: SingleOsmElementTagChangesUpload, + private val downloadedTilesDao: DownloadedTilesDao +) : OsmInChangesetsUpload(elementDB, elementGeometryDB, changesetManager, questGiver, + statisticsDB, elementGeometryCreator) { + + private val TAG = "OsmQuestUpload" + + @Synchronized override fun upload(cancelled: AtomicBoolean) { + Log.i(TAG, "Applying quest changes") + super.upload(cancelled) + } + + override fun getAll(): Collection = questDB.getAll(null, QuestStatus.ANSWERED) + + override fun uploadSingle(changesetId: Long, quest: OsmQuest, element: Element): List { + return listOf(singleChangeUpload.upload(changesetId, quest, element)) + } + + override fun onUploadSuccessful(quest: OsmQuest) { + quest.status = QuestStatus.CLOSED + questDB.update(quest) + Log.d(TAG, "Uploaded osm quest ${quest.toLogString()}") + } + + override fun onUploadFailed(quest: OsmQuest, e: Throwable) { + /* #812 conflicting quests may not reside in the database, otherwise they would + wrongfully be candidates for an undo - even though nothing was changed */ + questDB.delete(quest.id!!) + invalidateAreaAroundQuest(quest) + Log.d(TAG, "Dropped osm quest ${quest.toLogString()}: ${e.message}") + } + + private fun invalidateAreaAroundQuest(quest: OsmQuest) { + // called after a conflict. If there is a conflict, the user is not the only one in that + // area, so best invalidate all downloaded quests here and redownload on next occasion + val tile = SlippyMapMath.enclosingTile(quest.center, ApplicationConstants.QUEST_TILE_ZOOM) + downloadedTilesDao.remove(tile) + } + + override fun cleanUp(questTypes: Set>) { + super.cleanUp(questTypes) + questDB.deleteAllClosed(System.currentTimeMillis() - MAX_QUEST_UNDO_HISTORY_AGE) + } +} + +private fun OsmQuest.toLogString() = + type.javaClass.simpleName + " for " + elementType.name.toLowerCase(Locale.US) + " #" + elementId diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SingleOsmElementTagChangesUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SingleOsmElementTagChangesUpload.kt new file mode 100644 index 0000000000..8b1796984f --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SingleOsmElementTagChangesUpload.kt @@ -0,0 +1,130 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import javax.inject.Inject +import de.westnordost.osmapi.common.errors.OsmConflictException +import de.westnordost.osmapi.map.MapDataDao +import de.westnordost.osmapi.map.data.* +import de.westnordost.streetcomplete.data.osm.changes.StringMapChanges +import de.westnordost.streetcomplete.ktx.copy +import de.westnordost.streetcomplete.util.SphericalEarthMath + +import java.net.HttpURLConnection.HTTP_CONFLICT + +/** Uploads the changes made for one quest + * Returns the element that has been updated or throws a ConflictException */ +class SingleOsmElementTagChangesUpload @Inject constructor(private val osmDao: MapDataDao) { + + fun upload(changesetId: Long, quest: HasElementTagChanges, dbElement: Element): Element { + var element = dbElement + var handlingConflict = false + + while(true) { + val changes = quest.changes ?: throw ElementConflictException("No changes") + + val elementWithChangesApplied = element.changesApplied(changes) + val handler = UpdateElementsHandler() + try { + /* only necessary because of #1408: Elements where the version is invalid need to be + treated as element conflicts so that the element can be updated from server, + otherwise all users who already downloaded those incomplete elements are stuck + and can no longer upload anything see also + https://github.com/openstreetmap/operations/issues/303 */ + if (element.version < 0) + throw OsmConflictException(HTTP_CONFLICT, "Conflict", "Invalid element version") + + osmDao.uploadChanges(changesetId, setOf(elementWithChangesApplied), handler) + + } catch (e: OsmConflictException) { + if (handlingConflict) { + throw ElementConflictException("API reports conflict even after update", e) + } + handlingConflict = true + element = handleConflict(quest, element, e) + // try again (go back to beginning of loop) + continue + } + return handler.getElementUpdates(listOf(elementWithChangesApplied)).updated.single() + } + } + + private fun handleConflict(quest: HasElementTagChanges, element: Element, e: OsmConflictException): Element { + /* Conflict can either happen because of the changeset or because of the element(s) + uploaded. A changeset conflict cannot be handled here */ + val newElement = element.fetchUpdated() + if (newElement?.version == element.version) throw ChangesetConflictException(e.message, e) + + if (newElement == null) { + throw ElementDeletedException("Element has already been deleted") + } + + if (isGeometrySubstantiallyDifferent(element, newElement)) { + throw ElementIncompatibleException("Element geometry changed substantially") + } + + /* if after updating to the new version of the element, the quest is not applicable to + the element anymore, drop it (#720) */ + if (quest.isApplicableTo(newElement) == false) { + throw ElementConflictException("Quest no longer applicable to the element") + } + return newElement + } + + private fun Element.fetchUpdated() = + when (this) { + is Node -> osmDao.getNode(id) + is Way -> osmDao.getWay(id) + is Relation -> osmDao.getRelation(id) + else -> null + } +} + +private fun isGeometrySubstantiallyDifferent(element: Element, newElement: Element) = + when (element) { + is Node -> isNodeGeometrySubstantiallyDifferent(element, newElement as Node) + is Way -> isWayGeometrySubstantiallyDifferent(element, newElement as Way) + is Relation -> isRelationGeometrySubstantiallyDifferent(element, newElement as Relation) + else -> false + } + +private fun isNodeGeometrySubstantiallyDifferent(node: Node, newNode: Node) = + /* Moving the node a distance beyond what would pass as adjusting the position within a + building counts as substantial change. Also, the maximum distance should be not (much) + bigger than the usual GPS inaccuracy in the city. */ + SphericalEarthMath.distance(node.position, newNode.position) > 20 + +private fun isWayGeometrySubstantiallyDifferent(way: Way, newWay: Way) = + /* if the first or last node is different, it means that the way has either been extended or + shortened at one end, which is counted as being substantial: + If for example the surveyor has been asked to determine something for a certain way + and this way is now longer, his answer does not apply to the whole way anymore, so that + is an unsolvable conflict. */ + way.nodeIds.firstOrNull() != newWay.nodeIds.firstOrNull() || + way.nodeIds.lastOrNull() != newWay.nodeIds.lastOrNull() + +private fun isRelationGeometrySubstantiallyDifferent(relation: Relation, newRelation: Relation) = + /* a relation is counted as substantially different, if any member changed, even if just + the order changed because for some relations, the order has an important meaning */ + relation.members != newRelation.members + + +private fun Element.changesApplied(changes: StringMapChanges): Element { + val copy = this.copy() + try { + if (copy.tags == null) throw ElementConflictException("The element has no tags") + changes.applyTo(copy.tags) + } catch (e: IllegalStateException) { + throw ElementConflictException("Conflict while applying the changes") + } catch (e: IllegalArgumentException) { + /* There is a max key/value length limit of 255 characters in OSM. If we reach this + point, it means the UI did permit an input of more than that. So, we have to catch + this here latest. + This is a warning because the UI should prevent this in the first place, at least + for free-text input. For structured input, like opening hours, it is another matter + because it's awkward to explain to a non-technical user this technical limitation + + See also https://github.com/openstreetmap/openstreetmap-website/issues/2025 + */ + throw ElementConflictException("Key or value is too long") + } + return copy +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SplitSingleWayUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SplitSingleWayUpload.kt new file mode 100644 index 0000000000..d9b7cb286d --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SplitSingleWayUpload.kt @@ -0,0 +1,339 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import de.westnordost.osmapi.common.errors.OsmConflictException +import de.westnordost.osmapi.common.errors.OsmNotFoundException +import de.westnordost.osmapi.map.MapDataDao +import de.westnordost.osmapi.map.data.* +import javax.inject.Inject +import de.westnordost.osmapi.map.data.Element.Type.* +import de.westnordost.streetcomplete.data.osm.changes.SplitAtLinePosition +import de.westnordost.streetcomplete.data.osm.changes.SplitAtPoint +import de.westnordost.streetcomplete.data.osm.changes.SplitPolylineAtPosition +import de.westnordost.streetcomplete.ktx.* +import de.westnordost.streetcomplete.util.SphericalEarthMath.distance +import de.westnordost.streetcomplete.util.SphericalEarthMath.pointOnPolylineFromStart +import kotlin.math.sign + +/** Uploads one split way + * Returns only the ways that have been updated or throws a ConflictException */ +class SplitSingleWayUpload @Inject constructor(private val osmDao: MapDataDao) { + + fun upload(changesetId: Long, way: Way, splits: List): List { + val updatedWay = way.fetchUpdated() + ?: throw ElementDeletedException("Way #${way.id} has been deleted") + if(updatedWay.isClosed() && splits.size < 2) + throw ElementConflictException("Must specify at least two split positions for a closed way") + checkForConflicts(way, updatedWay) + + val nodes = updatedWay.fetchNodes() + val positions = nodes.map { it.position } + /* the splits must be sorted strictly from start to end of way because the algorithm may + insert nodes in the way */ + val sortedSplits = splits.flatMap { it.toSplitWay(positions) }.sorted() + + val uploadElements = mutableListOf() + var newNodeId = -1L + + val splitAtIndices = mutableListOf() + var insertedNodeCount = 0 + for (split in sortedSplits) { + when(split) { + is SplitWayAtPoint -> { + splitAtIndices.add(split.index + insertedNodeCount) + } + is SplitWayAtLinePosition -> { + val splitNode = OsmNode(newNodeId--, 1, split.pos, null) + uploadElements.add(splitNode) + + val nodeIndex = split.index2 + insertedNodeCount + updatedWay.nodeIds.add(nodeIndex, splitNode.id) + splitAtIndices.add(nodeIndex) + ++insertedNodeCount + } + } + } + + uploadElements.addAll(splitWayAtIndices(updatedWay, splitAtIndices)) + val handler = UpdateElementsHandler() + try { + osmDao.uploadChanges(changesetId, uploadElements, handler) + } catch (e: OsmConflictException) { + throw ChangesetConflictException(e.message, e) + } + // the added nodes and updated relations are not relevant for quest creation, only the way are + return handler.getElementUpdates(uploadElements).updated.filterIsInstance() + } + + private fun checkForConflicts(old: Way, new: Way) { + if(old.version != new.version) { + // unsolvable conflict if other was shortened (e.g. cut in two) or extended + if(old.nodeIds.first() != new.nodeIds.first() || old.nodeIds.last() != new.nodeIds.last()) + throw ElementConflictException("Way #${old.id} has been changed and the conflict cannot be solved automatically") + } + } + + private fun splitWayAtIndices(originalWay: Way, splitIndices: List): List { + val newWays = createSplitWays(originalWay, splitIndices) + val updatedRelations = updateRelations(originalWay, newWays) + return newWays + updatedRelations + } + + /** Returns the elements that have been changed */ + private fun createSplitWays(originalWay: Way, splitIndices: List): List { + val nodesChunks = originalWay.nodeIds.splitIntoChunks(splitIndices) + /* Handle circular ways specially: If you split at a circular way at two nodes, you just + want to split it at these points, not also at the former endpoint. So if the last node is + the same first node, join the last and the first way chunk. (copied from JOSM) */ + if (nodesChunks.size > 1 && nodesChunks.first().first() == nodesChunks.last().last()) { + val lastChunk = nodesChunks.removeAt(nodesChunks.lastIndex) + lastChunk.removeAt(lastChunk.lastIndex) + nodesChunks.first().addAll(0, lastChunk) + } + + /* Instead of deleting the old way and replacing it with the new splitted chunks, one of the + chunks should use the id of the old way, so that it inherits the OSM history of the + previous way. The chunk with the most nodes is selected for this. + This is the same behavior as JOSM and Vespucci. */ + val indexOfChunkToKeep = nodesChunks.indexOfMaxBy { it.size } + val tags = originalWay.tags?.toMap() + var newWayId = -1L + return nodesChunks.mapIndexed { index, nodes -> + if(index == indexOfChunkToKeep) { + OsmWay(originalWay.id, originalWay.version, nodes, tags).apply { + isModified = true + } + } + else { + OsmWay(newWayId--, 0, nodes, tags) + } + } + } + + /** Returns the elements that have been changed */ + private fun updateRelations(originalWay: Way, newWays: List) : Collection { + val relations = originalWay.fetchParentRelations() + val result = mutableSetOf() + for (relation in relations) { + /* iterating in reverse because the relation member for the original way is replaced + by one or several new ways in-place within the loop. So, no relation member is + iterated twice. If in the future, there will be any special relation that removes + a relation member or replaces several relation members with only one relation member, + then this here won't work anymore and the algorithm needs to modify a copy of the + relation members list. */ + for(i in relation.members.size - 1 downTo 0) { + val relationMember = relation.members[i] + if (relationMember.type == WAY && relationMember.ref == originalWay.id) { + if (!updateSpecialRelation(relation, i, newWays)) { + updateNormalRelation(relation, i, originalWay, newWays) + } + result.add(relation) + } + } + } + return result + } + + /** Returns whether it has been treated as a special relation type */ + private fun updateSpecialRelation(relation: Relation, indexOfWayInRelation: Int, newWays: List): Boolean { + val relationType = relation.tags?.get("type") ?: "" + val originalWayRole = relation.members[indexOfWayInRelation].role + /* for a from-to-relation (i.e. turn restriction, destination sign, ...) only the two ways + directly connecting with the via node/way should be kept in the relation. If one of these + ways is split up, the correct way chunk must be selected to replace the old way. */ + if (originalWayRole == "from" || originalWayRole == "to") { + val viaNodeIds = relation.fetchViaNodeIds(relationType) + if (viaNodeIds.isNotEmpty()) { + val newWay = newWays.find { viaNodeIds.containsAny(it.nodeIds.firstAndLast()) } + if (newWay != null) { + val newRelationMember = OsmRelationMember(newWay.id, originalWayRole, WAY) + relation.members[indexOfWayInRelation] = newRelationMember + return true + } + } + } + // room for handling other special relation types here + + return false + } + + private fun updateNormalRelation(relation: Relation, indexOfWayInRelation: Int, + originalWay: Way, newWays: List) { + /* for any (non-special, see above) relation, the new way chunks that replace the original + way must be all inserted into each relation. In the correct order, if the relation is + ordered at all. */ + val originalWayRole = relation.members[indexOfWayInRelation].role + val newRelationMembers = newWays.map { way -> + OsmRelationMember(way.id, originalWayRole, WAY) }.toMutableList() + val isOrientedBackwards = originalWay.isOrientedForwardInOrderedRelation(relation, indexOfWayInRelation) == false + if (isOrientedBackwards) newRelationMembers.reverse() + + relation.members.removeAt(indexOfWayInRelation) + relation.members.addAll(indexOfWayInRelation, newRelationMembers) + } + + private fun Way.fetchNodes(): List { + try { + val nodesMap = osmDao.getNodes(nodeIds).associateBy { node -> node.id } + // the fetched nodes must be returned ordered in the way + return nodeIds.map { nodeId -> nodesMap.getValue(nodeId) } + } catch (e: OsmNotFoundException) { + throw ConflictException("Way was modified right while uploading the changes (what's the chance?)",e) + } + } + + private fun Way.fetchUpdated(): Way? = osmDao.getWay(id) + + private fun Way.fetchParentRelations() = osmDao.getRelationsForWay(id) + + /** returns null if the relation is not ordered, false if oriented backwards, true if oriented forward */ + private fun Way.isOrientedForwardInOrderedRelation(relation: Relation, indexInRelation: Int): Boolean? { + val wayIdBefore = relation.members.findPrevious(indexInRelation) { it.type == WAY }?.ref + val wayBefore = wayIdBefore?.let { osmDao.getWay(it) } + if (wayBefore != null) { + if (isAfterWayInChain(wayBefore)) return true + if (isBeforeWayInChain(wayBefore)) return false + } + + val wayIdAfter = relation.members.findNext(indexInRelation+1) { it.type == WAY }?.ref + val wayAfter = wayIdAfter?.let { osmDao.getWay(it) } + if (wayAfter != null) { + if (isBeforeWayInChain(wayAfter)) return true + if (isAfterWayInChain(wayAfter)) return false + } + + return null + } + + private fun Relation.fetchViaNodeIds(relationType: String): Set { + val vias = findVias(relationType) + val nodeIds = mutableSetOf() + for (via in vias) { + if (via.type == NODE) { + nodeIds.add(via.ref) + } else if (via.type == WAY) { + val way = osmDao.getWay(via.ref) + if (way != null) nodeIds.addAll(way.nodeIds.firstAndLast()) + } + } + return nodeIds + } + +} + +/** data class that carries the information for one split to perform on a random position on a way. + * So, same as SplitPolylineAtPosition, but additionally with the index of the split in the way. */ +private sealed class SplitWay : Comparable { + abstract val pos: LatLon + protected abstract val index: Int + protected abstract val delta: Double + + /** sort by index, then delta, ascending. The algorithm relies on this order! */ + override fun compareTo(other: SplitWay): Int { + val diffIndex = index - other.index + if (diffIndex != 0) return diffIndex + + val diffDelta = delta - other.delta + return diffDelta.sign.toInt() + } +} + +private data class SplitWayAtPoint(override val pos: LatLon, public override val index: Int) : SplitWay() { + override val delta get() = 0.0 +} + +private data class SplitWayAtLinePosition( val pos1: LatLon, val index1: Int, + val pos2: LatLon, val index2: Int, + public override val delta: Double) : SplitWay() { + override val index get() = index1 + override val pos: LatLon get() { + val line = listOf(pos1, pos2) + return pointOnPolylineFromStart(line, distance(line) * delta)!! + } +} + +/** creates a SplitWay from a SplitLineAtPosition, given the nodes of the way. So, basically it + * simply finds the node index/indices at which the split should be made. + * One SplitPolylineAtPosition will map to several SplitWays for self-intersecting ways that have + * a split at the position where they self-intersect. I.e. a way in the shape of an 8 split exactly + * in the centre. + * If the way changed significantly in the meantime, it will throw an ElementConflictException */ +private fun SplitPolylineAtPosition.toSplitWay(positions: List): Collection { + return when(this) { + is SplitAtPoint -> toSplitWay(positions) + is SplitAtLinePosition -> toSplitWay(positions) + } +} + +private fun SplitAtPoint.toSplitWay(positions: List): Collection { + // could be several indices, for example if the way has the shape of an 8. + var indicesOf = positions.osmIndicesOf(pos) + if (indicesOf.isEmpty()) throw ElementConflictException("To be split point has been moved") + + indicesOf = indicesOf.filter { index -> index > 0 && index < positions.lastIndex } + if (indicesOf.isEmpty()) + throw ElementConflictException("Split position is now at the very start or end of the way - can't split there") + + return indicesOf.map { indexOf -> SplitWayAtPoint(pos, indexOf) } +} + +private fun SplitAtLinePosition.toSplitWay(positions: List): Collection { + // could be several indices, for example if the way has the shape of an 8... + val indicesOf1 = positions.osmIndicesOf(pos1) + if (indicesOf1.isEmpty()) throw ElementConflictException("To be split line has been moved") + + val indicesOf2 = positions.osmIndicesOf(pos2) + if (indicesOf2.isEmpty()) throw ElementConflictException("To be split line has been moved") + + // ...and we need to find out which of the lines is meant + val result = mutableListOf() + for (i1 in indicesOf1) { + for (i2 in indicesOf2) { + /* For SplitAtLinePosition, the direction of the way does not matter. But for the + SplitWayAtLinePosition it must be in the same order as the OSM way. */ + if (i1 + 1 == i2) result.add(SplitWayAtLinePosition(pos1, i1, pos2, i2, delta)) + if (i2 + 1 == i1) result.add(SplitWayAtLinePosition(pos2, i2, pos1, i1, 1.0 - delta)) + } + } + if (result.isNotEmpty()) + return result + else + throw ElementConflictException("End points of the to be split line are not directly successive anymore") +} + +/** returns the indices at which the given pos is found in this list, taking into accound the limited + * precision of positions in OSM. */ +private fun List.osmIndicesOf(pos: LatLon): List = + mapIndexedNotNull { i, p -> if (p.equalsInOsm(pos)) i else null } + + +/** returns a copy of the list split at the given indices with each chunk sharing each the first and last element */ +private fun List.splitIntoChunks(indices: List): MutableList> { + val result = mutableListOf>() + var lastIndex = 0 + for (index in indices) { + result.add(subList(lastIndex, index+1).toMutableList()) + lastIndex = index + } + result.add(subList(lastIndex, size).toMutableList()) + return result +} + +/** returns whether this way immediately precedes the given way in a chain */ +private fun Way.isBeforeWayInChain(way: Way) = + nodeIds.last() == way.nodeIds.last() || nodeIds.last() == way.nodeIds.first() + +/** returns whether this way immediately follows the given way in a chain */ +private fun Way.isAfterWayInChain(way: Way) = + nodeIds.first() == way.nodeIds.last() || nodeIds.first() == way.nodeIds.first() + +private fun Relation.findVias(relationType: String): List { + val nodesAndWays = members.filter { it.type == NODE || it.type == WAY } + return when (relationType) { + "restriction" -> nodesAndWays.filter { it.role == "via" } + "destination_sign" -> { + nodesAndWays.filter { it.role == "intersection" }.takeUnless { it.isEmpty() } ?: + nodesAndWays.filter { it.role == "sign" } + } + else -> nodesAndWays.filter { it.role == "via" } + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SplitWaysUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SplitWaysUpload.kt new file mode 100644 index 0000000000..22d8a2dedb --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/SplitWaysUpload.kt @@ -0,0 +1,51 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import android.util.Log +import de.westnordost.osmapi.map.data.Element +import de.westnordost.osmapi.map.data.Way +import de.westnordost.streetcomplete.data.osm.OsmQuestGiver +import de.westnordost.streetcomplete.data.osm.OsmQuestSplitWay +import de.westnordost.streetcomplete.data.osm.download.ElementGeometryCreator +import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao +import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayDao +import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao +import java.util.concurrent.atomic.AtomicBoolean +import javax.inject.Inject + +/** Gets all split ways from local DB and uploads them via the OSM API */ +class SplitWaysUpload @Inject constructor( + elementDB: MergedElementDao, + elementGeometryDB: ElementGeometryDao, + changesetManager: OpenQuestChangesetsManager, + questGiver: OsmQuestGiver, + statisticsDB: QuestStatisticsDao, + elementGeometryCreator: ElementGeometryCreator, + private val splitWayDB: OsmQuestSplitWayDao, + private val splitSingleOsmWayUpload: SplitSingleWayUpload +) : OsmInChangesetsUpload(elementDB, elementGeometryDB, changesetManager, + questGiver, statisticsDB, elementGeometryCreator) { + + private val TAG = "SplitOsmWayUpload" + + @Synchronized override fun upload(cancelled: AtomicBoolean) { + Log.i(TAG, "Splitting ways") + super.upload(cancelled) + } + + override fun getAll(): Collection = splitWayDB.getAll() + + override fun uploadSingle(changesetId: Long, quest: OsmQuestSplitWay, element: Element): List { + return splitSingleOsmWayUpload.upload(changesetId, element as Way, quest.splits) + } + + override fun onUploadSuccessful(quest: OsmQuestSplitWay) { + splitWayDB.delete(quest.questId) + Log.d(TAG, "Uploaded split way #${quest.wayId}") + } + + override fun onUploadFailed(quest: OsmQuestSplitWay, e: Throwable) { + splitWayDB.delete(quest.questId) + Log.d(TAG, "Dropped split for way #${quest.wayId}: ${e.message}") + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UndoOsmQuestChangesetsUpload.java b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UndoOsmQuestChangesetsUpload.java deleted file mode 100644 index e20663cb95..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UndoOsmQuestChangesetsUpload.java +++ /dev/null @@ -1,41 +0,0 @@ -package de.westnordost.streetcomplete.data.osm.upload; - - -import javax.inject.Inject; -import javax.inject.Named; -import javax.inject.Provider; - -import de.westnordost.osmapi.map.MapDataDao; -import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao; -import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao; -import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao; -import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestDao; -import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao; -import de.westnordost.streetcomplete.data.tiles.DownloadedTilesDao; - -public class UndoOsmQuestChangesetsUpload extends AOsmQuestChangesetsUpload -{ - @Inject public UndoOsmQuestChangesetsUpload( - MapDataDao osmDao, UndoOsmQuestDao questDB, MergedElementDao elementDB, - ElementGeometryDao elementGeometryDB, QuestStatisticsDao statisticsDB, - OpenChangesetsDao openChangesetsDB, DownloadedTilesDao downloadedTilesDao, - @Named("undo") Provider osmQuestChangeUploadProvider, - ChangesetAutoCloser changesetAutoCloser) - { - super(osmDao, questDB, elementDB, elementGeometryDB, statisticsDB, openChangesetsDB, - downloadedTilesDao, osmQuestChangeUploadProvider, changesetAutoCloser); - } - - @Override protected String getLogTag() - { - return "UndoOsmQuestChangesetsUpload"; - } - - @Override protected boolean shouldCheckForQuestApplicability() - { - // can't ask the quest here if it is applicable to the element or not, because the change - // of the revert is exactly the opposite of what the quest would normally change and the - // element ergo has the changes already applied that a normal quest would add - return false; - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UndoOsmQuestsUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UndoOsmQuestsUpload.kt new file mode 100644 index 0000000000..788b4146e6 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UndoOsmQuestsUpload.kt @@ -0,0 +1,58 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import android.util.Log +import de.westnordost.osmapi.map.data.Element +import de.westnordost.streetcomplete.data.osm.OsmQuestGiver +import de.westnordost.streetcomplete.data.osm.UndoOsmQuest +import de.westnordost.streetcomplete.data.osm.download.ElementGeometryCreator +import de.westnordost.streetcomplete.data.osm.persist.ElementGeometryDao +import javax.inject.Inject + +import de.westnordost.streetcomplete.data.osm.persist.MergedElementDao +import de.westnordost.streetcomplete.data.osm.persist.UndoOsmQuestDao +import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao +import java.util.* +import java.util.concurrent.atomic.AtomicBoolean + +/** Gets all undo osm quests from local DB and uploads them via the OSM API */ +class UndoOsmQuestsUpload @Inject constructor( + elementDB: MergedElementDao, + elementGeometryDB: ElementGeometryDao, + changesetManager: OpenQuestChangesetsManager, + questGiver: OsmQuestGiver, + statisticsDB: QuestStatisticsDao, + elementGeometryCreator: ElementGeometryCreator, + private val undoQuestDB: UndoOsmQuestDao, + private val singleChangeUpload: SingleOsmElementTagChangesUpload +) : OsmInChangesetsUpload(elementDB, elementGeometryDB, changesetManager, questGiver, + statisticsDB, elementGeometryCreator) { + + private val TAG = "UndoOsmQuestUpload" + + @Synchronized override fun upload(cancelled: AtomicBoolean) { + Log.i(TAG, "Undoing quest changes") + super.upload(cancelled) + } + + override fun getAll() = undoQuestDB.getAll() + + override fun uploadSingle(changesetId: Long, quest: UndoOsmQuest, element: Element): List { + return listOf(singleChangeUpload.upload(changesetId, quest, element)) + } + + override fun onUploadSuccessful(quest: UndoOsmQuest) { + undoQuestDB.delete(quest.id!!) + Log.d(TAG, "Uploaded undo osm quest ${quest.toLogString()}") + + } + + override fun onUploadFailed(quest: UndoOsmQuest, e: Throwable) { + undoQuestDB.delete(quest.id!!) + Log.d(TAG, "Dropped undo osm quest ${quest.toLogString()}: ${e.message}") + + } +} + +private fun UndoOsmQuest.toLogString() = + type.javaClass.simpleName + " for " + elementType.name.toLowerCase(Locale.US) + " #" + elementId + diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UpdateElementsHandler.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UpdateElementsHandler.kt new file mode 100644 index 0000000000..d762648870 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UpdateElementsHandler.kt @@ -0,0 +1,75 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import de.westnordost.osmapi.common.Handler +import de.westnordost.osmapi.map.changes.DiffElement +import de.westnordost.osmapi.map.data.* +import de.westnordost.streetcomplete.data.osm.ElementKey + +/** Reads the answer of an update map call on the OSM API. */ +class UpdateElementsHandler : Handler { + private val nodeDiffs: MutableMap = mutableMapOf() + private val wayDiffs: MutableMap = mutableMapOf() + private val relationDiffs: MutableMap = mutableMapOf() + + override fun handle(d: DiffElement) { + when (d.type ?: return) { + Element.Type.NODE -> nodeDiffs[d.clientId] = d + Element.Type.WAY -> wayDiffs[d.clientId] = d + Element.Type.RELATION -> relationDiffs[d.clientId] = d + } + } + + fun getElementUpdates(elements: Collection): ElementUpdates { + val updatedElements = mutableListOf() + val deletedElementKeys = mutableListOf() + for (element in elements) { + val update = getDiff(element.type, element.id) ?: continue + if (update.serverId != null && update.serverVersion != null) { + updatedElements.add(createUpdatedElement(element, update.serverId, update.serverVersion)) + } else { + deletedElementKeys.add(ElementKey(update.type, update.clientId)) + } + } + return ElementUpdates(updatedElements, deletedElementKeys) + } + + private fun getDiff(type: Element.Type, id: Long): DiffElement? = when (type) { + Element.Type.NODE -> nodeDiffs[id] + Element.Type.WAY -> wayDiffs[id] + Element.Type.RELATION -> relationDiffs[id] + } + + private fun createUpdatedElement(element: Element, newId: Long, newVersion: Int): Element = + when (element) { + is Node -> createUpdatedNode(element, newId, newVersion) + is Way -> createUpdatedWay(element, newId, newVersion) + is Relation -> createUpdatedRelation(element, newId, newVersion) + else -> throw RuntimeException() + } + + private fun createUpdatedNode(node: Node, newId: Long, newVersion: Int): Node { + return OsmNode(newId, newVersion, node.position, node.tags?.let { HashMap(it) }) + } + + private fun createUpdatedWay(way: Way, newId: Long, newVersion: Int): Way { + val newNodeIds = ArrayList(way.nodeIds.size) + for (nodeId in way.nodeIds) { + val update = nodeDiffs[nodeId] + if (update == null) newNodeIds.add(nodeId) + else if (update.serverId != null) newNodeIds.add(update.serverId) + } + return OsmWay(newId, newVersion, newNodeIds, way.tags?.let { HashMap(it) }) + } + + private fun createUpdatedRelation(relation: Relation, newId: Long, newVersion: Int): Relation { + val newRelationMembers = ArrayList(relation.members.size) + for (member in relation.members) { + val update = getDiff(member.type, member.ref) + if (update == null) newRelationMembers.add(OsmRelationMember(member.ref, member.role, member.type)) + else if(update.serverId != null) newRelationMembers.add(OsmRelationMember(update.serverId, member.role, member.type)) + } + return OsmRelation(newId, newVersion, newRelationMembers, relation.tags?.let { HashMap(it) }) + } +} + +data class ElementUpdates(val updated: Collection, val deleted: Collection) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UploadableInChangeset.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UploadableInChangeset.kt new file mode 100644 index 0000000000..e3be5488bb --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/upload/UploadableInChangeset.kt @@ -0,0 +1,11 @@ +package de.westnordost.streetcomplete.data.osm.upload + +import de.westnordost.osmapi.map.data.Element +import de.westnordost.streetcomplete.data.osm.OsmElementQuestType + +interface UploadableInChangeset { + val source: String + val osmElementQuestType: OsmElementQuestType<*> + val elementType: Element.Type + val elementId: Long +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AbstractCreateNoteFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AbstractCreateNoteFragment.kt index 4a2f971f87..cea25c0e8d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AbstractCreateNoteFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AbstractCreateNoteFragment.kt @@ -61,7 +61,7 @@ abstract class AbstractCreateNoteFragment : AbstractBottomSheetFragment() { } private fun onClickOk() { - onLeaveNote(noteText, attachPhotoFragment?.imagePaths) + onComposedNote(noteText, attachPhotoFragment?.imagePaths) } override fun onDiscard() { @@ -75,5 +75,5 @@ abstract class AbstractCreateNoteFragment : AbstractBottomSheetFragment() { doneButton.isEnabled = !noteText.isEmpty() } - protected abstract fun onLeaveNote(text: String, imagePaths: List?) + protected abstract fun onComposedNote(text: String, imagePaths: List?) } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AttachPhotoUtils.java b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AttachPhotoUtils.java index 2ec8bc7bb2..786614a49e 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AttachPhotoUtils.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AttachPhotoUtils.java @@ -3,6 +3,8 @@ import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.graphics.Matrix; + +import androidx.annotation.Nullable; import androidx.exifinterface.media.ExifInterface; import java.io.File; @@ -13,7 +15,7 @@ public class AttachPhotoUtils { - public static String uploadAndGetAttachedPhotosText(ImageUploader imageUploader, List imagePaths) + public static String uploadAndGetAttachedPhotosText(ImageUploader imageUploader, @Nullable List imagePaths) { if(imagePaths != null && !imagePaths.isEmpty()) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNote.java b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNote.java index 0bcb69f3ae..97fb31677b 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNote.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNote.java @@ -2,6 +2,7 @@ import java.util.List; +import androidx.annotation.Nullable; import de.westnordost.osmapi.map.data.Element; import de.westnordost.osmapi.map.data.LatLon; @@ -9,11 +10,11 @@ public class CreateNote { public long id; public String text; - public String questTitle; + @Nullable public String questTitle; public LatLon position; - public Element.Type elementType; - public Long elementId; - public List imagePaths; + @Nullable public Element.Type elementType; + @Nullable public Long elementId; + @Nullable public List imagePaths; public boolean hasAssociatedElement() { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteFragment.kt index f2104c357d..1e7ad91c3a 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteFragment.kt @@ -1,6 +1,5 @@ package de.westnordost.streetcomplete.data.osmnotes -import android.content.Context import android.content.res.Configuration import android.graphics.Point import android.os.Bundle @@ -21,9 +20,12 @@ import kotlinx.android.synthetic.main.marker_create_note.* class CreateNoteFragment : AbstractCreateNoteFragment() { - override val layoutResId = R.layout.fragment_create_note + interface Listener { + /** Called when the user wants to leave a note which is not related to a quest */ + fun onCreatedNote(note: String, imagePaths: List?, screenPosition: Point) + } - private lateinit var callbackListener: CreateNoteListener + override val layoutResId = R.layout.fragment_create_note override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) @@ -63,17 +65,12 @@ class CreateNoteFragment : AbstractCreateNoteFragment() { return a } - override fun onAttach(context: Context) { - super.onAttach(context) - callbackListener = context as CreateNoteListener - } - override fun onDiscard() { super.onDiscard() markerLayoutContainer?.visibility = View.INVISIBLE } - override fun onLeaveNote(text: String, imagePaths: List?) { + override fun onComposedNote(text: String, imagePaths: List?) { if (closeKeyboard()) return val point = IntArray(2) @@ -83,7 +80,7 @@ class CreateNoteFragment : AbstractCreateNoteFragment() { markerLayoutContainer?.visibility = View.INVISIBLE - callbackListener.onLeaveNote(text, imagePaths, screenPos) + (activity as Listener).onCreatedNote(text, imagePaths, screenPos) } private fun closeKeyboard(): Boolean { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteListener.java b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteListener.java deleted file mode 100644 index c993873d54..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteListener.java +++ /dev/null @@ -1,12 +0,0 @@ -package de.westnordost.streetcomplete.data.osmnotes; - -import android.graphics.Point; -import androidx.annotation.Nullable; - -import java.util.List; - -public interface CreateNoteListener -{ - /** Called when the user wants to leave a note which is not related to a quest */ - void onLeaveNote(String note, @Nullable List imagePaths, Point screenPosition); -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteUpload.java b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteUpload.java deleted file mode 100644 index 65e23c51d6..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNoteUpload.java +++ /dev/null @@ -1,256 +0,0 @@ -package de.westnordost.streetcomplete.data.osmnotes; - -import android.util.Log; - -import java.util.List; -import java.util.Locale; -import java.util.concurrent.atomic.AtomicBoolean; - -import javax.inject.Inject; - -import de.westnordost.streetcomplete.ApplicationConstants; -import de.westnordost.streetcomplete.data.QuestStatus; -import de.westnordost.osmapi.common.SingleElementHandler; -import de.westnordost.osmapi.common.errors.OsmConflictException; -import de.westnordost.osmapi.map.MapDataDao; -import de.westnordost.osmapi.map.data.BoundingBox; -import de.westnordost.osmapi.map.data.Element; -import de.westnordost.osmapi.notes.Note; -import de.westnordost.osmapi.notes.NotesDao; -import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao; -import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener; -import de.westnordost.streetcomplete.util.ImageUploader; - -public class CreateNoteUpload -{ - private static final String TAG = "CreateNoteUpload"; - - private final NotesDao osmDao; - private final CreateNoteDao createNoteDB; - private final NoteDao noteDB; - private final OsmNoteQuestDao noteQuestDB; - private final MapDataDao mapDataDao; - private final OsmNoteQuestType questType; - private final QuestStatisticsDao statisticsDB; - private final ImageUploader imageUploader; - private OnUploadedChangeListener uploadedChangeListener; - - @Inject public CreateNoteUpload( - CreateNoteDao createNoteDB, NotesDao osmDao, NoteDao noteDB, - OsmNoteQuestDao noteQuestDB, MapDataDao mapDataDao, OsmNoteQuestType questType, - QuestStatisticsDao statisticsDB, ImageUploader imageUploader) - { - this.createNoteDB = createNoteDB; - this.noteQuestDB = noteQuestDB; - this.noteDB = noteDB; - this.osmDao = osmDao; - this.mapDataDao = mapDataDao; - this.questType = questType; - this.statisticsDB = statisticsDB; - this.imageUploader = imageUploader; - } - - public synchronized void setProgressListener(OnUploadedChangeListener uploadedChangeListener) - { - this.uploadedChangeListener = uploadedChangeListener; - } - - public synchronized void upload(AtomicBoolean cancelState) - { - int created = 0, obsolete = 0; - for(CreateNote createNote : createNoteDB.getAll(null)) - { - if(cancelState.get()) break; - - if(uploadCreateNote(createNote) != null) - { - uploadedChangeListener.onUploaded(); - created++; - } - else - { - uploadedChangeListener.onDiscarded(); - obsolete++; - } - } - String logMsg = "Created " + created + " notes"; - if(obsolete > 0) - { - logMsg += " but dropped " + obsolete + " because they were obsolete already"; - } - Log.i(TAG, logMsg); - } - - Note uploadCreateNote(CreateNote n) - { - if(isAssociatedElementDeleted(n)) - { - Log.i(TAG, "Dropped to be created note " + getCreateNoteStringForLog(n) + - " because the associated element has already been deleted"); - deleteNote(n); - return null; - } - - Note newNote = createOrCommentNote(n); - - if (newNote != null) - { - // add a closed quest as a blocker so that at this location no quests are created. - // if the note was not added, don't do this (see below) -> probably based on old data - OsmNoteQuest noteQuest = new OsmNoteQuest(newNote, questType); - noteQuest.setStatus(QuestStatus.CLOSED); - noteDB.put(newNote); - noteQuestDB.add(noteQuest); - statisticsDB.addOneNote(); - } - else - { - Log.i(TAG, "Dropped a to be created note " + getCreateNoteStringForLog(n) + - " because a note with the same associated element has already been closed"); - // so the problem has likely been solved by another mapper - } - - deleteNote(n); - - return newNote; - } - - private void deleteNote(CreateNote n) - { - createNoteDB.delete(n.id); - AttachPhotoUtils.deleteImages(n.imagePaths); - } - - private static String getCreateNoteStringForLog(CreateNote n) - { - return "\"" + n.text + "\" at " + n.position.getLatitude() + ", " + n.position.getLongitude(); - } - - private boolean isAssociatedElementDeleted(CreateNote n) - { - return n.hasAssociatedElement() && retrieveElement(n) == null; - } - - private Element retrieveElement(CreateNote n) - { - switch(n.elementType) - { - case NODE: return mapDataDao.getNode(n.elementId); - case WAY: return mapDataDao.getWay(n.elementId); - case RELATION: return mapDataDao.getRelation(n.elementId); - } - return null; - } - - /** Create a note at the given position, or, if there is already a note at the exact same - * position AND its associated element is the same, adds the user's message as another comment. - * - * Returns null and does not add the note comment if that note has already been closed because - * the contribution is very likely obsolete (based on old data)*/ - private Note createOrCommentNote(CreateNote n) - { - if(n.hasAssociatedElement()) - { - Note oldNote = findAlreadyExistingNoteWithSameAssociatedElement(n); - if(oldNote != null) - { - return commentNote(oldNote, n.text, n.imagePaths); - } - } - return createNote(n); - } - - private Note createNote(CreateNote n) - { - String text = getCreateNoteText(n); - text += AttachPhotoUtils.uploadAndGetAttachedPhotosText(imageUploader, n.imagePaths); - Note result = osmDao.create(n.position, text); - imageUploader.activate(result.id); - return result; - } - - private Note commentNote(Note note, String text, List attachedImagePaths) - { - if(note.isOpen()) - { - try - { - text += AttachPhotoUtils.uploadAndGetAttachedPhotosText(imageUploader, attachedImagePaths); - Note result = osmDao.comment(note.id, text); - imageUploader.activate(result.id); - return result; - } - catch (OsmConflictException e) - { - return null; - } - } - else - { - return null; - } - } - - private static String getCreateNoteText(CreateNote note) - { - if(note.hasAssociatedElement()) - { - if(note.questTitle != null) - { - return "Unable to answer \"" + note.questTitle + "\"" + - " for " + getAssociatedElementString(note) + - " via "+ ApplicationConstants.USER_AGENT+":\n\n" + note.text; - } - else - { - return "for " + getAssociatedElementString(note) + - " via "+ ApplicationConstants.USER_AGENT+":\n\n" + note.text; - } - } - return note.text + "\n\nvia "+ ApplicationConstants.USER_AGENT; - } - - private Note findAlreadyExistingNoteWithSameAssociatedElement(final CreateNote newNote) - { - SingleElementHandler handler = new SingleElementHandler() - { - @Override public void handle(Note oldNote) - { - if(newNote.hasAssociatedElement()) - { - String firstCommentText = oldNote.comments.get(0).text; - String newNoteRegex = getAssociatedElementRegex(newNote); - if(firstCommentText.matches(newNoteRegex)) - { - super.handle(oldNote); - } - } - } - }; - final int hideClosedNoteAfter = 7; - osmDao.getAll(new BoundingBox( - newNote.position.getLatitude(), newNote.position.getLongitude(), - newNote.position.getLatitude(), newNote.position.getLongitude() - ), handler, 10, hideClosedNoteAfter); - return handler.get(); - } - - private static String getAssociatedElementRegex(CreateNote n) - { - String elementType = n.elementType.name(); - // before 0.11 - i.e. "way #123" - String oldStyleRegex = elementType+"\\s*#"+n.elementId; - // i.e. www.openstreetmap.org/way/123 - String newStyleRegex = "(osm|openstreetmap)\\.org\\/"+elementType+"\\/"+n.elementId; - // i: turns on case insensitive regex, s: newlines are also captured with "." - return "(?is).*(("+oldStyleRegex+")|("+newStyleRegex+")).*"; - } - - static String getAssociatedElementString(CreateNote n) - { - if(!n.hasAssociatedElement()) return null; - - String elementName = n.elementType.name().toLowerCase(Locale.UK); - return "https://osm.org/" + elementName + "/" + n.elementId; - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNotesUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNotesUpload.kt new file mode 100644 index 0000000000..1b2e9b1e8c --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/CreateNotesUpload.kt @@ -0,0 +1,91 @@ +package de.westnordost.streetcomplete.data.osmnotes + +import android.util.Log + +import javax.inject.Inject + +import de.westnordost.streetcomplete.data.QuestStatus +import de.westnordost.osmapi.map.MapDataDao +import de.westnordost.osmapi.map.data.Element +import de.westnordost.osmapi.notes.Note +import de.westnordost.streetcomplete.data.osm.upload.ConflictException +import de.westnordost.streetcomplete.data.osm.upload.ElementDeletedException +import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao +import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener +import java.util.concurrent.atomic.AtomicBoolean + +/** Gets all create notes from local DB and uploads them via the OSM API */ +class CreateNotesUpload @Inject constructor( + private val createNoteDB: CreateNoteDao, + private val noteDB: NoteDao, + private val noteQuestDB: OsmNoteQuestDao, + private val mapDataDao: MapDataDao, + private val questType: OsmNoteQuestType, + private val statisticsDB: QuestStatisticsDao, + private val singleCreateNoteUpload: SingleCreateNoteUpload +) { + private val TAG = "CreateNotesUpload" + + var uploadedChangeListener: OnUploadedChangeListener? = null + + @Synchronized fun upload(cancelled: AtomicBoolean) { + var created = 0 + var obsolete = 0 + if (cancelled.get()) return + Log.i(TAG, "Uploading create notes") + for (createNote in createNoteDB.getAll(null)) { + if (cancelled.get()) break + + try { + val newNote = uploadSingle(createNote) + + // add a closed quest as a blocker so that at this location no quests are created. + // if the note was not added, don't do this (see below) -> probably based on old data + val noteQuest = OsmNoteQuest(newNote, questType) + noteQuest.status = QuestStatus.CLOSED + noteDB.put(newNote) + noteQuestDB.add(noteQuest) + statisticsDB.addOneNote() + + Log.d(TAG, "Uploaded note ${createNote.logString}") + uploadedChangeListener?.onUploaded() + created++ + } catch (e: ConflictException) { + Log.d(TAG, "Dropped note ${createNote.logString}: ${e.message}") + uploadedChangeListener?.onDiscarded() + obsolete++ + } + + createNoteDB.delete(createNote.id) + AttachPhotoUtils.deleteImages(createNote.imagePaths) + } + var logMsg = "Created $created notes" + if (obsolete > 0) { + logMsg += " but dropped $obsolete because they were obsolete already" + } + Log.i(TAG, logMsg) + } + + private fun uploadSingle(n: CreateNote): Note { + if (n.isAssociatedElementDeleted()) + throw ElementDeletedException("Associated element deleted") + + return singleCreateNoteUpload.upload(n) + } + + private fun CreateNote.isAssociatedElementDeleted(): Boolean { + return hasAssociatedElement() && fetchElement() == null + } + + private fun CreateNote.fetchElement(): Element? { + val type = elementType ?: return null + val id = elementId ?: return null + return when (type) { + Element.Type.NODE -> mapDataDao.getNode(id) + Element.Type.WAY -> mapDataDao.getWay(id) + Element.Type.RELATION -> mapDataDao.getRelation(id) + } + } +} + +private val CreateNote.logString get() = "\"$text\" at ${position.latitude}, ${position.longitude}" diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuest.java b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuest.java index d931704bb5..e1a1fb4859 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuest.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuest.java @@ -3,6 +3,7 @@ import java.util.Date; import java.util.List; +import androidx.annotation.Nullable; import de.westnordost.streetcomplete.data.Quest; import de.westnordost.streetcomplete.data.QuestStatus; import de.westnordost.streetcomplete.data.QuestType; @@ -17,8 +18,8 @@ public OsmNoteQuest(Note note, OsmNoteQuestType osmNoteQuestType) this(null, note, QuestStatus.NEW, null, new Date(), osmNoteQuestType, null); } - public OsmNoteQuest(Long id, Note note, QuestStatus status, String comment, Date lastUpdate, - OsmNoteQuestType questType, List imagePaths) + public OsmNoteQuest(@Nullable Long id, Note note, QuestStatus status, @Nullable String comment, Date lastUpdate, + OsmNoteQuestType questType, @Nullable List imagePaths) { this.id = id; this.note = note; @@ -29,14 +30,14 @@ public OsmNoteQuest(Long id, Note note, QuestStatus status, String comment, Date this.imagePaths = imagePaths; } - private Long id; + @Nullable private Long id; private Date lastUpdate; private QuestStatus status; private Note note; - private String comment; + @Nullable private String comment; - private List imagePaths; + @Nullable private List imagePaths; private final OsmNoteQuestType questType; @@ -97,12 +98,12 @@ public void setNote(Note note) this.note = note; } - public String getComment() + @Nullable public String getComment() { return comment; } - public void setComment(String comment) + public void setComment(@Nullable String comment) { this.comment = comment; } @@ -117,12 +118,12 @@ public void setComment(String comment) this.id = id; } - public void setImagePaths(List imagePaths) + public void setImagePaths(@Nullable List imagePaths) { this.imagePaths = imagePaths; } - public List getImagePaths() + @Nullable public List getImagePaths() { return imagePaths; } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuestChangesUpload.java b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuestChangesUpload.java deleted file mode 100644 index fabfd0f38c..0000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuestChangesUpload.java +++ /dev/null @@ -1,117 +0,0 @@ -package de.westnordost.streetcomplete.data.osmnotes; - -import android.util.Log; - -import java.util.concurrent.atomic.AtomicBoolean; - -import javax.inject.Inject; - -import de.westnordost.osmapi.common.errors.OsmNotFoundException; -import de.westnordost.streetcomplete.data.QuestStatus; -import de.westnordost.osmapi.common.errors.OsmConflictException; -import de.westnordost.osmapi.map.data.LatLon; -import de.westnordost.osmapi.notes.Note; -import de.westnordost.osmapi.notes.NotesDao; -import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao; -import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener; -import de.westnordost.streetcomplete.util.ImageUploader; - -public class OsmNoteQuestChangesUpload -{ - private static final String TAG = "CommentNoteUpload"; - - private final NotesDao osmDao; - private final OsmNoteQuestDao questDB; - private final QuestStatisticsDao statisticsDB; - private final NoteDao noteDB; - private final ImageUploader imageUploader; - private OnUploadedChangeListener uploadedChangeListener; - - @Inject public OsmNoteQuestChangesUpload( - NotesDao osmDao, OsmNoteQuestDao questDB, QuestStatisticsDao statisticsDB, - NoteDao noteDB, ImageUploader imageUploader) - { - this.osmDao = osmDao; - this.questDB = questDB; - this.statisticsDB = statisticsDB; - this.noteDB = noteDB; - this.imageUploader = imageUploader; - } - - public synchronized void setProgressListener(OnUploadedChangeListener uploadedChangeListener) - { - this.uploadedChangeListener = uploadedChangeListener; - } - - public synchronized void upload(AtomicBoolean cancelState) - { - int created = 0, obsolete = 0; - for(OsmNoteQuest quest : questDB.getAll(null, QuestStatus.ANSWERED)) - { - if(cancelState.get()) break; - - if(uploadNoteChanges(quest) != null) - { - uploadedChangeListener.onUploaded(); - created++; - } - else - { - uploadedChangeListener.onDiscarded(); - obsolete++; - } - } - String logMsg = "Commented on " + created + " notes"; - if(obsolete > 0) - { - logMsg += " but dropped " + obsolete + " comments because the notes have already been closed"; - } - Log.i(TAG, logMsg); - } - - Note uploadNoteChanges(OsmNoteQuest quest) - { - String text = quest.getComment(); - - try - { - text += AttachPhotoUtils.uploadAndGetAttachedPhotosText(imageUploader, quest.getImagePaths()); - Note newNote = osmDao.comment(quest.getNote().id, text); - imageUploader.activate(newNote.id); - - /* Unlike OSM quests, note quests are never deleted when the user contributed to it - but must remain in the database with the status CLOSED as long as they are not - solved. The reason is because as long as a note is unsolved, the problem at that - position persists and thus it should still block other quests to be created. - (Reminder: Note quests block other quests) - */ - // so, not this: questDB.delete(quest.getId()); - quest.setStatus(QuestStatus.CLOSED); - quest.setNote(newNote); - questDB.update(quest); - noteDB.put(newNote); - statisticsDB.addOneNote(); - AttachPhotoUtils.deleteImages(quest.getImagePaths()); - - return newNote; - } - catch(OsmNotFoundException | OsmConflictException e) - { - // someone else already closed the note -> our contribution is probably worthless. Delete - questDB.delete(quest.getId()); - noteDB.delete(quest.getNote().id); - AttachPhotoUtils.deleteImages(quest.getImagePaths()); - - Log.i(TAG, "Dropped the comment " + getNoteQuestStringForLog(quest) + - " because the note has already been closed"); - - return null; - } - } - - private static String getNoteQuestStringForLog(OsmNoteQuest n) - { - LatLon pos = n.getCenter(); - return "\"" + n.getComment() + "\" at " + pos.getLatitude() + ", " + pos.getLongitude(); - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuestsChangesUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuestsChangesUpload.kt new file mode 100644 index 0000000000..cfa0b51830 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/OsmNoteQuestsChangesUpload.kt @@ -0,0 +1,68 @@ +package de.westnordost.streetcomplete.data.osmnotes + +import android.util.Log + +import javax.inject.Inject + +import de.westnordost.streetcomplete.data.QuestStatus +import de.westnordost.streetcomplete.data.osm.upload.ConflictException +import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao +import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener +import java.util.concurrent.atomic.AtomicBoolean + +/** Gets all note quests from local DB and uploads them via the OSM API */ +class OsmNoteQuestsChangesUpload @Inject constructor( + private val questDB: OsmNoteQuestDao, + private val statisticsDB: QuestStatisticsDao, + private val noteDB: NoteDao, + private val singleNoteUpload: SingleOsmNoteQuestChangesUpload +) { + private val TAG = "CommentNoteUpload" + + var uploadedChangeListener: OnUploadedChangeListener? = null + + @Synchronized fun upload(cancelled: AtomicBoolean) { + var created = 0 + var obsolete = 0 + if (cancelled.get()) return + for (quest in questDB.getAll(null, QuestStatus.ANSWERED)) { + if (cancelled.get()) break + + try { + val newNote = singleNoteUpload.upload(quest) + + /* Unlike OSM quests, note quests are never deleted when the user contributed to it + but must remain in the database with the status CLOSED as long as they are not + solved. The reason is because as long as a note is unsolved, the problem at that + position persists and thus it should still block other quests to be created. + (Reminder: Note quests block other quests) + */ + // so, not this: questDB.delete(quest.getId()); + quest.status = QuestStatus.CLOSED + quest.note = newNote + questDB.update(quest) + noteDB.put(newNote) + statisticsDB.addOneNote() + + Log.d(TAG, "Uploaded note comment ${quest.logString}") + uploadedChangeListener?.onUploaded() + created++ + } catch (e: ConflictException) { + questDB.delete(quest.id!!) + noteDB.delete(quest.note.id) + + Log.d(TAG, "Dropped note comment ${quest.logString}: ${e.message}") + uploadedChangeListener?.onDiscarded() + obsolete++ + } + AttachPhotoUtils.deleteImages(quest.imagePaths) + } + var logMsg = "Commented on $created notes" + if (obsolete > 0) { + logMsg += " but dropped $obsolete comments because the notes have already been closed" + } + Log.i(TAG, logMsg) + } +} + +private val OsmNoteQuest.logString get() = "\"$comment\" at ${center.latitude}, ${center.longitude}" diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/SingleCreateNoteUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/SingleCreateNoteUpload.kt new file mode 100644 index 0000000000..d2ec5bed42 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/SingleCreateNoteUpload.kt @@ -0,0 +1,106 @@ +package de.westnordost.streetcomplete.data.osmnotes + +import de.westnordost.osmapi.common.SingleElementHandler +import de.westnordost.osmapi.common.errors.OsmConflictException +import de.westnordost.osmapi.map.data.BoundingBox +import de.westnordost.osmapi.notes.Note +import de.westnordost.osmapi.notes.NotesDao +import de.westnordost.streetcomplete.ApplicationConstants.USER_AGENT +import de.westnordost.streetcomplete.data.osm.upload.ConflictException +import de.westnordost.streetcomplete.util.ImageUploader +import java.util.* +import javax.inject.Inject + +/** Create a note at the given position, or, if there is already a note at the exact same + * position AND its associated element is the same, adds the user's message as another comment. + * + * Throws an ElementConflictException and does not add the note comment if that note has already + * been closed because the contribution is very likely obsolete (based on old data) */ +class SingleCreateNoteUpload @Inject constructor( + private val osmDao: NotesDao, + private val imageUploader: ImageUploader +) { + fun upload(n: CreateNote): Note { + if (n.hasAssociatedElement()) { + val oldNote = findAlreadyExistingNoteWithSameAssociatedElement(n) + if (oldNote != null) { + return commentNote(oldNote, n.text, n.imagePaths) + } + } + return createNote(n) + } + + private fun createNote(n: CreateNote): Note { + val attachedPhotosText = AttachPhotoUtils.uploadAndGetAttachedPhotosText(imageUploader, n.imagePaths) + val result = osmDao.create(n.position, n.fullNoteText + attachedPhotosText) + if (!n.imagePaths.isNullOrEmpty()) { + imageUploader.activate(result.id) + } + return result + } + + private fun commentNote(note: Note, text: String, attachedImagePaths: List?): Note { + return if (note.isOpen) { + try { + val attachedPhotosText = AttachPhotoUtils.uploadAndGetAttachedPhotosText(imageUploader, attachedImagePaths) + val result = osmDao.comment(note.id, text + attachedPhotosText) + if (!attachedImagePaths.isNullOrEmpty()) { + imageUploader.activate(result.id) + } + result + } catch (e: OsmConflictException) { + throw ConflictException(e.message, e) + } + } else throw ConflictException("Note already closed") + } + + + private fun findAlreadyExistingNoteWithSameAssociatedElement(newNote: CreateNote): Note? { + val handler = object : SingleElementHandler() { + override fun handle(oldNote: Note) { + val newNoteRegex = newNote.associatedElementRegex + if (newNoteRegex != null) { + val firstCommentText = oldNote.comments[0].text + if (firstCommentText.matches(newNoteRegex.toRegex())) { + super.handle(oldNote) + } + } + } + } + val bbox = BoundingBox( + newNote.position.latitude, newNote.position.longitude, + newNote.position.latitude, newNote.position.longitude + ) + val hideClosedNoteAfter = 7 + osmDao.getAll(bbox, handler, 10, hideClosedNoteAfter) + return handler.get() + } +} + +private val CreateNote.fullNoteText: String get() { + return if (hasAssociatedElement()) { + val title = questTitle + if (title != null) { + "Unable to answer \"$title\" for $associatedElementString via $USER_AGENT:\n\n$text" + } else { + "for $associatedElementString via $USER_AGENT:\n\n$text" + } + } else "$text\n\nvia $USER_AGENT" +} + +private val CreateNote.associatedElementRegex: String? get() { + val elementTypeName = elementType?.name ?: return null + val elementId = elementId ?: return null + // before 0.11 - i.e. "way #123" + val oldStyleRegex = "$elementTypeName\\s*#$elementId" + // i.e. www.openstreetmap.org/way/123 + val newStyleRegex = "(osm|openstreetmap)\\.org\\/$elementTypeName\\/$elementId" + // i: turns on case insensitive regex, s: newlines are also captured with "." + return "(?is).*(($oldStyleRegex)|($newStyleRegex)).*" +} + +private val CreateNote.associatedElementString: String? get() { + val lowercaseTypeName = elementType?.name?.toLowerCase(Locale.UK) ?: return null + val elementId = elementId ?: return null + return "https://osm.org/$lowercaseTypeName/$elementId" +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/SingleOsmNoteQuestChangesUpload.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/SingleOsmNoteQuestChangesUpload.kt new file mode 100644 index 0000000000..8562b04d6c --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/SingleOsmNoteQuestChangesUpload.kt @@ -0,0 +1,32 @@ +package de.westnordost.streetcomplete.data.osmnotes + +import de.westnordost.osmapi.common.errors.OsmConflictException +import de.westnordost.osmapi.common.errors.OsmNotFoundException +import de.westnordost.osmapi.notes.Note +import de.westnordost.osmapi.notes.NotesDao +import de.westnordost.streetcomplete.data.osm.upload.ConflictException +import de.westnordost.streetcomplete.util.ImageUploader +import javax.inject.Inject + +/** Uploads a single note quest to OSM */ +class SingleOsmNoteQuestChangesUpload @Inject constructor( + private val osmDao: NotesDao, + private val imageUploader: ImageUploader +){ + + fun upload(quest: OsmNoteQuest): Note { + try { + val attachedPhotosText = AttachPhotoUtils.uploadAndGetAttachedPhotosText(imageUploader, quest.imagePaths) + val newNote = osmDao.comment(quest.note.id, quest.comment + attachedPhotosText) + if (!quest.imagePaths.isNullOrEmpty()) { + imageUploader.activate(newNote.id) + } + return newNote + } catch (e: OsmNotFoundException) { + // someone else already closed the note -> our contribution is probably worthless + throw ConflictException(e.message, e) + } catch (e: OsmConflictException) { + throw ConflictException(e.message, e) + } + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/upload/QuestChangesUploadService.java b/app/src/main/java/de/westnordost/streetcomplete/data/upload/QuestChangesUploadService.java index 5f545440fa..9eb4a4710f 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/upload/QuestChangesUploadService.java +++ b/app/src/main/java/de/westnordost/streetcomplete/data/upload/QuestChangesUploadService.java @@ -15,32 +15,33 @@ import java.util.concurrent.atomic.AtomicBoolean; import javax.inject.Inject; -import javax.inject.Provider; import de.westnordost.osmapi.common.errors.OsmAuthorizationException; import de.westnordost.streetcomplete.ApplicationConstants; import de.westnordost.streetcomplete.Injector; import de.westnordost.streetcomplete.data.VisibleQuestListener; import de.westnordost.streetcomplete.data.VisibleQuestRelay; -import de.westnordost.streetcomplete.data.osm.upload.OsmQuestChangesetsUpload; -import de.westnordost.streetcomplete.data.osm.upload.UndoOsmQuestChangesetsUpload; -import de.westnordost.streetcomplete.data.osmnotes.CreateNoteUpload; -import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestChangesUpload; +import de.westnordost.streetcomplete.data.osm.upload.OsmQuestsUpload; +import de.westnordost.streetcomplete.data.osm.upload.SplitWaysUpload; +import de.westnordost.streetcomplete.data.osm.upload.UndoOsmQuestsUpload; +import de.westnordost.streetcomplete.data.osmnotes.CreateNotesUpload; +import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestsChangesUpload; import de.westnordost.streetcomplete.oauth.OAuthPrefs; /** Collects and uploads all changes the user has done: notes he left, comments he left on existing * notes and quests he answered */ public class QuestChangesUploadService extends IntentService { - private static final String TAG = "QuestChangesUpload"; + private static final String TAG = "Upload"; private static Boolean banned = null; private static String banReason = null; - @Inject Provider noteQuestUploadProvider; - @Inject Provider questUploadProvider; - @Inject Provider undoQuestUploadProvider; - @Inject Provider createNoteUploadProvider; + @Inject OsmNoteQuestsChangesUpload noteQuestUpload; + @Inject OsmQuestsUpload questUpload; + @Inject UndoOsmQuestsUpload undoQuestUpload; + @Inject CreateNotesUpload createNoteUpload; + @Inject SplitWaysUpload splitWaysUpload; @Inject OAuthPrefs oAuth; private final IBinder binder = new Interface(); @@ -108,35 +109,37 @@ public QuestChangesUploadService() throw new OsmAuthorizationException(401, "Unauthorized", "User is not authorized"); } - Log.i(TAG, "Starting upload changes"); + Log.i(TAG, "Starting upload"); - OsmNoteQuestChangesUpload noteQuestUpload = noteQuestUploadProvider.get(); - noteQuestUpload.setProgressListener(uploadedChangeRelay); + noteQuestUpload.setUploadedChangeListener(uploadedChangeRelay); noteQuestUpload.upload(cancelState); if (cancelState.get()) return; - UndoOsmQuestChangesetsUpload undoOsmQuestUpload = undoQuestUploadProvider.get(); - undoOsmQuestUpload.setProgressListener(uploadedChangeRelay); - undoOsmQuestUpload.setVisibleQuestListener(visibleQuestRelay); - undoOsmQuestUpload.upload(cancelState); + undoQuestUpload.setUploadedChangeListener(uploadedChangeRelay); + undoQuestUpload.setVisibleQuestListener(visibleQuestRelay); + undoQuestUpload.upload(cancelState); if (cancelState.get()) return; - OsmQuestChangesetsUpload osmQuestUpload = questUploadProvider.get(); - osmQuestUpload.setProgressListener(uploadedChangeRelay); - osmQuestUpload.setVisibleQuestListener(visibleQuestRelay); - osmQuestUpload.upload(cancelState); + questUpload.setUploadedChangeListener(uploadedChangeRelay); + questUpload.setVisibleQuestListener(visibleQuestRelay); + questUpload.upload(cancelState); if (cancelState.get()) return; - CreateNoteUpload createNoteUpload = createNoteUploadProvider.get(); - createNoteUpload.setProgressListener(uploadedChangeRelay); - createNoteUpload.upload(cancelState); + splitWaysUpload.setUploadedChangeListener(uploadedChangeRelay); + splitWaysUpload.setVisibleQuestListener(visibleQuestRelay); + splitWaysUpload.upload(cancelState); + + if (cancelState.get()) return; + + createNoteUpload.setUploadedChangeListener(uploadedChangeRelay); + createNoteUpload.upload(new AtomicBoolean(false)); } catch (Exception e) { - Log.e(TAG, "Unable to upload changes", e); + Log.e(TAG, "Unable to upload", e); if(progressListener != null) { progressListener.onError(e); @@ -148,7 +151,7 @@ public QuestChangesUploadService() progressListener.onFinished(); } - Log.i(TAG, "Finished upload changes"); + Log.i(TAG, "Finished upload"); } private static boolean isBanned() diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/visiblequests/OrderedVisibleQuestTypesProvider.kt b/app/src/main/java/de/westnordost/streetcomplete/data/visiblequests/OrderedVisibleQuestTypesProvider.kt new file mode 100644 index 0000000000..faa2dff86c --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/visiblequests/OrderedVisibleQuestTypesProvider.kt @@ -0,0 +1,21 @@ +package de.westnordost.streetcomplete.data.visiblequests + +import de.westnordost.streetcomplete.data.QuestType +import de.westnordost.streetcomplete.data.QuestTypeRegistry +import javax.inject.Inject + +class OrderedVisibleQuestTypesProvider @Inject constructor( + private val questTypeRegistry: QuestTypeRegistry, + private val visibleQuestTypeDao: VisibleQuestTypeDao, + private val questTypeOrderList: QuestTypeOrderList +) { + fun get(): List> { + val visibleQuestTypes = questTypeRegistry.all.mapNotNull { questType -> + questType.takeIf { visibleQuestTypeDao.isVisible(it) } + }.toMutableList() + + questTypeOrderList.sort(visibleQuestTypes) + + return visibleQuestTypes + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/ktx/Collections.kt b/app/src/main/java/de/westnordost/streetcomplete/ktx/Collections.kt new file mode 100644 index 0000000000..a4ca8a0a4a --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/ktx/Collections.kt @@ -0,0 +1,64 @@ +package de.westnordost.streetcomplete.ktx + +/** Return the first and last element of this list. If it contains only one element, just that one */ +fun List.firstAndLast() = if (size == 1) listOf(first()) else listOf(first(), last()) + +/** Returns whether the collection contains any of the [elements] */ +fun Collection.containsAny(elements: Collection) = elements.any { contains(it) } + +/** + * Starting at [index] (exclusive), iterating the list in reverse, returns the first element that + * matches the given [predicate], or `null` if no such element was found. + */ +inline fun List.findPrevious(index: Int, predicate: (T) -> Boolean): T? { + val iterator = this.listIterator(index) + while (iterator.hasPrevious()) { + val element = iterator.previous() + if (predicate(element)) return element + } + return null +} + +/** + * Starting at [index] (inclusive), iterating the list, returns the first element that + * matches the given [predicate], or `null` if no such element was found. + */ +inline fun List.findNext(index: Int, predicate: (T) -> Boolean): T? { + val iterator = this.listIterator(index) + while (iterator.hasNext()) { + val element = iterator.next() + if (predicate(element)) return element + } + return null +} + +/** Iterate through the given list in pairs */ +inline fun Iterable.forEachPair(predicate: (first: T, second: T) -> Unit) { + val it = iterator() + if (!it.hasNext()) return + var item1 = it.next() + while (it.hasNext()) { + val item2 = it.next() + predicate(item1, item2) + item1 = item2 + } +} + +/** returns the index of the first element yielding the largest value of the given function or -1 + * if there are no elements. Analogous to the maxBy extension function. */ +inline fun > Iterable.indexOfMaxBy(selector: (T) -> R): Int { + val iterator = iterator() + if (!iterator.hasNext()) return -1 + var indexOfMaxElem = 0 + var i = 0 + var maxValue = selector(iterator.next()) + while (iterator.hasNext()) { + ++i + val v = selector(iterator.next()) + if (maxValue < v) { + indexOfMaxElem = i + maxValue = v + } + } + return indexOfMaxElem +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/ktx/Cursor.kt b/app/src/main/java/de/westnordost/streetcomplete/ktx/Cursor.kt new file mode 100644 index 0000000000..172be7ad36 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/ktx/Cursor.kt @@ -0,0 +1,59 @@ +package de.westnordost.streetcomplete.ktx + +import android.content.ContentValues +import android.database.Cursor +import androidx.core.database.* + +fun Cursor.getLong(columnName: String): Long = getLong(getColumnIndexOrThrow(columnName)) +fun Cursor.getInt(columnName: String): Int = getInt(getColumnIndexOrThrow(columnName)) +fun Cursor.getShort(columnName: String): Short = getShort(getColumnIndexOrThrow(columnName)) +fun Cursor.getDouble(columnName: String): Double = getDouble(getColumnIndexOrThrow(columnName)) +fun Cursor.getFloat(columnName: String): Float = getFloat(getColumnIndexOrThrow(columnName)) +fun Cursor.getString(columnName: String): String = getString(getColumnIndexOrThrow(columnName)) +fun Cursor.getBlob(columnName: String): ByteArray = getBlob(getColumnIndexOrThrow(columnName)) + +fun Cursor.getLongOrNull(columnName: String): Long? = getLongOrNull(getColumnIndexOrThrow(columnName)) +fun Cursor.getIntOrNull(columnName: String): Int? = getIntOrNull(getColumnIndexOrThrow(columnName)) +fun Cursor.getShortOrNull(columnName: String): Short? = getShortOrNull(getColumnIndexOrThrow(columnName)) +fun Cursor.getDoubleOrNull(columnName: String): Double? = getDoubleOrNull(getColumnIndexOrThrow(columnName)) +fun Cursor.getFloatOrNull(columnName: String): Float? = getFloatOrNull(getColumnIndexOrThrow(columnName)) +fun Cursor.getStringOrNull(columnName: String): String? = getStringOrNull(getColumnIndexOrThrow(columnName)) +fun Cursor.getBlobOrNull(columnName: String): ByteArray? = getBlobOrNull(getColumnIndexOrThrow(columnName)) + +@Deprecated("Better not use until/if there are union types that make this typesafe on compile time") +// still keeping it around though... +inline fun Cursor.get(columnName: String): T { + val index = getColumnIndexOrThrow(columnName) + return when(T::class) { + Long::class -> getLong(index) + Int::class -> getInt(index) + Short::class -> getShort(index) + Double::class -> getDouble(index) + Float::class -> getFloat(index) + String::class -> getString(index) + ByteArray::class -> getBlob(index) + else -> throw ClassCastException("Expected either an Int, Short, Long, Float, Double, String or ByteArray") + } as T +} + +@Deprecated("Better not use until/if there are union types that make this typesafe on compile time") +// still keeping it around though... +fun contentValuesOf(vararg pairs: Pair): ContentValues { + val r = ContentValues() + for ((key, value) in pairs) { + when(value) { + null -> r.putNull(key) + is Long -> r.put(key, value) + is Int -> r.put(key, value) + is Short -> r.put(key, value) + is Byte -> r.put(key, value) + is Double -> r.put(key, value) + is Float -> r.put(key, value) + is Boolean -> r.put(key, value) + is String -> r.put(key, value) + is ByteArray -> r.put(key, value) + else -> throw ClassCastException("Expected either an Int, Short, Long, Byte, Float, Double, Boolean, String or ByteArray") + } + } + return r +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/ktx/Element.kt b/app/src/main/java/de/westnordost/streetcomplete/ktx/Element.kt new file mode 100644 index 0000000000..6aca32fe19 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/ktx/Element.kt @@ -0,0 +1,16 @@ +package de.westnordost.streetcomplete.ktx + +import de.westnordost.osmapi.map.data.* +import java.util.ArrayList + +fun Element.copy(newId: Long = id, newVersion: Int = version): Element { + val tags = tags?.let { HashMap(it) } + return when (this) { + is Node -> OsmNode(newId, newVersion, position, tags) + is Way -> OsmWay(newId, newVersion, ArrayList(nodeIds), tags) + is Relation -> OsmRelation(newId, newVersion, ArrayList(members), tags) + else -> throw RuntimeException() + } +} + +fun Way.isClosed() = nodeIds.size >= 3 && nodeIds.first() == nodeIds.last() diff --git a/app/src/main/java/de/westnordost/streetcomplete/ktx/LatLon.kt b/app/src/main/java/de/westnordost/streetcomplete/ktx/LatLon.kt new file mode 100644 index 0000000000..2b86608316 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/ktx/LatLon.kt @@ -0,0 +1,11 @@ +package de.westnordost.streetcomplete.ktx + +import de.westnordost.osmapi.map.data.LatLon +import kotlin.math.abs + +/** OSM has limited precision of 7 decimals */ +fun LatLon.equalsInOsm(other: LatLon) = + !latitude.isDifferent(other.latitude, 1e-7) && + !longitude.isDifferent(other.longitude, 1e-7) + +private fun Double.isDifferent(other: Double, delta: Double) = abs(this - other) >= delta diff --git a/app/src/main/java/de/westnordost/streetcomplete/ktx/SQLiteDatabase.kt b/app/src/main/java/de/westnordost/streetcomplete/ktx/SQLiteDatabase.kt index b1e705fc61..af0c063c8d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/ktx/SQLiteDatabase.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/ktx/SQLiteDatabase.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.ktx +import android.database.Cursor import android.database.sqlite.SQLiteDatabase /** @@ -15,3 +16,30 @@ inline fun SQLiteDatabase.transaction(body: SQLiteDatabase.() -> T): T { endTransaction() } } + + +fun SQLiteDatabase.query( + table: String, + columns: Array? = null, + selection: String? = null, + selectionArgs: Array? = null, + transform: (Cursor) -> R +): List = query(table, columns, selection, selectionArgs, null, null, null, null).use { cursor -> + val result = mutableListOf() + cursor.moveToFirst() + while(!cursor.isAfterLast) { + result.add(transform(cursor)) + cursor.moveToNext() + } + result +} + +fun SQLiteDatabase.queryOne( + table: String, + columns: Array? = null, + selection: String? = null, + selectionArgs: Array? = null, + transform: (Cursor) -> R? +): R? = query(table, columns, selection, selectionArgs, null, null, null, "1").use { cursor -> + if (cursor.moveToFirst()) transform(cursor) else null +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/ktx/View.kt b/app/src/main/java/de/westnordost/streetcomplete/ktx/View.kt new file mode 100644 index 0000000000..e9c70a6379 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/ktx/View.kt @@ -0,0 +1,22 @@ +package de.westnordost.streetcomplete.ktx + +import android.view.View +import android.view.animation.AccelerateInterpolator +import android.view.animation.DecelerateInterpolator + +fun View.popIn() { + visibility = View.VISIBLE + animate() + .alpha(1f).scaleX(1f).scaleY(1f) + .setDuration(100) + .setInterpolator(DecelerateInterpolator()) + .withEndAction(null) +} + +fun View.popOut() { + animate() + .alpha(0f).scaleX(0.5f).scaleY(0.5f) + .setDuration(100) + .setInterpolator(AccelerateInterpolator()) + .withEndAction { visibility = View.GONE } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractBottomSheetFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractBottomSheetFragment.kt index 60f456cb30..f200900fc5 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractBottomSheetFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractBottomSheetFragment.kt @@ -19,8 +19,9 @@ import de.westnordost.streetcomplete.R import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_COLLAPSED import com.google.android.material.bottomsheet.BottomSheetBehavior.STATE_EXPANDED +import de.westnordost.osmapi.map.data.LatLon -abstract class AbstractBottomSheetFragment : Fragment() { +abstract class AbstractBottomSheetFragment : Fragment(), IsCloseableBottomSheet { private lateinit var bottomSheet: LinearLayout private lateinit var bottomSheetBehavior: BottomSheetBehavior<*> private lateinit var closeButton: View @@ -36,7 +37,7 @@ abstract class AbstractBottomSheetFragment : Fragment() { } closeButton = view.findViewById(R.id.closeButton) - closeButton.setOnClickListener { activity!!.onBackPressed() } + closeButton.setOnClickListener { activity?.onBackPressed() } bottomSheetBehavior = BottomSheetBehavior.from(bottomSheet) @@ -88,9 +89,7 @@ abstract class AbstractBottomSheetFragment : Fragment() { bottomSheetBehavior.peekHeight = resources.getDimensionPixelSize(R.dimen.quest_form_peekHeight) view?.findViewById(R.id.bottomSheetContainer)?.let { it.setBackgroundResource(R.drawable.speechbubbles_gradient_background) - it.updateLayoutParams { - width = resources.getDimensionPixelSize(R.dimen.quest_form_width) - } + it.updateLayoutParams { width = resources.getDimensionPixelSize(R.dimen.quest_form_width) } } } @@ -104,9 +103,13 @@ abstract class AbstractBottomSheetFragment : Fragment() { closeButton.visibility = if (coversToolbar) View.VISIBLE else View.INVISIBLE } + @UiThread override fun onClickMapAt(position: LatLon, clickAreaSizeInMeters: Double): Boolean { + return false + } + /** Request to close the form through user interaction (back button, clicked other quest,..), * requires user confirmation if any changes have been made */ - @UiThread fun onClickClose(onConfirmed: Runnable) { + @UiThread override fun onClickClose(onConfirmed: Runnable) { if (!isRejectingClose()) { onDiscard() onConfirmed.run() diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestAnswerFragment.kt index e10883f404..cb01e762f4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestAnswerFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestAnswerFragment.kt @@ -22,6 +22,7 @@ import java.util.Locale import javax.inject.Inject import de.westnordost.osmapi.map.data.OsmElement +import de.westnordost.osmapi.map.data.Way import de.westnordost.osmfeatures.FeatureDictionary import de.westnordost.streetcomplete.Injector import de.westnordost.streetcomplete.R @@ -30,12 +31,13 @@ import de.westnordost.streetcomplete.data.QuestType import de.westnordost.streetcomplete.data.QuestTypeRegistry import de.westnordost.streetcomplete.data.meta.CountryInfo import de.westnordost.streetcomplete.data.meta.CountryInfos +import de.westnordost.streetcomplete.data.meta.OsmAreas import de.westnordost.streetcomplete.data.osm.ElementGeometry import kotlinx.android.synthetic.main.fragment_quest_answer.* import java.util.concurrent.FutureTask /** Abstract base class for any dialog with which the user answers a specific quest(ion) */ -abstract class AbstractQuestAnswerFragment : AbstractBottomSheetFragment() { +abstract class AbstractQuestAnswerFragment : AbstractBottomSheetFragment(), IsShowingQuestDetails { private val countryInfos: CountryInfos private val questTypeRegistry: QuestTypeRegistry @@ -78,8 +80,8 @@ abstract class AbstractQuestAnswerFragment : AbstractBottomSheetFragment() { private var startedOnce = false - val questId:Long get() = questAnswerComponent.questId - val questGroup:QuestGroup get() = questAnswerComponent.questGroup + override val questId: Long get() = questAnswerComponent.questId + override val questGroup: QuestGroup get() = questAnswerComponent.questGroup open val contentLayoutResId: Int? = null open val buttonsResId: Int? = null @@ -137,10 +139,7 @@ abstract class AbstractQuestAnswerFragment : AbstractBottomSheetFragment() { content.visibility = View.GONE } - - val cantSay = OtherAnswer(R.string.quest_generic_answer_notApplicable) { onClickCantSay() } - val answers = listOf(cantSay) + otherAnswers - + val answers = assembleOtherAnswers() if (answers.size == 1) { otherAnswersButton.setText(answers.first().titleResourceId) otherAnswersButton.setOnClickListener { answers.first().action() } @@ -163,6 +162,31 @@ abstract class AbstractQuestAnswerFragment : AbstractBottomSheetFragment() { } } + private fun assembleOtherAnswers() : List { + val answers = mutableListOf() + + val cantSay = OtherAnswer(R.string.quest_generic_answer_notApplicable) { onClickCantSay() } + answers.add(cantSay) + + val way = osmElement as? Way + if (way != null) { + /* splitting up a closed roundabout can be very complex if it is part of a route + relation, so it is not supported + https://wiki.openstreetmap.org/wiki/Relation:route#Bus_routes_and_roundabouts + */ + val isClosedRoundabout = way.nodeIds.firstOrNull() == way.nodeIds.lastOrNull() && + way.tags?.get("junction") == "roundabout" + if (!isClosedRoundabout && !OsmAreas.isArea(way)) { + val splitWay = OtherAnswer(R.string.quest_generic_answer_differs_along_the_way) { + onClickSplitWayAnswer() + } + answers.add(splitWay) + } + } + answers.addAll(otherAnswers) + return answers + } + private fun getLevelLabelText(): String? { val tags = osmElement?.tags ?: return null /* prefer addr:floor etc. over level as level is rather an index than how the floor is @@ -220,7 +244,7 @@ abstract class AbstractQuestAnswerFragment : AbstractBottomSheetFragment() { } protected fun onClickCantSay() { - AlertDialog.Builder(context!!) + context?.let { AlertDialog.Builder(it) .setTitle(R.string.quest_leave_new_note_title) .setMessage(R.string.quest_leave_new_note_description) .setNegativeButton(R.string.quest_leave_new_note_no) { _, _ -> skipQuest() } @@ -229,10 +253,22 @@ abstract class AbstractQuestAnswerFragment : AbstractBottomSheetFragment() { questAnswerComponent.onComposeNote(questTitle) } .show() + } + } + + private fun onClickSplitWayAnswer() { + context?.let { AlertDialog.Builder(it) + .setMessage(R.string.quest_split_way_description) + .setNegativeButton(android.R.string.cancel, null) + .setPositiveButton(android.R.string.ok) { _, _ -> + questAnswerComponent.onSplitWay() + } + .show() + } } protected fun applyAnswer(data: T) { - questAnswerComponent.onAnswerQuest(data as Any) + questAnswerComponent.onAnsweredQuest(data as Any) } protected fun skipQuest() { diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestFormAnswerFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestFormAnswerFragment.kt index 6ede581908..c5a65cc62e 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestFormAnswerFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractQuestFormAnswerFragment.kt @@ -2,10 +2,10 @@ package de.westnordost.streetcomplete.quests import android.os.Bundle import android.view.View -import android.view.animation.AccelerateInterpolator -import android.view.animation.DecelerateInterpolator import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.ktx.popIn +import de.westnordost.streetcomplete.ktx.popOut import de.westnordost.streetcomplete.ktx.toast import kotlinx.android.synthetic.main.fragment_quest_answer.* @@ -26,18 +26,9 @@ abstract class AbstractQuestFormAnswerFragment : AbstractQuestAnswerFragment< protected fun checkIsFormComplete() { if (isFormComplete()) { - okButton.visibility = View.VISIBLE - okButton.animate() - .alpha(1f).scaleX(1f).scaleY(1f) - .setDuration(100) - .setInterpolator(DecelerateInterpolator()) - .withEndAction(null) + okButton.popIn() } else { - okButton.animate() - .alpha(0f).scaleX(0.5f).scaleY(0.5f) - .setDuration(100) - .setInterpolator(AccelerateInterpolator()) - .withEndAction { okButton?.visibility = View.GONE } + okButton.popOut() } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/IsCloseableBottomSheet.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/IsCloseableBottomSheet.kt new file mode 100644 index 0000000000..79c8ba0fcb --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/IsCloseableBottomSheet.kt @@ -0,0 +1,9 @@ +package de.westnordost.streetcomplete.quests + +import de.westnordost.osmapi.map.data.LatLon + +interface IsCloseableBottomSheet { + /** Returns true if the bottom sheet shall consume the event */ + fun onClickMapAt(position: LatLon, clickAreaSizeInMeters: Double): Boolean + fun onClickClose(onConfirmed: Runnable) +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/IsShowingQuestDetails.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/IsShowingQuestDetails.kt new file mode 100644 index 0000000000..639eff3c50 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/IsShowingQuestDetails.kt @@ -0,0 +1,8 @@ +package de.westnordost.streetcomplete.quests + +import de.westnordost.streetcomplete.data.QuestGroup + +interface IsShowingQuestDetails { + val questId: Long + val questGroup: QuestGroup +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt index 681627fda5..b0776dd3d3 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt @@ -1,25 +1,32 @@ package de.westnordost.streetcomplete.quests -import android.content.Context import android.os.Bundle import android.view.View +import androidx.core.os.bundleOf import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.data.QuestGroup import de.westnordost.streetcomplete.data.osmnotes.AbstractCreateNoteFragment import kotlinx.android.synthetic.main.form_leave_note.* import kotlinx.android.synthetic.main.fragment_quest_answer.* -class LeaveNoteInsteadFragment : AbstractCreateNoteFragment() { +class LeaveNoteInsteadFragment : AbstractCreateNoteFragment(), IsShowingQuestDetails { + + interface Listener { + fun onCreatedNoteInstead(questId: Long, group: QuestGroup, questTitle: String, note: String, imagePaths: List?) + } override val layoutResId = R.layout.fragment_quest_answer - private val questAnswerComponent: QuestAnswerComponent = QuestAnswerComponent() private lateinit var questTitle: String + override var questId: Long = 0L + override lateinit var questGroup: QuestGroup override fun onCreate(inState: Bundle?) { super.onCreate(inState) - questAnswerComponent.onCreate(arguments) questTitle = arguments!!.getString(ARG_QUEST_TITLE)!! + questId = arguments!!.getLong(ARG_QUEST_ID) + questGroup = QuestGroup.valueOf(arguments!!.getString(ARG_QUEST_GROUP)!!) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -29,16 +36,24 @@ class LeaveNoteInsteadFragment : AbstractCreateNoteFragment() { descriptionLabel.text = null } - override fun onAttach(context: Context) { - super.onAttach(context) - questAnswerComponent.onAttach(context as OsmQuestAnswerListener) - } - - override fun onLeaveNote(text: String, imagePaths: List?) { - questAnswerComponent.onLeaveNote(questTitle, text, imagePaths) + override fun onComposedNote(text: String, imagePaths: List?) { + (activity as Listener).onCreatedNoteInstead(questId, questGroup, questTitle, text, imagePaths) } companion object { - const val ARG_QUEST_TITLE = "questTitle" + private const val ARG_QUEST_TITLE = "questTitle" + private const val ARG_QUEST_ID = "questId" + private const val ARG_QUEST_GROUP = "questGroup" + + @JvmStatic + fun create(questId: Long, group: QuestGroup, questTitle: String): LeaveNoteInsteadFragment { + val f = LeaveNoteInsteadFragment() + f.arguments = bundleOf( + ARG_QUEST_GROUP to group.name, + ARG_QUEST_ID to questId, + ARG_QUEST_TITLE to questTitle + ) + return f + } } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/OsmQuestAnswerListener.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/OsmQuestAnswerListener.kt index 65b2237c01..ec344951f5 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/OsmQuestAnswerListener.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/OsmQuestAnswerListener.kt @@ -10,8 +10,8 @@ interface OsmQuestAnswerListener { /** Called when the user chose to leave a note instead */ fun onComposeNote(questId: Long, group: QuestGroup, questTitle: String) - /** Called when the user did not answer the quest with the given id but instead left a note */ - fun onLeaveNote(questId: Long, group: QuestGroup, questTitle: String, note: String, imagePaths: List?) + /** Called when the user chose to split the way */ + fun onSplitWay(osmQuestId: Long) /** Called when the user chose to skip the quest */ fun onSkippedQuest(questId: Long, group: QuestGroup) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/QuestAnswerComponent.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/QuestAnswerComponent.kt index ad3d6cce56..407dd309af 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/QuestAnswerComponent.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/QuestAnswerComponent.kt @@ -32,7 +32,7 @@ class QuestAnswerComponent { callbackListener = listener } - fun onAnswerQuest(answer: Any) { + fun onAnsweredQuest(answer: Any) { callbackListener.onAnsweredQuest(questId, questGroup, answer) } @@ -40,8 +40,9 @@ class QuestAnswerComponent { callbackListener.onComposeNote(questId, questGroup, questTitle) } - fun onLeaveNote(questTitle: String, text: String, imagePaths: List?) { - callbackListener.onLeaveNote(questId, questGroup, questTitle, text, imagePaths) + fun onSplitWay() { + if (questGroup != QuestGroup.OSM) throw IllegalStateException() + callbackListener.onSplitWay(questId) } fun onSkippedQuest() { diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/SplitWayFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/SplitWayFragment.kt new file mode 100644 index 0000000000..6ecb9f7fdd --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/SplitWayFragment.kt @@ -0,0 +1,257 @@ +package de.westnordost.streetcomplete.quests + +import android.animation.AnimatorInflater +import android.content.res.Configuration +import android.graphics.PointF +import android.graphics.drawable.Animatable +import android.os.Bundle +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import android.view.animation.AnimationUtils +import android.widget.RelativeLayout +import androidx.annotation.UiThread +import androidx.appcompat.app.AlertDialog +import androidx.core.os.bundleOf +import androidx.core.view.updateLayoutParams +import androidx.fragment.app.Fragment +import de.westnordost.osmapi.map.data.LatLon +import de.westnordost.osmapi.map.data.OsmLatLon +import de.westnordost.osmapi.map.data.Way +import de.westnordost.streetcomplete.Injector + +import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.data.QuestGroup +import de.westnordost.streetcomplete.data.osm.ElementGeometry +import de.westnordost.streetcomplete.data.osm.changes.SplitAtLinePosition +import de.westnordost.streetcomplete.data.osm.changes.SplitAtPoint +import de.westnordost.streetcomplete.data.osm.changes.SplitPolylineAtPosition +import de.westnordost.streetcomplete.ktx.* +import de.westnordost.streetcomplete.sound.SoundFx +import de.westnordost.streetcomplete.util.SphericalEarthMath.* +import kotlinx.android.synthetic.main.fragment_split_way.* +import javax.inject.Inject + +class SplitWayFragment : Fragment(), IsCloseableBottomSheet, IsShowingQuestDetails { + + interface Listener { + fun onAddSplit(point: LatLon) + fun onRemoveSplit(point: LatLon) + fun onSplittedWay(osmQuestId: Long, splits: List) + } + private val splits: MutableList> = mutableListOf() + + @Inject internal lateinit var soundFx: SoundFx + + override val questId: Long get() = osmQuestId + override val questGroup: QuestGroup get() = QuestGroup.OSM + + private var osmQuestId: Long = 0L + private lateinit var way: Way + private lateinit var positions: List + private var clickPos: PointF? = null + + private val hasChanges get() = splits.isNotEmpty() + private val isFormComplete get() = splits.size >= if (way.isClosed()) 2 else 1 + + init { + Injector.instance.applicationComponent.inject(this) + } + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + osmQuestId = arguments!!.getLong(ARG_QUEST_ID) + way = arguments!!.getSerializable(ARG_WAY) as Way + val elementGeometry = arguments!!.getSerializable(ARG_ELEMENT_GEOMETRY) as ElementGeometry + positions = elementGeometry.polylines.single().map { OsmLatLon(it.latitude, it.longitude) } + soundFx.prepare(R.raw.snip) + soundFx.prepare(R.raw.plop2) + } + + override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { + return inflater.inflate(R.layout.fragment_split_way, container, false) + } + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + + splitWayRoot.setOnTouchListener { _, event -> + clickPos = PointF(event.x, event.y) + false + } + + okButton.setOnClickListener { onClickOk() } + cancelButton.setOnClickListener { activity?.onBackPressed() } + undoButton.setOnClickListener { onClickUndo() } + + undoButton.visibility = if (hasChanges) View.VISIBLE else View.INVISIBLE + okButton.visibility = if (isFormComplete) View.VISIBLE else View.INVISIBLE + + if (savedInstanceState == null) { + view.findViewById(R.id.speechbubbleContentContainer).startAnimation( + AnimationUtils.loadAnimation(context, R.anim.inflate_answer_bubble) + ) + } + } + + override fun onConfigurationChanged(newConfig: Configuration?) { + super.onConfigurationChanged(newConfig) + // see rant comment in AbstractBottomSheetFragment + resources.updateConfiguration(newConfig, resources.displayMetrics) + + bottomSheetContainer.setBackgroundResource(R.drawable.speechbubbles_gradient_background) + bottomSheetContainer.updateLayoutParams { width = resources.getDimensionPixelSize(R.dimen.quest_form_width) } + } + + private fun onClickOk() { + if (splits.size > 2) { + confirmManySplits { onSplittedWayConfirmed() } + } else { + onSplittedWayConfirmed() + } + } + + private fun onSplittedWayConfirmed() { + (activity as Listener).onSplittedWay(osmQuestId, splits.map { it.first }) + } + + private fun confirmManySplits(callback: () -> (Unit)) { + activity?.let { + AlertDialog.Builder(it) + .setTitle(R.string.quest_generic_confirmation_title) + .setMessage(R.string.quest_split_way_many_splits_confirmation_description) + .setPositiveButton(R.string.quest_generic_confirmation_yes) { _, _ -> callback() } + .setNegativeButton(R.string.quest_generic_confirmation_no, null) + .show() + } + } + + private fun onClickUndo() { + if (splits.isNotEmpty()) { + val item = splits.removeAt(splits.lastIndex) + animateButtonVisibilities() + soundFx.play(R.raw.plop2) + (activity as? Listener)?.onRemoveSplit(item.second) + } + } + + @UiThread + override fun onClickMapAt(position: LatLon, clickAreaSizeInMeters: Double): Boolean { + + val splitWayCandidates = createSplits(position, clickAreaSizeInMeters) + if (splitWayCandidates.isEmpty()) return true + + // show toast only if it is possible to zoom in further + if (splitWayCandidates.size > 1 && clickAreaSizeInMeters > CLICK_AREA_SIZE_AT_MAX_ZOOM) { + context?.toast(R.string.quest_split_way_too_imprecise) + } + val splitWay = splitWayCandidates.minBy { distance(it.pos, position) }!! + val splitPosition = splitWay.pos + + // new split point is too close to existing split points + if (splits.any { distance(it.second, splitPosition) < clickAreaSizeInMeters } ) { + context?.toast(R.string.quest_split_way_too_imprecise) + } else { + splits.add(Pair(splitWay, splitPosition)) + animateButtonVisibilities() + animateScissors() + (activity as? Listener)?.onAddSplit(splitPosition) + } + + // always consume event. User should press the cancel button to exit + return true + } + + + private fun animateScissors() { + val scissorsPos = clickPos ?: return + + (scissors.drawable as? Animatable)?.start() + + scissors.updateLayoutParams { + leftMargin = (scissorsPos.x - scissors.width/2).toInt() + topMargin = (scissorsPos.y - scissors.height/2).toInt() + } + scissors.alpha = 1f + val animator = AnimatorInflater.loadAnimator(context, R.animator.scissors_snip) + animator.setTarget(scissors) + animator.start() + + soundFx.play(R.raw.snip) + } + + private fun createSplits(clickPosition: LatLon, clickAreaSizeInMeters: Double): Set { + val splitWaysAtNodes = createSplitsAtNodes(clickPosition, clickAreaSizeInMeters) + // if a split on a node is possible, do that and don't even check if a split on a way is also possible + if (splitWaysAtNodes.isNotEmpty()) return splitWaysAtNodes + return createSplitsForLines(clickPosition, clickAreaSizeInMeters) + } + + private fun createSplitsAtNodes(clickPosition: LatLon, clickAreaSizeInMeters: Double): Set { + // ignore first and last node (cannot be split at the very start or end) + val result = mutableSetOf() + for (pos in positions.subList(1, positions.size - 1)) { + val nodeDistance = distance(clickPosition, pos) + if (clickAreaSizeInMeters > nodeDistance) { + result.add(SplitAtPoint(pos)) + } + } + return result + } + + private fun createSplitsForLines(clickPosition: LatLon, clickAreaSizeInMeters: Double): Set { + val result = mutableSetOf() + positions.forEachPair { first, second -> + val crossTrackDistance = crossTrackDistance(first, second, clickPosition) + if (clickAreaSizeInMeters > crossTrackDistance) { + val alongTrackDistance = alongTrackDistance(first, second, clickPosition) + val distance = distance(first, second) + if (distance > alongTrackDistance && alongTrackDistance > 0) { + val delta = alongTrackDistance / distance + result.add(SplitAtLinePosition(first, second, delta)) + } + } + } + return result + } + + @UiThread override fun onClickClose(onConfirmed: Runnable) { + if (!hasChanges) { + onConfirmed.run() + } else { + activity?.let { + AlertDialog.Builder(it) + .setMessage(R.string.confirmation_discard_title) + .setPositiveButton(R.string.confirmation_discard_positive) { _, _ -> + onConfirmed.run() + } + .setNegativeButton(R.string.confirmation_discard_negative, null) + .show() + } + } + } + + private fun animateButtonVisibilities() { + if (isFormComplete) okButton.popIn() else okButton.popOut() + if (hasChanges) undoButton.popIn() else undoButton.popOut() + } + + companion object { + private const val CLICK_AREA_SIZE_AT_MAX_ZOOM = 2.6 + + private const val ARG_QUEST_ID = "questId" + private const val ARG_WAY = "way" + private const val ARG_ELEMENT_GEOMETRY = "elementGeometry" + + @JvmStatic + fun create(osmQuestId: Long, way: Way, elementGeometry: ElementGeometry): SplitWayFragment { + val f = SplitWayFragment() + f.arguments = bundleOf( + ARG_QUEST_ID to osmQuestId, + ARG_WAY to way, + ARG_ELEMENT_GEOMETRY to elementGeometry + ) + return f + } + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCycleway.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCycleway.kt index a0f723736e..757fb964d0 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCycleway.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCycleway.kt @@ -47,6 +47,8 @@ class AddCycleway(private val overpassServer: OverpassMapDataDao) : OsmElementQu "US-AZ","US-TX" ) + override val isSplitWayEnabled = true + override fun getTitle(tags: Map) = R.string.quest_cycleway_title2 override fun isApplicableTo(element: Element):Boolean? = null diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCyclewayForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCyclewayForm.kt index 072604bced..587241244b 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCyclewayForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/bikeway/AddCyclewayForm.kt @@ -28,13 +28,13 @@ class AddCyclewayForm : AbstractQuestFormAnswerFragment() { override val contentLayoutResId = R.layout.quest_street_side_puzzle override val contentPadding = false - override val otherAnswers:List get() { + override val otherAnswers: List get() { val isNoRoundabout = osmElement!!.tags["junction"] != "roundabout" - return if (!isDefiningBothSides && isNoRoundabout) { - listOf(OtherAnswer(R.string.quest_cycleway_answer_contraflow_cycleway) { showBothSides() }) - } else { - listOf() + val result = mutableListOf() + if (!isDefiningBothSides && isNoRoundabout) { + result.add(OtherAnswer(R.string.quest_cycleway_answer_contraflow_cycleway) { showBothSides() }) } + return result } private val likelyNoBicycleContraflow = FiltersParser().parse(""" diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddProhibitedForPedestrians.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddProhibitedForPedestrians.kt index fb804d2d95..c49d3614d0 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddProhibitedForPedestrians.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddProhibitedForPedestrians.kt @@ -31,10 +31,11 @@ class AddProhibitedForPedestrians(o: OverpassMapDataDao) : SimpleOverpassQuestTy override val commitMessage = "Add whether roads are accessible for pedestrians" override val icon = R.drawable.ic_quest_no_pedestrians + override val isSplitWayEnabled = true override fun getTitle(tags: Map) = R.string.quest_accessible_for_pedestrians_title_prohibited - override fun createForm() = AddAccessibleForPedestriansForm() + override fun createForm() = AddProhibitedForPedestriansForm() override fun applyAnswerTo(answer: ProhibitedForPedestriansAnswer, changes: StringMapChangesBuilder) { when(answer) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddAccessibleForPedestriansForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddProhibitedForPedestriansForm.kt similarity index 95% rename from app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddAccessibleForPedestriansForm.kt rename to app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddProhibitedForPedestriansForm.kt index 9069a7ae01..5524933485 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddAccessibleForPedestriansForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/foot/AddProhibitedForPedestriansForm.kt @@ -5,13 +5,12 @@ import android.view.View import android.widget.ImageView import androidx.appcompat.app.AlertDialog import de.westnordost.streetcomplete.R -import de.westnordost.streetcomplete.quests.AYesNoQuestAnswerFragment import de.westnordost.streetcomplete.quests.AbstractQuestAnswerFragment import de.westnordost.streetcomplete.quests.OtherAnswer import de.westnordost.streetcomplete.quests.foot.ProhibitedForPedestriansAnswer.* import kotlinx.android.synthetic.main.quest_buttonpanel_yes_no_sidewalk.* -class AddAccessibleForPedestriansForm : AbstractQuestAnswerFragment() { +class AddProhibitedForPedestriansForm : AbstractQuestAnswerFragment() { override val buttonsResId = R.layout.quest_buttonpanel_yes_no_sidewalk diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/localized_name/AddRoadName.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/localized_name/AddRoadName.kt index 1905c694f5..e5c2df96ce 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/localized_name/AddRoadName.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/localized_name/AddRoadName.kt @@ -24,6 +24,7 @@ class AddRoadName( override val commitMessage = "Determine road names and types" override val icon = R.drawable.ic_quest_street_name override val hasMarkersAtEnds = true + override val isSplitWayEnabled = true override fun getTitle(tags: Map) = if (tags["highway"] == "pedestrian") diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeed.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeed.kt index 6466e90d59..5660175432 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeed.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeed.kt @@ -22,6 +22,7 @@ class AddMaxSpeed(o: OverpassMapDataDao) : SimpleOverpassQuestType) = R.string.quest_segregated_title override fun createForm() = AddCyclewaySegregationForm() diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/sidewalk/AddSidewalk.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/sidewalk/AddSidewalk.kt index 89b7d4a0e6..d56067c773 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/sidewalk/AddSidewalk.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/sidewalk/AddSidewalk.kt @@ -15,6 +15,7 @@ class AddSidewalk(private val overpassServer: OverpassMapDataDao) : OsmElementQu override val commitMessage = "Add whether there are sidewalks" override val icon = R.drawable.ic_quest_sidewalk + override val isSplitWayEnabled = true override fun getTitle(tags: Map) = R.string.quest_sidewalk_title diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt index 3e858bcfe0..7d88c63dab 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt @@ -13,6 +13,7 @@ class AddPathSurface(o: OverpassMapDataDao) : SimpleOverpassQuestType(o) """ override val commitMessage = "Add path surfaces" override val icon = R.drawable.ic_quest_way_surface + override val isSplitWayEnabled = true override fun getTitle(tags: Map) = when { tags["area"] == "yes" -> R.string.quest_streetSurface_square_title diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddRoadSurface.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddRoadSurface.kt index cfa1cca7ee..a23c961a2d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddRoadSurface.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddRoadSurface.kt @@ -14,6 +14,7 @@ class AddRoadSurface(o: OverpassMapDataDao) : SimpleOverpassQuestType(o) """ override val commitMessage = "Add road surfaces" override val icon = R.drawable.ic_quest_street_surface + override val isSplitWayEnabled = true override fun getTitle(tags: Map): Int { val hasName = tags.containsKey("name") diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt index 1f3b206785..a19d0398ea 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt @@ -13,6 +13,7 @@ class AddTracktype(o: OverpassMapDataDao) : SimpleOverpassQuestType(o) { """ override val commitMessage = "Add tracktype" override val icon = R.drawable.ic_quest_tractor + override val isSplitWayEnabled = true override fun getTitle(tags: Map) = R.string.quest_tracktype_title diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/way_lit/AddWayLit.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/way_lit/AddWayLit.kt index 9673e19a57..a6b1fc17d4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/way_lit/AddWayLit.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/way_lit/AddWayLit.kt @@ -30,6 +30,7 @@ class AddWayLit(o: OverpassMapDataDao) : SimpleOverpassQuestType(o) { override val commitMessage = "Add whether way is lit" override val icon = R.drawable.ic_quest_lantern + override val isSplitWayEnabled = true override fun getTitle(tags: Map): Int { val type = tags["highway"] diff --git a/app/src/main/java/de/westnordost/streetcomplete/sound/SoundFx.java b/app/src/main/java/de/westnordost/streetcomplete/sound/SoundFx.java index 4715355c47..434a4a0b95 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/sound/SoundFx.java +++ b/app/src/main/java/de/westnordost/streetcomplete/sound/SoundFx.java @@ -29,12 +29,9 @@ public void prepare(@RawRes int resId) public void play(@RawRes int resId) { + if (soundIds.get(resId) == 0) prepare(resId); boolean isTouchSoundsEnabled = Settings.System.getInt(context.getContentResolver(), Settings.System.SOUND_EFFECTS_ENABLED, 1) != 0; - if(isTouchSoundsEnabled) - { - if (soundIds.get(resId) == 0) prepare(resId); - sounds.play(soundIds.get(resId), 1, 1, 1, 0, 1); - } + if(isTouchSoundsEnabled) sounds.play(soundIds.get(resId), 1, 1, 1, 0, 1); } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/statistics/AnswersCounter.java b/app/src/main/java/de/westnordost/streetcomplete/statistics/AnswersCounter.java index 97a8fcf2db..215a3339b4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/statistics/AnswersCounter.java +++ b/app/src/main/java/de/westnordost/streetcomplete/statistics/AnswersCounter.java @@ -11,6 +11,7 @@ import de.westnordost.streetcomplete.data.QuestStatus; import de.westnordost.streetcomplete.data.osm.persist.OsmQuestDao; +import de.westnordost.streetcomplete.data.osm.persist.OsmQuestSplitWayDao; import de.westnordost.streetcomplete.data.osmnotes.CreateNoteDao; import de.westnordost.streetcomplete.data.osmnotes.OsmNoteQuestDao; import de.westnordost.streetcomplete.data.statistics.QuestStatisticsDao; @@ -20,6 +21,7 @@ public class AnswersCounter private final OsmQuestDao questDB; private final OsmNoteQuestDao noteQuestDB; private final CreateNoteDao createNoteDB; + private final OsmQuestSplitWayDao splitWayDB; private final QuestStatisticsDao questStatisticsDB; private int uploaded; @@ -33,11 +35,13 @@ public class AnswersCounter private boolean isAutosync; @Inject public AnswersCounter(OsmQuestDao questDB, OsmNoteQuestDao noteQuestDB, - CreateNoteDao createNoteDB, QuestStatisticsDao questStatisticsDB) + CreateNoteDao createNoteDB, OsmQuestSplitWayDao splitWayDB, + QuestStatisticsDao questStatisticsDB) { this.questDB = questDB; this.noteQuestDB = noteQuestDB; this.createNoteDB = createNoteDB; + this.splitWayDB = splitWayDB; this.questStatisticsDB = questStatisticsDB; } @@ -104,6 +108,7 @@ public Integer uploaded(){ unsynced = 0; unsynced += questDB.getCount(null, QuestStatus.ANSWERED); unsynced += noteQuestDB.getCount(null, QuestStatus.ANSWERED); + unsynced += splitWayDB.getCount(); unsynced += createNoteDB.getCount(); return null; } diff --git a/app/src/main/java/de/westnordost/streetcomplete/tangram/QuestsMapFragment.java b/app/src/main/java/de/westnordost/streetcomplete/tangram/QuestsMapFragment.java index ecdf837e82..fbcef6bc71 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/tangram/QuestsMapFragment.java +++ b/app/src/main/java/de/westnordost/streetcomplete/tangram/QuestsMapFragment.java @@ -8,13 +8,13 @@ import android.os.Handler; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import androidx.annotation.UiThread; import com.mapzen.tangram.LabelPickResult; import com.mapzen.tangram.LngLat; import com.mapzen.tangram.MapController; import com.mapzen.tangram.MapData; +import com.mapzen.tangram.Marker; import com.mapzen.tangram.SceneError; import com.mapzen.tangram.SceneUpdate; import com.mapzen.tangram.TouchInput; @@ -28,17 +28,20 @@ import java.util.Set; import javax.inject.Inject; -import javax.inject.Provider; import de.westnordost.streetcomplete.Injector; +import de.westnordost.streetcomplete.R; import de.westnordost.streetcomplete.data.Quest; import de.westnordost.streetcomplete.data.QuestGroup; import de.westnordost.streetcomplete.data.QuestType; import de.westnordost.streetcomplete.data.osm.ElementGeometry; +import de.westnordost.streetcomplete.data.visiblequests.OrderedVisibleQuestTypesProvider; import de.westnordost.streetcomplete.quests.bikeway.AddCycleway; +import de.westnordost.streetcomplete.util.DpUtil; import de.westnordost.streetcomplete.util.SlippyMapMath; import de.westnordost.osmapi.map.data.BoundingBox; import de.westnordost.osmapi.map.data.LatLon; +import de.westnordost.streetcomplete.util.SphericalEarthMath; public class QuestsMapFragment extends MapFragment implements TouchInput.TapResponder, MapController.LabelPickListener @@ -58,26 +61,34 @@ public class QuestsMapFragment extends MapFragment implements TouchInput.TapResp private LngLat lastPos; private Float lastRotation, lastTilt; + private LatLon lastClickPos; + private double lastFingerRadiusInMeters; + private Rect lastDisplayedRect; private final Set retrievedTiles; private static final int TILES_ZOOM = 14; private static final float MAX_QUEST_ZOOM = 19; + private static final int CLICK_AREA_SIZE_IN_DP = 48; + private String sceneFile; private Listener listener; private Rect questOffset; - @Inject Provider> questTypesProvider; + // LatLon -> Marker Id + private final Map markerIds = new HashMap<>(); + + @Inject OrderedVisibleQuestTypesProvider questTypesProvider; @Inject TangramQuestSpriteSheetCreator spriteSheetCreator; private Map questTypeOrder; public interface Listener { void onClickedQuest(QuestGroup questGroup, Long questId); - void onClickedMapAt(@Nullable LatLon position); + void onClickedMapAt(@NonNull LatLon position, double clickAreaSizeInMeters); /** Called once the given bbox comes into view first (listener should get quests there) */ void onFirstInView(BoundingBox bbox); } @@ -166,7 +177,12 @@ public QuestsMapFragment() @Override public boolean onSingleTapConfirmed(float x, float y) { - if(controller != null) controller.pickLabel(x,y); + + if(controller != null) { + onClickedMap(x, y); + + controller.pickLabel(x,y); + } return true; } @@ -180,7 +196,10 @@ public void onLabelPick(LabelPickResult labelPickResult, float positionX, float || labelPickResult.getProperties() == null || labelPickResult.getProperties().get(MARKER_QUEST_ID) == null) { - onClickedMap(positionX, positionY); + if (lastClickPos != null) + { + listener.onClickedMapAt(lastClickPos, lastFingerRadiusInMeters); + } return; } @@ -264,7 +283,18 @@ private float getMaxZoomThatContains(ElementGeometry geometry) private void onClickedMap(float positionX, float positionY) { LngLat pos = controller.screenPositionToLngLat(new PointF(positionX, positionY)); - if(pos != null) listener.onClickedMapAt(TangramConst.toLatLon(pos)); + if(pos != null) { + float fingerSize = DpUtil.toPx(CLICK_AREA_SIZE_IN_DP, getContext())/2; + LngLat fingerEdge = controller.screenPositionToLngLat(new PointF(positionX + fingerSize, positionY)); + if (fingerEdge != null) + { + LatLon clickPos = TangramConst.toLatLon(pos); + LatLon fingerEdgePos = TangramConst.toLatLon(fingerEdge); + double fingerRadiusInMeters = SphericalEarthMath.distance(clickPos, fingerEdgePos); + lastClickPos = clickPos; + lastFingerRadiusInMeters = fingerRadiusInMeters; + } + } } @Override protected boolean shouldCenterCurrentPosition() @@ -361,12 +391,16 @@ else if(g.center != null) } } - @UiThread - public void removeQuestGeometry() + @UiThread public void removeQuestGeometry() { if(geometryLayer != null) geometryLayer.clear(); if(controller != null) { + for (Long markerId : markerIds.values()) + { + controller.removeMarker(markerId); + } + markerIds.clear(); if(zoomBeforeShowingQuest != null) controller.setZoomEased(zoomBeforeShowingQuest, 500); if(positionBeforeShowingQuest != null) controller.setPositionEased(positionBeforeShowingQuest, 500); zoomBeforeShowingQuest = null; @@ -374,23 +408,27 @@ public void removeQuestGeometry() followPosition(); } } -/* - public void addQuest(Quest quest, QuestGroup group) - { - // TODO: this method may also be called for quests that are already displayed on this map - if(questsLayer == null) return; - LngLat pos = TangramConst.toLngLat(quest.getMarkerLocation()); - Map props = new HashMap<>(); - props.put("type", "point"); - props.put("kind", quest.getType().getIconName()); - props.put(MARKER_QUEST_GROUP, group.name()); - props.put(MARKER_QUEST_ID, String.valueOf(quest.getId())); - questsLayer.addPoint(pos, props); + @UiThread public void putMarkerForCurrentQuest(LatLon pos) { + deleteMarkerForCurrentQuest(pos); + if (controller != null) { + Marker marker = controller.addMarker(); + marker.setDrawable(R.drawable.crosshair_marker); + marker.setStylingFromString("{ style: 'points', color: 'white', size: 48px, order: 2000, collide: false }"); + marker.setPoint(TangramConst.toLngLat(pos)); + markerIds.put(pos, marker.getMarkerId()); + } + } - controller.applySceneUpdates(); + @UiThread public void deleteMarkerForCurrentQuest(LatLon pos) { + if (controller != null) { + Long markerId = markerIds.get(pos); + if (markerId != null) { + controller.removeMarker(markerId); + markerIds.remove(pos); + } + } } -*/ private int getQuestPriority(Quest quest){ // priority is decided by diff --git a/app/src/main/java/de/westnordost/streetcomplete/util/KryoSerializer.java b/app/src/main/java/de/westnordost/streetcomplete/util/KryoSerializer.java index b3f8422c76..3de06c4dcd 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/util/KryoSerializer.java +++ b/app/src/main/java/de/westnordost/streetcomplete/util/KryoSerializer.java @@ -13,6 +13,9 @@ import javax.inject.Singleton; import de.westnordost.osmapi.map.data.Fixed1E7LatLon; +import de.westnordost.streetcomplete.data.osm.changes.SplitAtLinePosition; +import de.westnordost.streetcomplete.data.osm.changes.SplitAtPoint; +import de.westnordost.streetcomplete.data.osm.changes.SplitPolylineAtPosition; import de.westnordost.streetcomplete.data.osm.changes.StringMapChanges; import de.westnordost.streetcomplete.data.osm.changes.StringMapEntryAdd; import de.westnordost.streetcomplete.data.osm.changes.StringMapEntryDelete; @@ -59,6 +62,8 @@ public class KryoSerializer implements Serializer LocalizedName.class, WeekdaysTimesRow.class, OsmLatLon.class, + SplitAtPoint.class, + SplitAtLinePosition.class }; diff --git a/app/src/main/java/de/westnordost/streetcomplete/util/SphericalEarthMath.java b/app/src/main/java/de/westnordost/streetcomplete/util/SphericalEarthMath.java index 27512062aa..4243aa8f4c 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/util/SphericalEarthMath.java +++ b/app/src/main/java/de/westnordost/streetcomplete/util/SphericalEarthMath.java @@ -71,9 +71,9 @@ public static BoundingBox enclosingBoundingBox(Iterable positions) /** @return a new position in the given distance and angle from the original position */ public static LatLon translate(LatLon pos, double distance, double angle) { - double φ1 = Math.toRadians(pos.getLatitude()); - double λ1 = Math.toRadians(pos.getLongitude()); - double α1 = Math.toRadians(angle); + double φ1 = toRadians(pos.getLatitude()); + double λ1 = toRadians(pos.getLongitude()); + double α1 = toRadians(angle); double σ12 = distance / EARTH_RADIUS; double y = sin(φ1) * cos(σ12) + cos(φ1) * sin(σ12) * cos(α1); @@ -104,12 +104,43 @@ public static double enclosedArea(BoundingBox bbox) */ public static double distance(LatLon pos1, LatLon pos2) { - return EARTH_RADIUS * distance( - Math.toRadians(pos1.getLatitude()), - Math.toRadians(pos1.getLongitude()), - Math.toRadians(pos2.getLatitude()), - Math.toRadians(pos2.getLongitude() - )); + return distance( + toRadians(pos1.getLatitude()), + toRadians(pos1.getLongitude()), + toRadians(pos2.getLatitude()), + toRadians(pos2.getLongitude()) + ); + } + + /** + * @return the shortest distance of the arc between two points and another point in meters + */ + public static double crossTrackDistance(LatLon start, LatLon end, LatLon point) + { + return crossTrackDistance( + toRadians(start.getLatitude()), + toRadians(start.getLongitude()), + toRadians(end.getLatitude()), + toRadians(end.getLongitude()), + toRadians(point.getLatitude()), + toRadians(point.getLongitude()) + ); + } + + /** + * @return same as crossTrackDistance, only the distance returned here is the distance between + * the start point and the point to where the perpendicular drawn from point crosses the arc. + */ + public static double alongTrackDistance(LatLon start, LatLon end, LatLon point) + { + return alongTrackDistance( + toRadians(start.getLatitude()), + toRadians(start.getLongitude()), + toRadians(end.getLatitude()), + toRadians(end.getLongitude()), + toRadians(point.getLatitude()), + toRadians(point.getLongitude()) + ); } /** @@ -155,10 +186,10 @@ public static boolean isWithinDistance(double distance, List line1, List public static double bearing(LatLon pos1, LatLon pos2) { double bearing = Math.toDegrees(bearing( - Math.toRadians(pos1.getLatitude()), - Math.toRadians(pos1.getLongitude()), - Math.toRadians(pos2.getLatitude()), - Math.toRadians(pos2.getLongitude()) + toRadians(pos1.getLatitude()), + toRadians(pos1.getLongitude()), + toRadians(pos2.getLatitude()), + toRadians(pos2.getLongitude()) )); if(bearing < 0) bearing += 360; @@ -173,10 +204,10 @@ public static double bearing(LatLon pos1, LatLon pos2) public static double finalBearing(LatLon pos1, LatLon pos2) { double bearing = Math.toDegrees(finalBearing( - Math.toRadians(pos1.getLatitude()), - Math.toRadians(pos1.getLongitude()), - Math.toRadians(pos2.getLatitude()), - Math.toRadians(pos2.getLongitude()) + toRadians(pos1.getLatitude()), + toRadians(pos1.getLongitude()), + toRadians(pos2.getLatitude()), + toRadians(pos2.getLongitude()) )); if(bearing < 0) bearing += 360; @@ -408,32 +439,53 @@ public static boolean isRingDefinedClockwise(List ring) return sum > 0; } - // https://en.wikipedia.org/wiki/Great-circle_navigation#cite_note-2 + private static double crossTrackDistance(double φ1, double λ1, double φ2, double λ2, double φ3, double λ3) + { + double θ12 = bearing(φ1, λ1, φ2, λ2); + double θ13 = bearing(φ1, λ1, φ3, λ3); + double δ13 = distance(φ1, λ1, φ3, λ3) / EARTH_RADIUS; + + double δxt = asin(sin(δ13) * sin(θ13-θ12)); + return abs(δxt * EARTH_RADIUS); + } + + private static double alongTrackDistance(double φ1, double λ1, double φ2, double λ2, double φ3, double λ3) + { + double θ12 = bearing(φ1, λ1, φ2, λ2); + double θ13 = bearing(φ1, λ1, φ3, λ3); + double δ13 = distance(φ1, λ1, φ3, λ3) / EARTH_RADIUS; + + double δxt = asin(sin(δ13) * sin(θ13-θ12)); + double δat = acos(cos(δ13) / abs(cos(δxt))); + return δat * signum(cos(θ12-θ13)) * EARTH_RADIUS; + } + + // http://www.movable-type.co.uk/scripts/latlong.html private static double distance(double φ1, double λ1, double φ2, double λ2) { double Δλ = λ2 - λ1; + double Δφ = φ2 - φ1; - double y = sqrt(sqr(cos(φ2)*sin(Δλ)) + sqr(cos(φ1)*sin(φ2) - sin(φ1)*cos(φ2)*cos(Δλ))); - double x = sin(φ1)*sin(φ2) + cos(φ1)*cos(φ2)*cos(Δλ); - return atan2(y, x); + double a = sqr(sin(Δφ/2)) + cos(φ1) * cos(φ2) * sqr(sin(Δλ/2)); + double c = 2 * atan2(sqrt(a), sqrt(1-a)); + return c * EARTH_RADIUS; } - //See https://en.wikipedia.org/wiki/Great-circle_navigation#Course_and_distance private static double bearing(double φ1, double λ1, double φ2, double λ2) { double Δλ = λ2 - λ1; - return Math.atan2(sin(Δλ), cos(φ1) * tan(φ2) - sin(φ1) * cos(Δλ)); + return Math.atan2(sin(Δλ) * cos(φ2), cos(φ1) * sin(φ2) - sin(φ1) * cos(φ2) * cos(Δλ)); } private static double finalBearing(double φ1, double λ1, double φ2, double λ2) { double Δλ = λ2 - λ1; - return Math.atan2(sin(Δλ), -cos(φ2)*tan(φ1) + sin(φ1)*cos(Δλ)); + return Math.atan2(sin(Δλ) * cos(φ1), -cos(φ2) * sin(φ1) + sin(φ2) * cos(φ1) * cos(Δλ)); } private static double sqr(double x) { return Math.pow(x, 2); } - private static LatLon createTranslated(double lat, double lon) + public static LatLon createTranslated(double lat, double lon) { lon = normalizeLongitude(lon); diff --git a/app/src/main/res/animator/scissor_left.xml b/app/src/main/res/animator/scissor_left.xml new file mode 100644 index 0000000000..506446559e --- /dev/null +++ b/app/src/main/res/animator/scissor_left.xml @@ -0,0 +1,16 @@ + + + + + diff --git a/app/src/main/res/animator/scissor_right.xml b/app/src/main/res/animator/scissor_right.xml new file mode 100644 index 0000000000..88020764b3 --- /dev/null +++ b/app/src/main/res/animator/scissor_right.xml @@ -0,0 +1,16 @@ + + + + + diff --git a/app/src/main/res/animator/scissors_snip.xml b/app/src/main/res/animator/scissors_snip.xml new file mode 100644 index 0000000000..4c1497dd71 --- /dev/null +++ b/app/src/main/res/animator/scissors_snip.xml @@ -0,0 +1,43 @@ + + + + + + + + + + + + diff --git a/app/src/main/res/authors.txt b/app/src/main/res/authors.txt index a8b9b72fa2..5ef194403a 100644 --- a/app/src/main/res/authors.txt +++ b/app/src/main/res/authors.txt @@ -103,6 +103,8 @@ produce_mate.jpg CC-BY-SA 3.0 https://commons.wikimedia.org/w produce_jojoba.jpg CC-BY 2.0 https://de.wikipedia.org/wiki/Datei:Jojoba_Nut_-_Flickr_-_treegrow.jpg produce_tomato.jpg CC-BY-SA 3.0 https://commons.wikimedia.org/wiki/File:Solanum_lycopersicum_%27Cronos%27,_tomaat_%27Cronos%27.jpg +snip.wav CC0 https://freesound.org/people/Godowan/sounds/240473/ + surface_asphalt.jpg Public Domain https://commons.wikimedia.org/wiki/File:Ground_frost_damages.JPG surface_cobblestone.jpg CC-BY-SA 3.0 https://commons.wikimedia.org/wiki/File:Bad_Radkersburg_Murgasse_IMG_0583.jpg surface_compacted.jpg CC-BY 2.0 https://www.flickr.com/photos/ian_munroe/3633361321 diff --git a/app/src/main/res/drawable-hdpi/crosshair_marker.png b/app/src/main/res/drawable-hdpi/crosshair_marker.png new file mode 100644 index 0000000000..dec5b07c2c Binary files /dev/null and b/app/src/main/res/drawable-hdpi/crosshair_marker.png differ diff --git a/app/src/main/res/drawable-mdpi/crosshair_marker.png b/app/src/main/res/drawable-mdpi/crosshair_marker.png new file mode 100644 index 0000000000..85d6a2d54d Binary files /dev/null and b/app/src/main/res/drawable-mdpi/crosshair_marker.png differ diff --git a/app/src/main/res/drawable-v21/scissors_animation.xml b/app/src/main/res/drawable-v21/scissors_animation.xml new file mode 100644 index 0000000000..aef523b612 --- /dev/null +++ b/app/src/main/res/drawable-v21/scissors_animation.xml @@ -0,0 +1,10 @@ + + + + + diff --git a/app/src/main/res/drawable-xhdpi/crosshair_marker.png b/app/src/main/res/drawable-xhdpi/crosshair_marker.png new file mode 100644 index 0000000000..5b89dacf28 Binary files /dev/null and b/app/src/main/res/drawable-xhdpi/crosshair_marker.png differ diff --git a/app/src/main/res/drawable-xxhdpi/crosshair_marker.png b/app/src/main/res/drawable-xxhdpi/crosshair_marker.png new file mode 100644 index 0000000000..cb5d2d6bcf Binary files /dev/null and b/app/src/main/res/drawable-xxhdpi/crosshair_marker.png differ diff --git a/app/src/main/res/drawable/ic_undo_black_24dp.xml b/app/src/main/res/drawable/ic_undo_black_24dp.xml new file mode 100644 index 0000000000..3f477a3527 --- /dev/null +++ b/app/src/main/res/drawable/ic_undo_black_24dp.xml @@ -0,0 +1,9 @@ + + + diff --git a/app/src/main/res/drawable/scissors.xml b/app/src/main/res/drawable/scissors.xml new file mode 100644 index 0000000000..a2c5bc666a --- /dev/null +++ b/app/src/main/res/drawable/scissors.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + diff --git a/app/src/main/res/drawable/scissors_animation.xml b/app/src/main/res/drawable/scissors_animation.xml new file mode 100644 index 0000000000..85300ba541 --- /dev/null +++ b/app/src/main/res/drawable/scissors_animation.xml @@ -0,0 +1,5 @@ + + + + diff --git a/app/src/main/res/layout/fragment_split_way.xml b/app/src/main/res/layout/fragment_split_way.xml new file mode 100644 index 0000000000..9001b3e75f --- /dev/null +++ b/app/src/main/res/layout/fragment_split_way.xml @@ -0,0 +1,116 @@ + + + + + + + + + + + + + + + + + + + + + + + + + +