Let ServiceRefresh en-/disable repos with respect to previous stae and user modifications

diff --git a/zypp/RepoManager.cc b/zypp/RepoManager.cc
index d82f443..894902d 100644 (file)
--- a/zypp/RepoManager.cc
+++ b/zypp/RepoManager.cc
@@ -1838,12 +1838,24 @@ namespace zypp
       uglyHack.second = e;
     }
 
+    ////////////////////////////////////////////////////////////////////////////
+    // On the fly remember the new repo states as defined the reopoindex.xml.
+    // Move into ServiceInfo later.
+    ServiceInfo::RepoStates newRepoStates;
+
     // set service alias and base url for all collected repositories
     for_( it, collector.repos.begin(), collector.repos.end() )
     {
+      // First of all: Prepend service alias:
+      it->setAlias( str::form( "%s:%s", service.alias().c_str(), it->alias().c_str() ) );
+      // set refrence to the parent service
+      it->setService( service.alias() );
+
+      // remember the new parsed repo state
+      newRepoStates[it->alias()] = *it;
+
       // if the repo url was not set by the repoindex parser, set service's url
       Url url;
-
       if ( it->baseUrlsEmpty() )
         url = service.url();
       else
@@ -1862,13 +1874,8 @@ namespace zypp
         it->setPath("");
       }
 
-      // Prepend service alias:
-      it->setAlias( str::form( "%s:%s", service.alias().c_str(), it->alias().c_str() ) );
-
       // save the url
       it->setBaseUrl( url );
-      // set refrence to the parent service
-      it->setService( service.alias() );
     }
 
     ////////////////////////////////////////////////////////////////////////////
@@ -1877,22 +1884,29 @@ namespace zypp
     RepoInfoList oldRepos;
     getRepositoriesInService( service.alias(), std::back_inserter( oldRepos ) );
 
+    ////////////////////////////////////////////////////////////////////////////
     // find old repositories to remove...
-    for_( it, oldRepos.begin(), oldRepos.end() )
+    for_( oldRepo, oldRepos.begin(), oldRepos.end() )
     {
-      if ( ! foundAliasIn( it->alias(), collector.repos ) )
+      if ( ! foundAliasIn( oldRepo->alias(), collector.repos ) )
       {
-        if ( it->enabled() && ! service.repoToDisableFind( it->alias() ) )
-        {
-          DBG << "Service removes enabled repo " << it->alias() << endl;
-          service.addRepoToEnable( it->alias() );
-          serviceModified = true;
-        }
-        else
-        {
-          DBG << "Service removes disabled repo " << it->alias() << endl;
-        }
-        removeRepository( *it );
+       if ( oldRepo->enabled() )
+       {
+         // Currently enabled. If this was a user modification remember the state.
+         const auto & last = service.repoStates().find( oldRepo->alias() );
+         if ( last != service.repoStates().end() && ! last->second.enabled )
+         {
+           DBG << "Service removes user enabled repo " << oldRepo->alias() << endl;
+           service.addRepoToEnable( oldRepo->alias() );
+           serviceModified = true;
+         }
+         else
+           DBG << "Service removes enabled repo " << oldRepo->alias() << endl;
+       }
+       else
+         DBG << "Service removes disabled repo " << oldRepo->alias() << endl;
+
+        removeRepository( *oldRepo );
       }
     }
 
@@ -1900,76 +1914,101 @@ namespace zypp
     // create missing repositories and modify exising ones if needed...
     for_( it, collector.repos.begin(), collector.repos.end() )
     {
-      // Service explicitly requests the repo being enabled?
-      // Service explicitly requests the repo being disabled?
+      // User explicitly requested the repo being enabled?
+      // User explicitly requested the repo being disabled?
       // And hopefully not both ;) If so, enable wins.
-      bool beEnabled = service.repoToEnableFind( it->alias() );
-      bool beDisabled = service.repoToDisableFind( it->alias() );
 
-      // Make sure the service repo is created with the
-      // appropriate enable
-      if ( beEnabled ) it->setEnabled(true);
-      if ( beDisabled ) it->setEnabled(false);
+      TriBool toBeEnabled( indeterminate );    // indeterminate - follow the service request
+      DBG << "Service request to " << (it->enabled()?"enable":"disable") << " service repo " << it->alias() << endl;
 
-      if ( beEnabled )
+      if ( service.repoToEnableFind( it->alias() ) )
       {
+       DBG << "User request to enable service repo " << it->alias() << endl;
+       toBeEnabled = true;
         // Remove from enable request list.
         // NOTE: repoToDisable is handled differently.
         //       It gets cleared on each refresh.
         service.delRepoToEnable( it->alias() );
         serviceModified = true;
       }
+      else if ( service.repoToDisableFind( it->alias() ) )
+      {
+       DBG << "User request to disable service repo " << it->alias() << endl;
+       toBeEnabled = false;
+      }
+
 
       RepoInfoList::iterator oldRepo( findAlias( it->alias(), oldRepos ) );
       if ( oldRepo == oldRepos.end() )
       {
         // Not found in oldRepos ==> a new repo to add
 
-        // At that point check whether a repo with the same alias
-        // exists outside this service. Maybe forcefully re-alias
-        // the existing repo?
+       // Make sure the service repo is created with the appropriate enablement
+       if ( ! indeterminate(toBeEnabled) )
+         it->setEnabled( toBeEnabled );
+
         DBG << "Service adds repo " << it->alias() << " " << (it->enabled()?"enabled":"disabled") << endl;
         addRepository( *it );
-
-        // save repo credentials
-        // ma@: task for modifyRepository?
       }
       else
       {
         // ==> an exising repo to check
         bool oldRepoModified = false;
 
+       if ( indeterminate(toBeEnabled) )
+       {
+         // No user request: check for an old user modificaton otherwise follow service request.
+         // NOTE: Assert toBeEnabled is boolean afterwards!
+         if ( oldRepo->enabled() == it->enabled() )
+           toBeEnabled = it->enabled();        // service requests no change to the system
+         else
+         {
+           const auto & last = service.repoStates().find( oldRepo->alias() );
+           if ( last == service.repoStates().end() || last->second.enabled != it->enabled() )
+             toBeEnabled = it->enabled();      // service request has changed since last refresh -> follow
+           else
+           {
+             toBeEnabled = oldRepo->enabled(); // service request unchaned since last refresh -> keep user modification
+             DBG << "User modified service repo " << it->alias() <<  " may stay " << (toBeEnabled?"enabled":"disabled") << endl;
+           }
+         }
+       }
+
         // changed enable?
-        if ( beEnabled )
-        {
-          if ( ! oldRepo->enabled() )
-          {
-            DBG << "Service repo " << it->alias() << " gets enabled" << endl;
-            oldRepo->setEnabled( true );
-            oldRepoModified = true;
-          }
-          else
-          {
-            DBG << "Service repo " << it->alias() << " stays enabled" << endl;
-          }
-        }
-        else if ( beDisabled )
-        {
-          if ( oldRepo->enabled() )
-          {
-            DBG << "Service repo " << it->alias() << " gets disabled" << endl;
-            oldRepo->setEnabled( false );
-            oldRepoModified = true;
-          }
-          else
-          {
-            DBG << "Service repo " << it->alias() << " stays disabled" << endl;
-          }
-        }
-        else
+       if ( toBeEnabled == oldRepo->enabled() )
+       {
+         DBG << "Service repo " << it->alias() << " stays " <<  (oldRepo->enabled()?"enabled":"disabled") << endl;
+       }
+       else if ( toBeEnabled )
+       {
+         DBG << "Service repo " << it->alias() << " gets enabled" << endl;
+         oldRepo->setEnabled( true );
+         oldRepoModified = true;
+       }
+       else
         {
-          DBG << "Service repo " << it->alias() << " stays " <<  (oldRepo->enabled()?"enabled":"disabled") << endl;
-        }
+         DBG << "Service repo " << it->alias() << " gets disabled" << endl;
+         oldRepo->setEnabled( false );
+         oldRepoModified = true;
+       }
+
+       // all other attributes follow the service request:
+
+       // changed autorefresh
+       if ( oldRepo->autorefresh() != it->autorefresh() )
+       {
+         DBG << "Service repo " << it->alias() << " gets new AUTOREFRESH " << it->autorefresh() << endl;
+         oldRepo->setAutorefresh( it->autorefresh() );
+         oldRepoModified = true;
+       }
+
+       // changed priority?
+       if ( oldRepo->priority() != it->priority() )
+       {
+         DBG << "Service repo " << it->alias() << " gets new PRIORITY " << it->priority() << endl;
+         oldRepo->setPriority( it->priority() );
+         oldRepoModified = true;
+       }
 
         // changed url?
         // service repo can contain only one URL now, so no need to iterate.
@@ -1995,6 +2034,13 @@ namespace zypp
       serviceModified = true;
     }
 
+    // Remember original service request for next refresh
+    if ( service.repoStates() != newRepoStates )
+    {
+      service.setRepoStates( std::move(newRepoStates) );
+      serviceModified = true;
+    }
+
     ////////////////////////////////////////////////////////////////////////////
     // save service if modified:
     if ( serviceModified )

diff --git a/zypp/ServiceInfo.cc b/zypp/ServiceInfo.cc
index 48c5597..11220d3 100644 (file)
--- a/zypp/ServiceInfo.cc
+++ b/zypp/ServiceInfo.cc
@@ -41,6 +41,8 @@ namespace zypp
     repo::ServiceType type;
     ReposToEnable  reposToEnable;
     ReposToDisable reposToDisable;
+    RepoStates     repoStates;
+
 
   public:
     Impl()
@@ -164,6 +166,19 @@ namespace zypp
   void ServiceInfo::clearReposToDisable()
   { _pimpl->reposToDisable.clear(); }
 
+  const ServiceInfo::RepoStates & ServiceInfo::repoStates() const
+  { return _pimpl->repoStates; }
+
+  void ServiceInfo::setRepoStates( RepoStates newStates_r )
+  { swap( _pimpl->repoStates, newStates_r ); }
+
+  std::ostream & operator<<( std::ostream & str, const ServiceInfo::RepoState & obj )
+  {
+    return str
+       << "enabled=" << obj.enabled << " "
+       << "autorefresh=" << obj.autorefresh << " "
+       << "priority=" << obj.priority;
+  }
 
   std::ostream & ServiceInfo::dumpAsIniOn( std::ostream & str ) const
   {
@@ -171,6 +186,24 @@ namespace zypp
       << "url = " << url() << endl
       << "type = " << type() << endl;
 
+    if ( ! repoStates().empty() )
+    {
+      unsigned cnt = 0U;
+      for ( const auto & el : repoStates() )
+      {
+       std::string tag( "repo_" );
+       tag += str::numstring( ++cnt );
+       const RepoState & state( el.second );
+
+       str << tag << "=" << el.first << endl
+           << tag << "_enabled=" << state.enabled << endl
+           << tag << "_autorefresh=" << state.autorefresh << endl;
+       if ( state.priority != RepoInfo::defaultPriority() )
+         str
+           << tag << "_priority=" << state.priority << endl;
+      }
+    }
+
     if ( ! reposToEnableEmpty() )
       str << "repostoenable = " << str::joinEscaped( reposToEnableBegin(), reposToEnableEnd() ) << endl;
     if ( ! reposToDisableEmpty() )

diff --git a/zypp/ServiceInfo.h b/zypp/ServiceInfo.h
index d5e0bb2..d40ad8a 100644 (file)
--- a/zypp/ServiceInfo.h
+++ b/zypp/ServiceInfo.h
@@ -18,7 +18,7 @@
 #include "zypp/Url.h"
 
 #include "zypp/repo/ServiceType.h"
-#include "zypp/repo/RepoInfoBase.h"
+#include "zypp/RepoInfo.h"
 
 
 ///////////////////////////////////////////////////////////////////
@@ -137,6 +137,40 @@ namespace zypp
     void clearReposToDisable();
     //@}
 
+    /** \name The original repo state as defined by the repoindex.xml upon last refresh.
+     *
+     * This state is remembered to detect any user modifications applied to the repos.
+     * It may not be available for all repos or in plugin services. In this case all
+     * changes requested by a service refresh are applied unconditionally.
+     */
+    //@{
+    struct RepoState
+    {
+      bool     enabled;
+      bool     autorefresh;
+      unsigned priority;
+
+      RepoState()
+       : enabled( false ), autorefresh( true ), priority( RepoInfo::defaultPriority() )
+      {}
+      RepoState( const RepoInfo & repo_r )
+       : enabled( repo_r.enabled() ), autorefresh( repo_r.autorefresh() ), priority( repo_r.priority() )
+      {}
+      bool operator==( const RepoState & rhs ) const
+      { return( enabled==rhs.enabled && autorefresh==rhs.autorefresh && priority==rhs.priority ); }
+      bool operator!=( const RepoState & rhs ) const
+      { return ! operator==( rhs ); }
+      friend std::ostream & operator<<( std::ostream & str, const RepoState & obj );
+    };
+    typedef std::map<std::string,RepoState> RepoStates;
+
+    /** Access the remembered repository states. */
+    const RepoStates & repoStates() const;
+
+    /** Remember a new set of repository states. */
+    void setRepoStates( RepoStates newStates_r );
+    //@}
+
   public:
     /**
      * Writes ServiceInfo to stream in ".service" format

diff --git a/zypp/parser/ServiceFileReader.cc b/zypp/parser/ServiceFileReader.cc
index 49c3c31..e814dac 100644 (file)
--- a/zypp/parser/ServiceFileReader.cc
+++ b/zypp/parser/ServiceFileReader.cc
@@ -12,6 +12,7 @@
 #include <iostream>
 #include "zypp/base/Logger.h"
 #include "zypp/base/String.h"
+#include "zypp/base/Regex.h"
 #include "zypp/base/InputStream.h"
 #include "zypp/base/UserRequestException.h"
 
@@ -54,6 +55,7 @@ namespace zypp
         MIL << (*its) << endl;
 
         ServiceInfo service(*its);
+       std::map<std::string,std::pair<std::string,ServiceInfo::RepoState>> repoStates; // <repo_NUM,< alias,RepoState >>
 
         for ( IniDict::entry_const_iterator it = dict.entriesBegin(*its);
               it != dict.entriesEnd(*its);
@@ -88,10 +90,52 @@ namespace zypp
               service.addRepoToDisable( *ait );
             }
           }
+          else if ( str::startsWith( it->first, "repo_" ) )
+         {
+           static str::regex rxexpr( "([0-9]+)(_(.*))?" );
+           str::smatch what;
+           if ( str::regex_match( it->first.c_str()+5/*repo_*/, what, rxexpr ) )
+           {
+             std::string tag( what[1] );
+             if (  what.size() > 3 )
+             {
+               // attribute
+               if ( what[3] == "enabled" )
+                 repoStates[tag].second.enabled = str::strToBool( it->second, repoStates[tag].second.enabled );
+               else if ( what[3] == "autorefresh" )
+                 repoStates[tag].second.autorefresh = str::strToBool( it->second, repoStates[tag].second.autorefresh );
+               else if ( what[3] == "priority" )
+                 str::strtonum( it->second, repoStates[tag].second.priority );
+               else
+                 ERR << "Unknown attribute " << it->first << " ignored" << endl;
+             }
+             else
+             {
+               // alias
+               repoStates[tag].first = it->second;
+             }
+           }
+           else
+             ERR << "Unknown attribute " << it->first << " ignored" << endl;
+         }
           else
             ERR << "Unknown attribute " << it->first << " ignored" << endl;
         }
 
+       if ( ! repoStates.empty() )
+       {
+         ServiceInfo::RepoStates data;
+         for ( const auto & el : repoStates )
+         {
+           if ( el.second.first.empty() )
+             ERR << "Missing alias for repo_" << el.first << "; ignore entry" << endl;
+           else
+             data[el.second.first] = el.second.second;
+         }
+         if ( ! data.empty() )
+           service.setRepoStates( std::move(data) );
+       }
+
         MIL << "Linking ServiceInfo with file " << file << endl;
         service.setFilepath(file);