Skip to content

Loading…

sending a pull request #92

Merged
merged 1 commit into from

2 participants

@justinharding

This should fix the "deleted file downloaded again" problem - the server time for the lastest change is not stored in State::m_last_sync instead of the client time - this makes time comparisons more useful - I haven't thoroughly that m_last_sync is not being compared elsewhere with client side times. I also removed the check for the remote changestamp as this is not appropriate - the remote file will get a changestamp when it is created as part of the upload.

@nestal nestal merged commit 8547bbf into Grive:master
@nestal
The development team of Grive member
@justinharding
@nestal
The development team of Grive member

I suggest we bring this to the discussion group: grive-devel@googlegroups.com

Have you subscribed yet?

@justinharding

yes I have - I've also made the change you recommended - m_mtime is updated in the Upload function - sync_time is no longer a param except in the Sync function. I will send you a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 15, 2012
  1. fix deleted local file downloaded again

    justin committed
Showing with 31 additions and 18 deletions.
  1. +12 −11 libgrive/src/drive/Resource.cc
  2. +5 −5 libgrive/src/drive/Resource.hh
  3. +14 −2 libgrive/src/drive/State.cc
View
23 libgrive/src/drive/Resource.cc
@@ -360,20 +360,20 @@ Resource* Resource::FindChild( const std::string& name )
}
// try to change the state to "sync"
-void Resource::Sync( http::Agent *http, const http::Header& auth )
+void Resource::Sync( http::Agent *http, const http::Header& auth, DateTime& sync_time )
{
assert( m_state != unknown ) ;
assert( !IsRoot() || m_state == sync ) ; // root folder is already synced
- SyncSelf( http, auth ) ;
+ SyncSelf( http, auth, sync_time ) ;
// if myself is deleted, no need to do the childrens
if ( m_state != local_deleted && m_state != remote_deleted )
std::for_each( m_child.begin(), m_child.end(),
- boost::bind( &Resource::Sync, _1, http, auth ) ) ;
+ boost::bind( &Resource::Sync, _1, http, auth, boost::ref(sync_time) ) ) ;
}
-void Resource::SyncSelf( http::Agent* http, const http::Header& auth )
+void Resource::SyncSelf( http::Agent* http, const http::Header& auth, DateTime& sync_time )
{
assert( !IsRoot() || m_state == sync ) ; // root is always sync
assert( IsRoot() || http == 0 || fs::is_directory( m_parent->Path() ) ) ;
@@ -387,7 +387,7 @@ void Resource::SyncSelf( http::Agent* http, const http::Header& auth )
case local_new :
Log( "sync %1% doesn't exist in server, uploading", path, log::info ) ;
- if ( http != 0 && Create( http, auth ) )
+ if ( http != 0 && Create( http, auth, sync_time ) )
m_state = sync ;
break ;
@@ -399,7 +399,7 @@ void Resource::SyncSelf( http::Agent* http, const http::Header& auth )
case local_changed :
Log( "sync %1% changed in local. uploading", path, log::info ) ;
- if ( http != 0 && EditContent( http, auth ) )
+ if ( http != 0 && EditContent( http, auth, sync_time ) )
m_state = sync ;
break ;
@@ -512,7 +512,7 @@ void Resource::Download( http::Agent* http, const fs::path& file, const http::He
}
}
-bool Resource::EditContent( http::Agent* http, const http::Header& auth )
+bool Resource::EditContent( http::Agent* http, const http::Header& auth, DateTime& sync_time )
{
assert( http != 0 ) ;
assert( m_parent != 0 ) ;
@@ -525,10 +525,10 @@ bool Resource::EditContent( http::Agent* http, const http::Header& auth )
return false ;
}
- return Upload( http, m_edit, auth, false ) ;
+ return Upload( http, m_edit, auth, false, sync_time ) ;
}
-bool Resource::Create( http::Agent* http, const http::Header& auth )
+bool Resource::Create( http::Agent* http, const http::Header& auth, DateTime& sync_time )
{
assert( http != 0 ) ;
assert( m_parent != 0 ) ;
@@ -558,7 +558,7 @@ bool Resource::Create( http::Agent* http, const http::Header& auth )
}
else if ( !m_parent->m_create.empty() )
{
- return Upload( http, m_parent->m_create + "?convert=false", auth, true ) ;
+ return Upload( http, m_parent->m_create + "?convert=false", auth, true, sync_time ) ;
}
else
{
@@ -567,7 +567,7 @@ bool Resource::Create( http::Agent* http, const http::Header& auth )
}
}
-bool Resource::Upload( http::Agent* http, const std::string& link, const http::Header& auth, bool post )
+bool Resource::Upload( http::Agent* http, const std::string& link, const http::Header& auth, bool post, DateTime& sync_time )
{
assert( http != 0 ) ;
@@ -611,6 +611,7 @@ bool Resource::Upload( http::Agent* http, const std::string& link, const http::H
http->Put( uplink, data, &xml, uphdr ) ;
AssignIDs( Entry( xml.Response() ) ) ;
+ sync_time = std::max(Entry(xml.Response()).MTime(), sync_time);
return true ;
}
View
10 libgrive/src/drive/Resource.hh
@@ -77,7 +77,7 @@ public :
void FromRemote( const Entry& remote, const DateTime& last_sync ) ;
void FromLocal( const DateTime& last_sync ) ;
- void Sync( http::Agent* http, const http::Header& auth ) ;
+ void Sync( http::Agent* http, const http::Header& auth, DateTime& sync_time ) ;
// children access
iterator begin() const ;
@@ -125,9 +125,9 @@ private :
void SetState( State new_state ) ;
void Download( http::Agent* http, const fs::path& file, const http::Header& auth ) const ;
- bool EditContent( http::Agent* http, const http::Header& auth ) ;
- bool Create( http::Agent* http, const http::Header& auth ) ;
- bool Upload( http::Agent* http, const std::string& link, const http::Header& auth, bool post ) ;
+ bool EditContent( http::Agent* http, const http::Header& auth, DateTime& sync_time ) ;
+ bool Create( http::Agent* http, const http::Header& auth, DateTime& sync_time ) ;
+ bool Upload( http::Agent* http, const std::string& link, const http::Header& auth, bool post, DateTime& sync_time ) ;
void FromRemoteFolder( const Entry& remote, const DateTime& last_sync ) ;
void FromRemoteFile( const Entry& remote, const DateTime& last_sync ) ;
@@ -136,7 +136,7 @@ private :
void DeleteRemote( http::Agent* http, const http::Header& auth ) ;
void AssignIDs( const Entry& remote ) ;
- void SyncSelf( http::Agent* http, const http::Header& auth ) ;
+ void SyncSelf( http::Agent* http, const http::Header& auth, DateTime& sync_time ) ;
private :
std::string m_name ;
View
16 libgrive/src/drive/State.cc
@@ -264,8 +264,20 @@ void State::Write( const fs::path& filename ) const
void State::Sync( http::Agent *http, const http::Header& auth )
{
- m_res.Root()->Sync( http, auth ) ;
- m_last_sync = DateTime::Now() ;
+ // set the last sync time from the time returned by the server for the last file synced
+ // if the sync time hasn't changed (i.e. now files have been uploaded)
+ // set the last sync time to the time on the client
+ // ideally because we compare server file times to the last sync time
+ // the last sync time would always be a server time rather than a client time
+ // TODO - WARNING - do we use the last sync time to compare to client file times
+ // need to check if this introduces a new problem
+ DateTime last_sync_time = m_last_sync;
+ m_res.Root()->Sync( http, auth, last_sync_time ) ;
+ if (last_sync_time == m_last_sync) {
+ m_last_sync = DateTime::Now();
+ } else {
+ m_last_sync = last_sync_time;
+ }
}
long State::ChangeStamp() const
Something went wrong with that request. Please try again.