Merge pull request #2511 from deathbaba/fixes

Editor statistics and minor fixes.
This commit is contained in:
Viktor Govako 2016-03-25 12:55:20 +03:00
commit 86d5bd040d
10 changed files with 73 additions and 27 deletions

View file

@ -227,7 +227,7 @@ Java_com_mapswithme_maps_editor_Editor_nativeHasWifi(JNIEnv *, jclass)
JNIEXPORT jboolean JNICALL
Java_com_mapswithme_maps_editor_Editor_nativeHasSomethingToUpload(JNIEnv * env, jclass clazz)
{
return Editor::Instance().HaveSomethingToUpload();
return Editor::Instance().HaveMapEditsOrNotesToUpload();
}
JNIEXPORT void JNICALL

View file

@ -53,6 +53,8 @@ public:
/// Throws exceptions from above list.
void Delete(editor::XMLFeature node);
uint64_t GetChangesetId() const { return m_changesetId; }
private:
/// Unfortunately, pugi can't return xml_documents from methods.
/// Throws exceptions from above list.

View file

@ -38,6 +38,7 @@
#include "std/unordered_map.hpp"
#include "std/unordered_set.hpp"
#include "3party/Alohalytics/src/alohalytics.h"
#include "3party/pugixml/src/pugixml.hpp"
using namespace pugi;
@ -223,7 +224,7 @@ void Editor::LoadMapEdits()
case FeatureStatus::Deleted: ++deleted; break;
case FeatureStatus::Modified: ++modified; break;
case FeatureStatus::Created: ++created; break;
case FeatureStatus::Untouched: ASSERT(false, ()); break;
case FeatureStatus::Untouched: ASSERT(false, ()); continue;
}
// Insert initialized structure at the end: exceptions are possible in above code.
m_features[fid.m_mwmId].emplace(fid.m_index, move(fti));
@ -507,10 +508,8 @@ EditableProperties Editor::GetEditablePropertiesForTypes(feature::TypesHolder co
return {};
}
bool Editor::HaveSomethingToUpload() const
bool Editor::HaveMapEditsToUpload() const
{
if (m_notes->NotUploadedNotesCount() != 0)
return true;
for (auto const & id : m_features)
{
for (auto const & index : id.second)
@ -522,7 +521,15 @@ bool Editor::HaveSomethingToUpload() const
return false;
}
bool Editor::HaveSomethingToUpload(MwmSet::MwmId const & mwmId) const
bool Editor::HaveMapEditsOrNotesToUpload() const
{
if (m_notes->NotUploadedNotesCount() != 0)
return true;
return HaveMapEditsToUpload();
}
bool Editor::HaveMapEditsToUpload(MwmSet::MwmId const & mwmId) const
{
auto const found = m_features.find(mwmId);
if (found != m_features.end())
@ -542,11 +549,14 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
if (m_notes->NotUploadedNotesCount())
UploadNotes(key, secret);
if (!HaveSomethingToUpload())
if (!HaveMapEditsToUpload())
{
LOG(LDEBUG, ("There are no local edits to upload."));
return;
}
alohalytics::LogEvent("Editor_DataSync_started");
// TODO(AlexZ): features access should be synchronized.
auto const upload = [this](string key, string secret, TChangesetTags tags, TFinishUploadCallback callBack)
{
@ -564,15 +574,20 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
// Do not process already uploaded features or those failed permanently.
if (!NeedsUpload(fti.m_uploadStatus))
continue;
string ourDebugFeatureString;
try
{
XMLFeature feature = fti.m_feature.ToXML();
if (!fti.m_street.empty())
feature.SetTagValue(kAddrStreetTag, fti.m_street);
ourDebugFeatureString = DebugPrint(feature);
switch (fti.m_status)
{
case FeatureStatus::Untouched: CHECK(false, ("It's impossible.")); break;
case FeatureStatus::Untouched: CHECK(false, ("It's impossible.")); continue;
case FeatureStatus::Created:
{
@ -638,15 +653,12 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
break;
}
fti.m_uploadStatus = kUploaded;
// TODO(AlexZ): Use timestamp from the server.
fti.m_uploadAttemptTimestamp = time(nullptr);
fti.m_uploadError.clear();
++uploadedFeaturesCount;
}
catch (ChangesetWrapper::OsmObjectWasDeletedException const & ex)
{
fti.m_uploadStatus = kDeletedFromOSMServer;
fti.m_uploadAttemptTimestamp = time(nullptr);
fti.m_uploadError = ex.what();
++errorsCount;
LOG(LWARNING, (ex.what()));
@ -654,7 +666,6 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
catch (ChangesetWrapper::RelationFeatureAreNotSupportedException const & ex)
{
fti.m_uploadStatus = kRelationsAreNotSupported;
fti.m_uploadAttemptTimestamp = time(nullptr);
fti.m_uploadError = ex.what();
++errorsCount;
LOG(LWARNING, (ex.what()));
@ -662,7 +673,6 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
catch (ChangesetWrapper::EmptyFeatureException const & ex)
{
fti.m_uploadStatus = kWrongMatch;
fti.m_uploadAttemptTimestamp = time(nullptr);
fti.m_uploadError = ex.what();
++errorsCount;
LOG(LWARNING, (ex.what()));
@ -670,16 +680,30 @@ void Editor::UploadChanges(string const & key, string const & secret, TChangeset
catch (RootException const & ex)
{
fti.m_uploadStatus = kNeedsRetry;
fti.m_uploadAttemptTimestamp = time(nullptr);
fti.m_uploadError = ex.what();
++errorsCount;
LOG(LWARNING, (ex.what()));
}
// TODO(AlexZ): Use timestamp from the server.
fti.m_uploadAttemptTimestamp = time(nullptr);
if (fti.m_uploadStatus != kUploaded)
{
ms::LatLon const ll = MercatorBounds::ToLatLon(feature::GetCenter(fti.m_feature));
alohalytics::LogEvent("Editor_DataSync_error", {{"type", fti.m_uploadStatus},
{"details", fti.m_uploadError}, {"our", ourDebugFeatureString},
{"mwm", fti.m_feature.GetID().GetMwmName()},
{"mwm_version", strings::to_string(fti.m_feature.GetID().GetMwmVersion())}},
alohalytics::Location::FromLatLon(ll.lat, ll.lon));
}
// Call Save every time we modify each feature's information.
SaveUploadedInformation(fti);
}
}
alohalytics::LogEvent("Editor_DataSync_finished", {{"errors", strings::to_string(errorsCount)},
{"uploaded", strings::to_string(uploadedFeaturesCount)},
{"changeset", strings::to_string(changeset.GetChangesetId())}});
if (callBack)
{
UploadResult result = UploadResult::NothingToUpload;
@ -831,6 +855,7 @@ void Editor::CreateNote(m2::PointD const & point, string const & note)
void Editor::UploadNotes(string const & key, string const & secret)
{
alohalytics::LogEvent("Editor_UploadNotes", strings::to_string(m_notes->NotUploadedNotesCount()));
m_notes->Upload(OsmOAuth::ServerAuth({key, secret}));
}

View file

@ -102,8 +102,9 @@ public:
EditableProperties GetEditableProperties(FeatureType const & feature) const;
bool HaveSomethingToUpload() const;
bool HaveSomethingToUpload(MwmSet::MwmId const & mwmId) const;
bool HaveMapEditsOrNotesToUpload() const;
bool HaveMapEditsToUpload(MwmSet::MwmId const & mwmId) const;
bool HaveMapEditsToUpload() const;
using TChangesetTags = map<string, string>;
/// Tries to upload all local changes to OSM server in a separate thread.
/// @param[in] tags should provide additional information about client to use in changeset.

View file

@ -247,11 +247,7 @@ using namespace osm_auth_ios;
- (IBAction)cancel
{
UINavigationController * parentNavController = self.navigationController.navigationController;
if (parentNavController)
[parentNavController popViewControllerAnimated:YES];
else
[self dismissViewControllerAnimated:YES completion:nil];
[self.navigationController popViewControllerAnimated:YES];
}
- (IBAction)localChangesAction

View file

@ -2,6 +2,7 @@
#import "MWMAuthorizationCommon.h"
#import "MWMAuthorizationOSMLoginViewController.h"
#import "MWMCircularProgress.h"
#import "Statistics.h"
#import "UIColor+MapsMeColor.h"
#import "UITextField+RuntimeAttributes.h"
@ -138,6 +139,9 @@ using namespace osm;
catch (exception const & ex)
{
LOG(LWARNING, ("Error login", ex.what()));
[Statistics logEvent:@"Editor_Reg_request_result" withParameters:@{kStatIsSuccess : kStatNo,
kStatErrorData : @(ex.what()),
kStatType : kStatOSM}];
}
dispatch_async(dispatch_get_main_queue(), ^
{
@ -145,7 +149,9 @@ using namespace osm;
if (auth.IsAuthorized())
{
osm_auth_ios::AuthorizationStoreCredentials(auth.GetKeySecret());
[self dismissViewControllerAnimated:YES completion:nil];
[Statistics logEvent:@"Editor_Reg_request_result" withParameters:@{kStatIsSuccess : kStatYes,
kStatType : kStatOSM}];
[self.navigationController popToRootViewControllerAnimated:YES];
}
else
{

View file

@ -2,6 +2,7 @@
#import "MWMAuthorizationCommon.h"
#import "MWMAuthorizationWebViewLoginViewController.h"
#import "MWMCircularProgress.h"
#import "Statistics.h"
#include "base/logging.hpp"
#include "editor/osm_auth.hpp"
@ -128,6 +129,15 @@ NSString * getVerifier(NSString * urlString)
self.webView.userInteractionEnabled = YES;
}
- (NSString *)authTypeAsString
{
switch (self.authType)
{
case MWMWebViewAuthorizationTypeGoogle: return kStatGoogle;
case MWMWebViewAuthorizationTypeFacebook: return kStatFacebook;
}
}
- (void)checkAuthorization:(NSString *)verifier
{
[self startSpinner];
@ -142,6 +152,9 @@ NSString * getVerifier(NSString * urlString)
catch (exception const & ex)
{
LOG(LWARNING, ("checkAuthorization error", ex.what()));
[Statistics logEvent:@"Editor_Reg_request_result" withParameters:@{kStatIsSuccess : kStatNo,
kStatErrorData : @(ex.what()),
kStatType : self.authTypeAsString}];
}
dispatch_async(dispatch_get_main_queue(), ^
{
@ -149,10 +162,13 @@ NSString * getVerifier(NSString * urlString)
if (OsmOAuth::IsValid(ks))
{
osm_auth_ios::AuthorizationStoreCredentials(ks);
[self dismissViewControllerAnimated:NO completion:nil];
[Statistics logEvent:@"Editor_Reg_request_result" withParameters:@{kStatIsSuccess : kStatYes,
kStatType : self.authTypeAsString}];
[self.navigationController popToRootViewControllerAnimated:YES];
}
else
{
// Do not log statistics here because it has been already logged in catch above.
[self loadAuthorizationPage];
[self.alertController presentInvalidUserNameOrPasswordAlert];
}

View file

@ -493,7 +493,7 @@ using namespace osm_auth_ios;
[Alohalytics forceUpload:callback];
});
// 2. Upload map edits (if any).
if (osm::Editor::Instance().HaveSomethingToUpload() && AuthorizationHaveCredentials())
if (osm::Editor::Instance().HaveMapEditsOrNotesToUpload() && AuthorizationHaveCredentials())
{
runFetchTask(^
{
@ -534,7 +534,7 @@ using namespace osm_auth_ios;
}];
}
// Upload map edits if any, but only if we have Internet connection and user has already been authorized.
if (osm::Editor::Instance().HaveSomethingToUpload() &&
if (osm::Editor::Instance().HaveMapEditsOrNotesToUpload() &&
AuthorizationHaveCredentials() &&
Platform::EConnectionType::CONNECTION_NONE != Platform::ConnectionStatus())
{

View file

@ -531,7 +531,7 @@ bool Framework::HasUnsavedEdits(storage::TCountryId const & countryId)
{
if (groupNode)
return;
hasUnsavedChanges |= osm::Editor::Instance().HaveSomethingToUpload(
hasUnsavedChanges |= osm::Editor::Instance().HaveMapEditsToUpload(
m_model.GetIndex().GetMwmIdByCountryFile(platform::CountryFile(fileName)));
};
Storage().ForEachInSubtree(countryId, forEachInSubtree);

View file

@ -502,7 +502,7 @@ void MainWindow::OnUploadEditsMenuItem()
else
{
auto & editor = osm::Editor::Instance();
if (editor.HaveSomethingToUpload())
if (editor.HaveMapEditsOrNotesToUpload())
editor.UploadChanges(key, secret, {{"created_by", "MAPS.ME " OMIM_OS_NAME}});
}
}