Merge pull request #159 from joshdekock/middleware-refactor-string

Make client middleware use StringStore and remove ClientStore

server/store/client_store.go

@@ -1,49 +0,0 @@
-// Copyright 2016 The Chihaya Authors. All rights reserved.
-// Use of this source code is governed by the BSD 2-Clause license,
-// which can be found in the LICENSE file.
-
-package store
-
-import (
-	"fmt"
-	"github.com/chihaya/chihaya"
-)
-
-var clientStoreDrivers = make(map[string]ClientStoreDriver)
-
-// ClientStore represents an interface for manipulating clientIDs.
-type ClientStore interface {
-	CreateClient(clientID string) error
-	FindClient(peerID chihaya.PeerID) (bool, error)
-	DeleteClient(clientID string) error
-}
-
-// ClientStoreDriver represents an interface for creating a handle to the
-// storage of swarms.
-type ClientStoreDriver interface {
-	New(*DriverConfig) (ClientStore, error)
-}
-
-// RegisterClientStoreDriver makes a driver available by the provided name.
-//
-// If this function is called twice with the same name or if the driver is nil,
-// it panics.
-func RegisterClientStoreDriver(name string, driver ClientStoreDriver) {
-	if driver == nil {
-		panic("store: could not register nil ClientStoreDriver")
-	}
-	if _, dup := clientStoreDrivers[name]; dup {
-		panic("store: could not register duplicate ClientStoreDriver: " + name)
-	}
-	clientStoreDrivers[name] = driver
-}
-
-// OpenClientStore returns a ClientStore specified by a configuration.
-func OpenClientStore(cfg *DriverConfig) (ClientStore, error) {
-	driver, ok := clientStoreDrivers[cfg.Name]
-	if !ok {
-		return nil, fmt.Errorf("store: unknown ClientStoreDriver %q (forgotten import?)", cfg)
-	}
-
-	return driver.New(cfg)
-}

server/store/memory/client_store.go

@@ -1,60 +0,0 @@
-// Copyright 2016 The Chihaya Authors. All rights reserved.
-// Use of this source code is governed by the BSD 2-Clause license,
-// which can be found in the LICENSE file.
-
-package memory
-
-import (
-	"sync"
-
-	"github.com/chihaya/chihaya"
-	"github.com/chihaya/chihaya/pkg/clientid"
-	"github.com/chihaya/chihaya/server/store"
-)
-
-func init() {
-	store.RegisterClientStoreDriver("memory", &clientStoreDriver{})
-}
-
-type clientStoreDriver struct{}
-
-func (d *clientStoreDriver) New(_ *store.DriverConfig) (store.ClientStore, error) {
-	return &clientStore{
-		clientIDs: make(map[string]struct{}),
-	}, nil
-}
-
-type clientStore struct {
-	clientIDs map[string]struct{}
-	sync.RWMutex
-}
-
-var _ store.ClientStore = &clientStore{}
-
-func (s *clientStore) CreateClient(clientID string) error {
-	s.Lock()
-	defer s.Unlock()
-
-	s.clientIDs[clientID] = struct{}{}
-
-	return nil
-}
-
-func (s *clientStore) FindClient(peerID chihaya.PeerID) (bool, error) {
-	clientID := clientid.New(string(peerID))
-	s.RLock()
-	defer s.RUnlock()
-
-	_, ok := s.clientIDs[clientID]
-
-	return ok, nil
-}
-
-func (s *clientStore) DeleteClient(clientID string) error {
-	s.Lock()
-	defer s.Unlock()
-
-	delete(s.clientIDs, clientID)
-
-	return nil
-}

server/store/middleware/client/README.md

@@ -4,22 +4,22 @@ This package provides the announce middlewares `client_whitelist` and `client_bl
 
 ### `client_blacklist`
 
-The `client_blacklist` middleware uses all clientIDs stored in the `ClientStore` to blacklist, i.e. block announces.
+The `client_blacklist` middleware uses all clientIDs stored in the `StringStore` to blacklist, i.e. block announces.
 
-The clientID part of the peerID of an announce is matched against the `ClientStore`, if it's contained within the `ClientStore`, the announce is aborted.
+The clientID part of the peerID of an announce is matched against the `StringStore`, if it's contained within the `StringStore`, the announce is aborted.
 
 ### `client_whitelist`
 
-The `client_whitelist` middleware uses all clientIDs stored in the `ClientStore` to whitelist, i.e. allow announces.
+The `client_whitelist` middleware uses all clientIDs stored in the `StringStore` to whitelist, i.e. allow announces.
 
-The clientID part of the peerID of an announce is matched against the `ClientStore`, if it's _not_ contained within the `ClientStore`, the announce is aborted.
+The clientID part of the peerID of an announce is matched against the `StringStore`, if it's _not_ contained within the `StringStore`, the announce is aborted.
 
 ### Important things to notice
 
 Both middlewares operate on announce requests only.
 
-Both middlewares use the same `ClientStore`.
+Both middlewares use the same `StringStore`.
 It is therefore not advised to have both the `client_blacklist` and the `client_whitelist` middleware running.
-(If you add clientID to the `ClientStore`, it will be used for blacklisting and whitelisting.
+(If you add clientID to the `StringStore`, it will be used for blacklisting and whitelisting.
 If your store contains no clientIDs, no announces will be blocked by the blacklist, but all announces will be blocked by the whitelist.
 If your store contains all clientIDs, no announces will be blocked by the whitelist, but all announces will be blocked by the blacklist.)

server/store/middleware/client/blacklist.go

@@ -6,6 +6,7 @@ package client
 
 import (
 	"github.com/chihaya/chihaya"
+	"github.com/chihaya/chihaya/pkg/clientid"
 	"github.com/chihaya/chihaya/server/store"
 	"github.com/chihaya/chihaya/tracker"
 )
@@ -14,20 +15,19 @@ func init() {
 	tracker.RegisterAnnounceMiddleware("client_blacklist", blacklistAnnounceClient)
 }
 
-// ErrBlockedClient is returned by an announce middleware if the announcing
+// ErrBlacklistedClient is returned by an announce middleware if the announcing
-// Client is disallowed.
+// Client is blacklisted.
-var ErrBlockedClient = tracker.ClientError("disallowed client")
+var ErrBlacklistedClient = tracker.ClientError("client blacklisted")
 
 // blacklistAnnounceClient provides a middleware that only allows Clients to
-// announce that are not stored in a ClientStore.
+// announce that are not stored in the StringStore.
 func blacklistAnnounceClient(next tracker.AnnounceHandler) tracker.AnnounceHandler {
 	return func(cfg *chihaya.TrackerConfig, req *chihaya.AnnounceRequest, resp *chihaya.AnnounceResponse) error {
-		blacklisted, err := store.MustGetStore().FindClient(req.PeerID)
+		blacklisted, err := store.MustGetStore().HasString(PrefixClient + clientid.New(string(req.PeerID)))
-
 		if err != nil {
 			return err
 		} else if blacklisted {
-			return ErrBlockedClient
+			return ErrBlacklistedClient
 		}
 		return next(cfg, req, resp)
 	}

server/store/middleware/client/whitelist.go

@@ -6,6 +6,7 @@ package client
 
 import (
 	"github.com/chihaya/chihaya"
+	"github.com/chihaya/chihaya/pkg/clientid"
 	"github.com/chihaya/chihaya/server/store"
 	"github.com/chihaya/chihaya/tracker"
 )
@@ -14,16 +15,22 @@ func init() {
 	tracker.RegisterAnnounceMiddleware("client_whitelist", whitelistAnnounceClient)
 }
 
+// PrefixClient is the prefix to be used for client peer IDs.
+const PrefixClient = "c-"
+
+// ErrNotWhitelistedClient is returned by an announce middleware if the
+// announcing Client is not whitelisted.
+var ErrNotWhitelistedClient = tracker.ClientError("client not whitelisted")
+
 // whitelistAnnounceClient provides a middleware that only allows Clients to
-// announce that are stored in a ClientStore.
+// announce that are stored in the StringStore.
 func whitelistAnnounceClient(next tracker.AnnounceHandler) tracker.AnnounceHandler {
 	return func(cfg *chihaya.TrackerConfig, req *chihaya.AnnounceRequest, resp *chihaya.AnnounceResponse) error {
-		whitelisted, err := store.MustGetStore().FindClient(req.PeerID)
+		whitelisted, err := store.MustGetStore().HasString(PrefixClient + clientid.New(string(req.PeerID)))
-
 		if err != nil {
 			return err
 		} else if !whitelisted {
-			return ErrBlockedClient
+			return ErrNotWhitelistedClient
 		}
 		return next(cfg, req, resp)
 	}

server/store/middleware/infohash/README.md

@@ -7,11 +7,11 @@ It also provides the configurable scrape middleware `infohash_blacklist` and `in
 
 #### For Announces
 
-The `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `store.PrefixInfohash` prefix to blacklist, i.e. block announces.
+The `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `PrefixInfohash` prefix to blacklist, i.e. block announces.
 
 #### For Scrapes
 
-The configurable `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `store.PrefixInfohash` prefix to blacklist scrape requests.
+The configurable `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `PrefixInfohash` prefix to blacklist scrape requests.
 
 The scrape middleware has two modes of operation: _Block_ and _Filter_.
 
@@ -25,11 +25,11 @@ See the configuration section for information about how to configure the scrape
 
 #### For Announces
 
-The `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `store.PrefixInfohash` prefix to whitelist, i.e. allow announces.
+The `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `PrefixInfohash` prefix to whitelist, i.e. allow announces.
 
 #### For Scrapes
 
-The configurable `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `store.PrefixInfohash` prefix to whitelist scrape requests.
+The configurable `infohash_blacklist` middleware uses all infohashes stored in the `StringStore` with the `PrefixInfohash` prefix to whitelist scrape requests.
 
 The scrape middleware has two modes of operation: _Block_ and _Filter_.
 

server/store/middleware/infohash/blacklist.go

@@ -23,7 +23,7 @@ var ErrBlockedInfohash = tracker.ClientError("disallowed infohash")
 // for infohashes that are not stored in a StringStore.
 func blacklistAnnounceInfohash(next tracker.AnnounceHandler) tracker.AnnounceHandler {
 	return func(cfg *chihaya.TrackerConfig, req *chihaya.AnnounceRequest, resp *chihaya.AnnounceResponse) (err error) {
-		blacklisted, err := store.MustGetStore().HasString(store.PrefixInfohash + string(req.InfoHash))
+		blacklisted, err := store.MustGetStore().HasString(PrefixInfohash + string(req.InfoHash))
 		if err != nil {
 			return err
 		} else if blacklisted {
@@ -67,7 +67,7 @@ func blacklistFilterScrape(next tracker.ScrapeHandler) tracker.ScrapeHandler {
 		infohashes := req.InfoHashes
 
 		for i, ih := range infohashes {
-			blacklisted, err = storage.HasString(store.PrefixInfohash + string(ih))
+			blacklisted, err = storage.HasString(PrefixInfohash + string(ih))
 
 			if err != nil {
 				return err
@@ -87,7 +87,7 @@ func blacklistBlockScrape(next tracker.ScrapeHandler) tracker.ScrapeHandler {
 		storage := store.MustGetStore()
 
 		for _, ih := range req.InfoHashes {
-			blacklisted, err = storage.HasString(store.PrefixInfohash + string(ih))
+			blacklisted, err = storage.HasString(PrefixInfohash + string(ih))
 
 			if err != nil {
 				return err

server/store/middleware/infohash/blacklist_test.go

@@ -27,9 +27,6 @@ func TestASetUp(t *testing.T) {
 			StringStore: store.DriverConfig{
 				Name: "memory",
 			},
-			ClientStore: store.DriverConfig{
-				Name: "memory",
-			},
 			IPStore: store.DriverConfig{
 				Name: "memory",
 			},
@@ -44,7 +41,7 @@ func TestASetUp(t *testing.T) {
 	assert.Nil(t, err)
 	srv.Start()
 
-	store.MustGetStore().PutString(store.PrefixInfohash + "abc")
+	store.MustGetStore().PutString(PrefixInfohash + "abc")
 }
 
 func TestBlacklistAnnounceMiddleware(t *testing.T) {

server/store/middleware/infohash/whitelist.go

@@ -15,11 +15,14 @@ func init() {
 	tracker.RegisterScrapeMiddlewareConstructor("infohash_whitelist", whitelistScrapeInfohash)
 }
 
+// PrefixInfohash is the prefix to be used for infohashes.
+const PrefixInfohash = "ih-"
+
 // whitelistAnnounceInfohash provides a middleware that only allows announces
 // for infohashes that are not stored in a StringStore
 func whitelistAnnounceInfohash(next tracker.AnnounceHandler) tracker.AnnounceHandler {
 	return func(cfg *chihaya.TrackerConfig, req *chihaya.AnnounceRequest, resp *chihaya.AnnounceResponse) (err error) {
-		whitelisted, err := store.MustGetStore().HasString(store.PrefixInfohash + string(req.InfoHash))
+		whitelisted, err := store.MustGetStore().HasString(PrefixInfohash + string(req.InfoHash))
 
 		if err != nil {
 			return err
@@ -63,7 +66,7 @@ func whitelistFilterScrape(next tracker.ScrapeHandler) tracker.ScrapeHandler {
 		infohashes := req.InfoHashes
 
 		for i, ih := range infohashes {
-			whitelisted, err = storage.HasString(store.PrefixInfohash + string(ih))
+			whitelisted, err = storage.HasString(PrefixInfohash + string(ih))
 
 			if err != nil {
 				return err
@@ -83,7 +86,7 @@ func whitelistBlockScrape(next tracker.ScrapeHandler) tracker.ScrapeHandler {
 		storage := store.MustGetStore()
 
 		for _, ih := range req.InfoHashes {
-			whitelisted, err = storage.HasString(store.PrefixInfohash + string(ih))
+			whitelisted, err = storage.HasString(PrefixInfohash + string(ih))
 
 			if err != nil {
 				return err

server/store/store.go

@@ -30,11 +30,6 @@ func constructor(srvcfg *chihaya.ServerConfig, tkr *tracker.Tracker) (server.Ser
 			return nil, errors.New("store: invalid store config: " + err.Error())
 		}
 
-		cs, err := OpenClientStore(&cfg.ClientStore)
-		if err != nil {
-			return nil, err
-		}
-
 		ps, err := OpenPeerStore(&cfg.PeerStore)
 		if err != nil {
 			return nil, err
@@ -54,7 +49,6 @@ func constructor(srvcfg *chihaya.ServerConfig, tkr *tracker.Tracker) (server.Ser
 			cfg:         cfg,
 			tkr:         tkr,
 			shutdown:    make(chan struct{}),
-			ClientStore: cs,
 			PeerStore:   ps,
 			IPStore:     ips,
 			StringStore: ss,
@@ -69,7 +63,6 @@ type Config struct {
 	ReadTimeout    time.Duration `yaml:"read_timeout"`
 	WriteTimeout   time.Duration `yaml:"write_timeout"`
 	GCAfter        time.Duration `yaml:"gc_after"`
-	ClientStore    DriverConfig  `yaml:"client_store"`
 	PeerStore      DriverConfig  `yaml:"peer_store"`
 	IPStore        DriverConfig  `yaml:"ip_store"`
 	StringStore    DriverConfig  `yaml:"string_store"`
@@ -113,7 +106,6 @@ type Store struct {
 	wg       sync.WaitGroup
 
 	PeerStore
-	ClientStore
 	IPStore
 	StringStore
 }

server/store/string_store.go

@@ -6,9 +6,6 @@ package store
 
 import "fmt"
 
-// PrefixInfohash is the prefix to be used for infohashes.
-const PrefixInfohash = "ih-"
-
 var stringStoreDrivers = make(map[string]StringStoreDriver)
 
 // StringStore represents an interface for manipulating strings.