Skip to content

Commit

Permalink
lib/config: Decouple VerifyConfiguration from Committer (syncthing#7939)
Browse files Browse the repository at this point in the history
... and remove 8/10 implementations, which were no-ops. This saves code
and time copying configurations.
  • Loading branch information
greatroar authored Nov 22, 2021
1 parent e2288fe commit bf89bff
Show file tree
Hide file tree
Showing 11 changed files with 20 additions and 40 deletions.
2 changes: 2 additions & 0 deletions lib/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ type service struct {
systemLog logger.Recorder
}

var _ config.Verifier = &service{}

type Service interface {
suture.Service
config.Committer
Expand Down
23 changes: 16 additions & 7 deletions lib/config/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ const (

var errTooManyModifications = errors.New("too many concurrent config modifications")

// The Committer interface is implemented by objects that need to know about
// or have a say in configuration changes.
// The Committer and Verifier interfaces are implemented by objects
// that need to know about or have a say in configuration changes.
//
// When the configuration is about to be changed, VerifyConfiguration() is
// called for each subscribing object, with the old and new configuration. A
// nil error is returned if the new configuration is acceptable (i.e. does not
// contain any errors that would prevent it from being a valid config).
// Otherwise an error describing the problem is returned.
// called for each subscribing object that implements it, with copies of the
// old and new configuration. A nil error is returned if the new configuration
// is acceptable (i.e., does not contain any errors that would prevent it from
// being a valid config). Otherwise an error describing the problem is returned.
//
// If any subscriber returns an error from VerifyConfiguration(), the
// configuration change is not committed and an error is returned to whoever
Expand All @@ -55,11 +55,16 @@ var errTooManyModifications = errors.New("too many concurrent config modificatio
// configuration (e.g. calling Wrapper.SetFolder), that are also acquired in any
// methods of the Committer interface.
type Committer interface {
VerifyConfiguration(from, to Configuration) error
CommitConfiguration(from, to Configuration) (handled bool)
String() string
}

// A Verifier can determine if a new configuration is acceptable.
// See the description for Committer, above.
type Verifier interface {
VerifyConfiguration(from, to Configuration) error
}

// Waiter allows to wait for the given config operation to complete.
type Waiter interface {
Wait()
Expand Down Expand Up @@ -300,6 +305,10 @@ func (w *wrapper) replaceLocked(to Configuration) (Waiter, error) {
}

for _, sub := range w.subs {
sub, ok := sub.(Verifier)
if !ok {
continue
}
l.Debugln(sub, "verifying configuration")
if err := sub.VerifyConfiguration(from.Copy(), to.Copy()); err != nil {
l.Debugln(sub, "rejected config:", err)
Expand Down
4 changes: 0 additions & 4 deletions lib/connections/limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,6 @@ func (lim *limiter) processDevicesConfigurationLocked(from, to config.Configurat
}
}

func (lim *limiter) VerifyConfiguration(from, to config.Configuration) error {
return nil
}

func (lim *limiter) CommitConfiguration(from, to config.Configuration) bool {
// to ensure atomic update of configuration
lim.mu.Lock()
Expand Down
4 changes: 0 additions & 4 deletions lib/connections/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -706,10 +706,6 @@ func (s *service) logListenAddressesChangedEvent(l ListenerAddresses) {
})
}

func (s *service) VerifyConfiguration(from, to config.Configuration) error {
return nil
}

func (s *service) CommitConfiguration(from, to config.Configuration) bool {
newDevices := make(map[protocol.DeviceID]bool, len(to.Devices))
for _, dev := range to.Devices {
Expand Down
4 changes: 0 additions & 4 deletions lib/discover/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,6 @@ func (m *manager) Cache() map[protocol.DeviceID]CacheEntry {
return res
}

func (m *manager) VerifyConfiguration(_, _ config.Configuration) error {
return nil
}

func (m *manager) CommitConfiguration(_, to config.Configuration) (handled bool) {
m.mut.Lock()
defer m.mut.Unlock()
Expand Down
2 changes: 2 additions & 0 deletions lib/model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ type model struct {
foldersRunning int32
}

var _ config.Verifier = &model{}

type folderFactory func(*model, *db.FileSet, *ignore.Matcher, config.FolderConfiguration, versioner.Versioner, events.Logger, *util.Semaphore) service

var (
Expand Down
5 changes: 0 additions & 5 deletions lib/model/progressemitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,6 @@ func (t *ProgressEmitter) computeProgressUpdates() []progressUpdate {
return progressUpdates
}

// VerifyConfiguration implements the config.Committer interface
func (t *ProgressEmitter) VerifyConfiguration(from, to config.Configuration) error {
return nil
}

// CommitConfiguration implements the config.Committer interface
func (t *ProgressEmitter) CommitConfiguration(_, to config.Configuration) bool {
t.mut.Lock()
Expand Down
4 changes: 0 additions & 4 deletions lib/nat/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ func NewService(id protocol.DeviceID, cfg config.Wrapper) *Service {
return s
}

func (s *Service) VerifyConfiguration(from, to config.Configuration) error {
return nil
}

func (s *Service) CommitConfiguration(from, to config.Configuration) bool {
s.mut.Lock()
if !s.enabled && to.Options.NATEnabled {
Expand Down
4 changes: 0 additions & 4 deletions lib/ur/failurereporting.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,6 @@ func (h *failureHandler) addReport(data FailureData, evTime time.Time) {
}
}

func (h *failureHandler) VerifyConfiguration(_, _ config.Configuration) error {
return nil
}

func (h *failureHandler) CommitConfiguration(from, to config.Configuration) bool {
if from.Options.CREnabled != to.Options.CREnabled || from.Options.CRURL != to.Options.CRURL {
h.optsChan <- to.Options
Expand Down
4 changes: 0 additions & 4 deletions lib/ur/usage_report.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,6 @@ func (s *Service) Serve(ctx context.Context) error {
}
}

func (s *Service) VerifyConfiguration(from, to config.Configuration) error {
return nil
}

func (s *Service) CommitConfiguration(from, to config.Configuration) bool {
if from.Options.URAccepted != to.Options.URAccepted || from.Options.URUniqueID != to.Options.URUniqueID || from.Options.URURL != to.Options.URURL {
select {
Expand Down
4 changes: 0 additions & 4 deletions lib/watchaggregator/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,6 @@ func (a *aggregator) String() string {
return fmt.Sprintf("aggregator/%s:", a.folderCfg.Description())
}

func (a *aggregator) VerifyConfiguration(from, to config.Configuration) error {
return nil
}

func (a *aggregator) CommitConfiguration(from, to config.Configuration) bool {
for _, folderCfg := range to.Folders {
if folderCfg.ID == a.folderID {
Expand Down

0 comments on commit bf89bff

Please sign in to comment.