From 2dd810d2ee6134b63bc6722054a509f57a457fc5 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Tue, 24 Sep 2013 23:10:48 -0400 Subject: [PATCH] Fixes & format changes from pull request comments --- cache/redis/redis.go | 161 +++++++++++++------------------ cache/redis/redis_bench_test.go | 58 ++++++------ cache/redis/redis_test.go | 62 +++--------- cache/redis/tx_test.go | 163 ++++++++++++++++---------------- models/models.go | 4 +- 5 files changed, 195 insertions(+), 253 deletions(-) diff --git a/cache/redis/redis.go b/cache/redis/redis.go index b178363..af62bd7 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -31,11 +31,11 @@ var ( ErrCreatePeer = errors.New("redis: Incorrect reply length for peer") ErrMarkActive = errors.New("redis: Torrent doesn't exist") - SeederPrefix = "seeders:" - LeecherPrefix = "leechers:" - TorrentPrefix = "torrent:" - UserPrefix = "user:" - PeerPrefix = "peer:" + SeedersPrefix = "seeders:" + LeechersPrefix = "leechers:" + TorrentPrefix = "torrent:" + UserPrefix = "user:" + PeerPrefix = "peer:" ) type driver struct{} @@ -82,10 +82,7 @@ func (p *Pool) Get() (cache.Tx, error) { done: false, Conn: p.pool.Get(), } - // Test valid connection before returning - _, err := retTx.Do("PING") - - return retTx, err + return retTx, nil } type Tx struct { @@ -106,76 +103,47 @@ func createUser(userVals []string) (*models.User, error) { if len(userVals) != 7 { return nil, ErrCreateUser } - ID, err := strconv.ParseUint(userVals[0], 10, 64) - if err != nil { - return nil, err + var user models.User + convErrors := make([]error, 7) + user.ID, convErrors[0] = strconv.ParseUint(userVals[0], 10, 64) + user.Passkey = userVals[1] + user.UpMultiplier, convErrors[2] = strconv.ParseFloat(userVals[2], 64) + user.DownMultiplier, convErrors[3] = strconv.ParseFloat(userVals[3], 64) + user.Slots, convErrors[4] = strconv.ParseInt(userVals[4], 10, 64) + user.SlotsUsed, convErrors[5] = strconv.ParseInt(userVals[5], 10, 64) + user.Snatches, convErrors[6] = strconv.ParseUint(userVals[6], 10, 64) + + for i := 0; i < 7; i++ { + if convErrors[i] != nil { + return nil, convErrors[i] + } } - Passkey := userVals[1] - UpMultiplier, err := strconv.ParseFloat(userVals[2], 64) - if err != nil { - return nil, err - } - DownMultiplier, err := strconv.ParseFloat(userVals[3], 64) - if err != nil { - return nil, err - } - Slots, err := strconv.ParseInt(userVals[4], 10, 64) - if err != nil { - return nil, err - } - SlotsUsed, err := strconv.ParseInt(userVals[5], 10, 64) - if err != nil { - return nil, err - } - Snatches, err := strconv.ParseUint(userVals[6], 10, 64) - if err != nil { - return nil, err - } - return &models.User{ID: ID, Passkey: Passkey, UpMultiplier: UpMultiplier, - DownMultiplier: DownMultiplier, Slots: Slots, SlotsUsed: SlotsUsed, Snatches: uint(Snatches)}, nil + return &user, nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { if len(torrentVals) != 7 { return nil, ErrCreateTorrent } - ID, err := strconv.ParseUint(torrentVals[0], 10, 64) - if err != nil { - return nil, err - } - Infohash := torrentVals[1] - Active, err := strconv.ParseBool(torrentVals[2]) - if err != nil { - return nil, err - } - Snatches, err := strconv.ParseUint(torrentVals[3], 10, 32) - if err != nil { - return nil, err - } - UpMultiplier, err := strconv.ParseFloat(torrentVals[4], 64) - if err != nil { - return nil, err - } - DownMultiplier, err := strconv.ParseFloat(torrentVals[5], 64) - if err != nil { - return nil, err - } - LastAction, err := strconv.ParseInt(torrentVals[6], 10, 64) - if err != nil { - return nil, err - } - seeders, err := tx.getPeers(ID, SeederPrefix) - if err != nil { - return nil, err - } - leechers, err := tx.getPeers(ID, LeecherPrefix) - if err != nil { - return nil, err - } + var torrent models.Torrent + convErrors := make([]error, 9) + torrent.ID, convErrors[0] = strconv.ParseUint(torrentVals[0], 10, 64) + torrent.Infohash = torrentVals[1] + torrent.Active, convErrors[2] = strconv.ParseBool(torrentVals[2]) + torrent.Snatches, convErrors[3] = strconv.ParseUint(torrentVals[3], 10, 32) + torrent.UpMultiplier, convErrors[4] = strconv.ParseFloat(torrentVals[4], 64) + torrent.DownMultiplier, convErrors[5] = strconv.ParseFloat(torrentVals[5], 64) + torrent.LastAction, convErrors[6] = strconv.ParseInt(torrentVals[6], 10, 64) + torrent.Seeders, convErrors[7] = tx.getPeers(torrent.ID, SeedersPrefix) + torrent.Leechers, convErrors[8] = tx.getPeers(torrent.ID, LeechersPrefix) - return &models.Torrent{ID: ID, Infohash: Infohash, Active: Active, Seeders: seeders, Leechers: leechers, - Snatches: uint(Snatches), UpMultiplier: UpMultiplier, DownMultiplier: DownMultiplier, LastAction: LastAction}, nil + for i := 0; i < 9; i++ { + if convErrors[i] != nil { + return nil, convErrors[i] + } + } + return &torrent, nil } // The peer hashkey relies on the combination of peerID, userID, and torrentID being unique @@ -210,7 +178,7 @@ func (tx *Tx) removePeer(peer *models.Peer, peerTypePrefix string) error { return nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { hashKey := tx.conf.Prefix + getPeerHashKey(&peer) @@ -218,7 +186,7 @@ func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTy if err != nil { return err } - delete(peers, peer.ID) + delete(peers, models.PeerMapKey(&peer)) } // Will only delete the set if all the peer deletions were successful setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) @@ -238,7 +206,7 @@ func getPeerSetKey(typePrefix string, peer *models.Peer) string { return typePrefix + strconv.FormatUint(peer.TorrentID, 36) } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) addPeers(peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { setKey := tx.conf.Prefix + getPeerSetKey(peerTypePrefix, &peer) @@ -291,7 +259,7 @@ func createPeer(peerVals []string) (*models.Peer, error) { } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[string]models.Peer, err error) { peers = make(map[string]models.Peer) setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) @@ -318,7 +286,7 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin return } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) AddTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("HMSET", hashkey, @@ -333,18 +301,18 @@ func (tx *Tx) AddTorrent(t *models.Torrent) error { return err } - err = tx.addPeers(t.Seeders, SeederPrefix) + err = tx.addPeers(t.Seeders, SeedersPrefix) if err != nil { return err } - err = tx.addPeers(t.Leechers, LeecherPrefix) + err = tx.addPeers(t.Leechers, LeechersPrefix) if err != nil { return err } return nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) RemoveTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("DEL", hashkey) @@ -352,11 +320,11 @@ func (tx *Tx) RemoveTorrent(t *models.Torrent) error { return err } // Remove seeders and leechers as well - err = tx.removePeers(t.ID, t.Seeders, SeederPrefix) + err = tx.removePeers(t.ID, t.Seeders, SeedersPrefix) if err != nil { return err } - err = tx.removePeers(t.ID, t.Leechers, LeecherPrefix) + err = tx.removePeers(t.ID, t.Leechers, LeechersPrefix) if err != nil { return err } @@ -403,7 +371,7 @@ func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { return foundUser, true, nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { hashkey := tx.conf.Prefix + TorrentPrefix + infohash torrentStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) @@ -437,7 +405,7 @@ func (tx *Tx) UnWhitelistClient(peerID string) error { return err } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { torrentKey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash @@ -445,14 +413,14 @@ func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { if err != nil { return err } - torrent.Snatches = uint(snatchCount) + torrent.Snatches = uint64(snatchCount) userKey := tx.conf.Prefix + UserPrefix + user.Passkey snatchCount, err = redis.Int(tx.Do("HINCRBY", userKey, "snatches", 1)) if err != nil { return err } - user.Snatches = uint(snatchCount) + user.Snatches = uint64(snatchCount) return nil } @@ -470,7 +438,7 @@ func (tx *Tx) MarkActive(torrent *models.Torrent) error { return nil } -func (tx *Tx) MarkInActive(torrent *models.Torrent) error { +func (tx *Tx) MarkInactive(torrent *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash activeExists, err := redis.Int(tx.Do("HSET", hashkey, "active", false)) if err != nil { @@ -479,14 +447,19 @@ func (tx *Tx) MarkInActive(torrent *models.Torrent) error { torrent.Active = false // HSET returns 1 if hash didn't exist before if activeExists == 1 { + // Clean-up incomplete torrent + _, err = tx.Do("DEL", hashkey) + if err != nil { + return err + } return ErrMarkActive } return nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { - setKey := tx.conf.Prefix + LeecherPrefix + strconv.FormatUint(torrent.ID, 36) + setKey := tx.conf.Prefix + LeechersPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) if err != nil { return err @@ -514,7 +487,7 @@ func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { } func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { - err := tx.removePeer(p, LeecherPrefix) + err := tx.removePeer(p, LeechersPrefix) if err != nil { return err } @@ -524,8 +497,8 @@ func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error { torrentIdKey := strconv.FormatUint(torrent.ID, 36) - seederSetKey := tx.conf.Prefix + SeederPrefix + torrentIdKey - leecherSetKey := tx.conf.Prefix + LeecherPrefix + torrentIdKey + seederSetKey := tx.conf.Prefix + SeedersPrefix + torrentIdKey + leecherSetKey := tx.conf.Prefix + LeechersPrefix + torrentIdKey _, err := tx.Do("SMOVE", leecherSetKey, seederSetKey, getPeerHashKey(peer)) if err != nil { @@ -538,9 +511,9 @@ func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error return err } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { - setKey := tx.conf.Prefix + SeederPrefix + strconv.FormatUint(torrent.ID, 36) + setKey := tx.conf.Prefix + SeedersPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) if err != nil { return err @@ -566,7 +539,7 @@ func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { } func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { - err := tx.removePeer(p, SeederPrefix) + err := tx.removePeer(p, SeedersPrefix) if err != nil { return err } diff --git a/cache/redis/redis_bench_test.go b/cache/redis/redis_bench_test.go index 61bd0da..59cfe00 100644 --- a/cache/redis/redis_bench_test.go +++ b/cache/redis/redis_bench_test.go @@ -15,13 +15,13 @@ func BenchmarkSuccessfulFindUser(b *testing.B) { b.StopTimer() tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if !found { b.Error("user not found", testUser) } @@ -40,7 +40,7 @@ func BenchmarkFailedFindUser(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { _, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if found { b.Error("user not found", testUser) } @@ -52,12 +52,12 @@ func BenchmarkSuccessfulFindTorrent(b *testing.B) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if !found { b.Error("torrent not found", testTorrent) } @@ -76,7 +76,7 @@ func BenchmarkFailFindTorrent(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if found { b.Error("torrent found", foundTorrent) } @@ -87,12 +87,12 @@ func BenchmarkSuccessfulClientWhitelisted(b *testing.B) { b.StopTimer() tx := createTestTx() testPeerID := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID)) + panicOnErr(tx.WhitelistClient(testPeerID)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { found, err := tx.ClientWhitelisted(testPeerID) - panicErrNil(err) + panicOnErr(err) if !found { b.Error("peerID not found", testPeerID) } @@ -107,7 +107,7 @@ func BenchmarkFailClientWhitelisted(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { found, err := tx.ClientWhitelisted(testPeerID2) - panicErrNil(err) + panicOnErr(err) if found { b.Error("peerID found", testPeerID2) } @@ -119,12 +119,12 @@ func BenchmarkRecordSnatch(b *testing.B) { tx := createTestTx() testTorrent := createTestTorrent() testUser := createTestUser() - panicErrNil(tx.AddTorrent(testTorrent)) - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddUser(testUser)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { - panicErrNil(tx.RecordSnatch(testUser, testTorrent)) + panicOnErr(tx.RecordSnatch(testUser, testTorrent)) } } @@ -133,11 +133,11 @@ func BenchmarkMarkActive(b *testing.B) { tx := createTestTx() testTorrent := createTestTorrent() testTorrent.Active = false - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { - panicErrNil(tx.MarkActive(testTorrent)) + panicOnErr(tx.MarkActive(testTorrent)) } } @@ -145,7 +145,7 @@ func BenchmarkAddSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { @@ -153,7 +153,7 @@ func BenchmarkAddSeeder(b *testing.B) { testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) b.StartTimer() - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) } } @@ -161,7 +161,7 @@ func BenchmarkRemoveSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) b.StartTimer() @@ -170,7 +170,7 @@ func BenchmarkRemoveSeeder(b *testing.B) { tx.AddSeeder(testTorrent, testSeeder) b.StartTimer() - panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + panicOnErr(tx.RemoveSeeder(testTorrent, testSeeder)) } } @@ -178,9 +178,9 @@ func BenchmarkSetSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) r := rand.New(rand.NewSource(time.Now().UnixNano())) b.StartTimer() @@ -197,11 +197,11 @@ func BenchmarkIncrementSlots(b *testing.B) { b.StopTimer() tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { - panicErrNil(tx.IncrementSlots(testUser)) + panicOnErr(tx.IncrementSlots(testUser)) } } @@ -209,17 +209,17 @@ func BenchmarkLeecherFinished(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { b.StopTimer() testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) testLeecher.Left = 0 b.StartTimer() - panicErrNil(tx.LeecherFinished(testTorrent, testLeecher)) + panicOnErr(tx.LeecherFinished(testTorrent, testLeecher)) } } @@ -228,18 +228,18 @@ func BenchmarkRemoveLeecherAddSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { b.StopTimer() testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) testLeecher.Left = 0 b.StartTimer() - panicErrNil(tx.RemoveLeecher(testTorrent, testLeecher)) - panicErrNil(tx.AddSeeder(testTorrent, testLeecher)) + panicOnErr(tx.RemoveLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddSeeder(testTorrent, testLeecher)) } } diff --git a/cache/redis/redis_test.go b/cache/redis/redis_test.go index d670bb0..f907a0d 100644 --- a/cache/redis/redis_test.go +++ b/cache/redis/redis_test.go @@ -76,7 +76,7 @@ func createTestPasskey() string { return string(uuid) } -func panicErrNil(err error) { +func panicOnErr(err error) { if err != nil { fmt.Println(err) panic(err) @@ -86,7 +86,7 @@ func panicErrNil(err error) { func createTestRedisTx() *Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) conf := &testConfig.Cache - panicErrNil(err) + panicOnErr(err) testPool := &Pool{ conf: conf, @@ -103,11 +103,11 @@ func createTestRedisTx() *Tx { done: false, Conn: testPool.pool.Get(), } - panicErrNil(err) + panicOnErr(err) // Test connection before returning _, err = txObj.Do("PING") - panicErrNil(err) + panicOnErr(err) return txObj } @@ -144,53 +144,18 @@ func createTestTorrent() *models.Torrent { return &testTorrent } -func comparePeers(lhPeers map[string]models.Peer, rhPeers map[string]models.Peer) bool { - if len(lhPeers) != len(rhPeers) { - return false - } - for rhKey, rhValue := range rhPeers { - lhValue, lhExists := lhPeers[rhKey] - if !lhExists || lhValue != rhValue { - return false - } - } - for lhKey, lhValue := range lhPeers { - rhValue, rhExists := rhPeers[lhKey] - if !rhExists || rhValue != lhValue { - return false - } - } - return true -} - -func torrentsEqual(lhTorrent *models.Torrent, rhTorrent *models.Torrent) bool { - fieldsEqual := lhTorrent.Infohash == rhTorrent.Infohash && - lhTorrent.ID == rhTorrent.ID && - lhTorrent.Active == rhTorrent.Active && - lhTorrent.Snatches == rhTorrent.Snatches && - lhTorrent.UpMultiplier == rhTorrent.UpMultiplier && - lhTorrent.DownMultiplier == rhTorrent.DownMultiplier && - lhTorrent.LastAction == rhTorrent.LastAction - - if !fieldsEqual { - return false - } - - return comparePeers(lhTorrent.Seeders, rhTorrent.Seeders) && comparePeers(lhTorrent.Leechers, rhTorrent.Leechers) -} - func TestValidPeers(t *testing.T) { testTx := createTestRedisTx() testTorrentID := createTestTorrentID() testPeers := createTestPeers(testTorrentID, 3) - panicErrNil(testTx.addPeers(testPeers, "test:")) + panicOnErr(testTx.addPeers(testPeers, "test:")) peerMap, err := testTx.getPeers(testTorrentID, "test:") - panicErrNil(err) + panicOnErr(err) if len(peerMap) != len(testPeers) { t.Error("Num Peers not equal ", len(peerMap), len(testPeers)) } - panicErrNil(testTx.removePeers(testTorrentID, testPeers, "test:")) + panicOnErr(testTx.removePeers(testTorrentID, testPeers, "test:")) } func TestInvalidPeers(t *testing.T) { @@ -200,17 +165,20 @@ func TestInvalidPeers(t *testing.T) { tempPeer := createTestPeer(createTestUserID(), testTorrentID) testPeers[models.PeerMapKey(tempPeer)] = *tempPeer - panicErrNil(testTx.addPeers(testPeers, "test:")) + panicOnErr(testTx.addPeers(testPeers, "test:")) // Imitate a peer being removed during get hashKey := testTx.conf.Prefix + getPeerHashKey(tempPeer) _, err := testTx.Do("DEL", hashKey) - panicErrNil(err) + panicOnErr(err) peerMap, err := testTx.getPeers(testTorrentID, "test:") - panicErrNil(err) + panicOnErr(err) // Expect 1 less peer due to delete if len(peerMap) != len(testPeers)-1 { - t.Error("Num Peers not equal ", len(peerMap), len(testPeers)) + t.Error("Num Peers not equal ", len(peerMap), len(testPeers)-1) + } + panicOnErr(testTx.removePeers(testTorrentID, testPeers, "test:")) + if len(testPeers) != 0 { + t.Errorf("All peers not removed, %d peers remain!", len(testPeers)) } - panicErrNil(testTx.removePeers(testTorrentID, testPeers, "test:")) } diff --git a/cache/redis/tx_test.go b/cache/redis/tx_test.go index f107f1e..4d39b47 100644 --- a/cache/redis/tx_test.go +++ b/cache/redis/tx_test.go @@ -7,6 +7,7 @@ package redis import ( "math/rand" "os" + "reflect" "testing" "time" @@ -17,14 +18,14 @@ import ( func createTestTx() cache.Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) - panicErrNil(err) + panicOnErr(err) conf := &testConfig.Cache testPool, err := cache.Open(conf) - panicErrNil(err) + panicOnErr(err) txObj, err := testPool.Get() - panicErrNil(err) + panicOnErr(err) return txObj } @@ -33,9 +34,9 @@ func TestFindUserSuccess(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("user not found", testUser) } @@ -49,7 +50,7 @@ func TestFindUserFail(t *testing.T) { testUser := createTestUser() foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if found { t.Error("user found", foundUser) } @@ -59,11 +60,11 @@ func TestRemoveUser(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) err := tx.RemoveUser(testUser) - panicErrNil(err) + panicOnErr(err) foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if found { t.Error("removed user found", foundUser) } @@ -72,14 +73,14 @@ func TestRemoveUser(t *testing.T) { func TestFindTorrentSuccess(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("torrent not found", testTorrent) } - if !torrentsEqual(foundTorrent, testTorrent) { + if !reflect.DeepEqual(foundTorrent, testTorrent) { t.Error("found torrent mismatch", foundTorrent, testTorrent) } } @@ -89,7 +90,7 @@ func TestFindTorrentFail(t *testing.T) { testTorrent := createTestTorrent() foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if found { t.Error("torrent found", foundTorrent) } @@ -98,11 +99,11 @@ func TestFindTorrentFail(t *testing.T) { func TestRemoveTorrent(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) - panicErrNil(tx.RemoveTorrent(testTorrent)) + panicOnErr(tx.RemoveTorrent(testTorrent)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if found { t.Error("removed torrent found", foundTorrent) } @@ -112,9 +113,9 @@ func TestClientWhitelistSuccess(t *testing.T) { tx := createTestTx() testPeerID := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID)) + panicOnErr(tx.WhitelistClient(testPeerID)) found, err := tx.ClientWhitelisted(testPeerID) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("peerID not found", testPeerID) } @@ -125,7 +126,7 @@ func TestClientWhitelistFail(t *testing.T) { testPeerID2 := "TIX0192" found, err := tx.ClientWhitelisted(testPeerID2) - panicErrNil(err) + panicOnErr(err) if found { t.Error("peerID found", testPeerID2) } @@ -135,18 +136,18 @@ func TestRecordSnatch(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() testUser := createTestUser() - panicErrNil(tx.AddTorrent(testTorrent)) - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddUser(testUser)) userSnatches := testUser.Snatches torrentSnatches := testTorrent.Snatches - panicErrNil(tx.RecordSnatch(testUser, testTorrent)) + panicOnErr(tx.RecordSnatch(testUser, testTorrent)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundUser, _, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if testUser.Snatches != userSnatches+1 { t.Error("snatch not recorded to local user", testUser.Snatches, userSnatches+1) @@ -166,11 +167,11 @@ func TestMarkActive(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() testTorrent.Active = false - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) - panicErrNil(tx.MarkActive(testTorrent)) + panicOnErr(tx.MarkActive(testTorrent)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if foundTorrent.Active != true { t.Error("cached torrent not activated") @@ -183,11 +184,11 @@ func TestMarkActive(t *testing.T) { func TestClientWhitelistRemove(t *testing.T) { tx := createTestTx() testPeerID := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID)) - panicErrNil(tx.UnWhitelistClient(testPeerID)) + panicOnErr(tx.WhitelistClient(testPeerID)) + panicOnErr(tx.UnWhitelistClient(testPeerID)) found, err := tx.ClientWhitelisted(testPeerID) - panicErrNil(err) + panicOnErr(err) if found { t.Error("removed peerID found", testPeerID) } @@ -196,12 +197,12 @@ func TestClientWhitelistRemove(t *testing.T) { func TestAddSeeder(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, found := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if found && foundSeeder != *testSeeder { t.Error("seeder not added to cache", testSeeder) @@ -215,12 +216,12 @@ func TestAddSeeder(t *testing.T) { func TestAddLeecher(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if found && foundLeecher != *testLeecher { t.Error("leecher not added to cache", testLeecher) @@ -234,18 +235,18 @@ func TestAddLeecher(t *testing.T) { func TestRemoveSeeder(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) - panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + panicOnErr(tx.RemoveSeeder(testTorrent, testSeeder)) foundSeeder, found := testTorrent.Seeders[models.PeerMapKey(testSeeder)] if found || foundSeeder == *testSeeder { t.Error("seeder not removed from local", foundSeeder) } foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, found = foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if found || foundSeeder == *testSeeder { t.Error("seeder not removed from cache", foundSeeder, *testSeeder) @@ -255,13 +256,13 @@ func TestRemoveSeeder(t *testing.T) { func TestRemoveLeecher(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) - panicErrNil(tx.RemoveLeecher(testTorrent, testLeecher)) + panicOnErr(tx.RemoveLeecher(testTorrent, testLeecher)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if found || foundLeecher == *testLeecher { t.Error("leecher not removed from cache", foundLeecher, *testLeecher) @@ -275,17 +276,17 @@ func TestRemoveLeecher(t *testing.T) { func TestSetSeeder(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) r := rand.New(rand.NewSource(time.Now().UnixNano())) testSeeder.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + panicOnErr(tx.SetSeeder(testTorrent, testSeeder)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if foundSeeder != *testSeeder { t.Error("seeder not updated in cache", foundSeeder, *testSeeder) @@ -299,16 +300,16 @@ func TestSetSeeder(t *testing.T) { func TestSetLeecher(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) r := rand.New(rand.NewSource(time.Now().UnixNano())) testLeecher.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetLeecher(testTorrent, testLeecher)) + panicOnErr(tx.SetLeecher(testTorrent, testLeecher)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, _ := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if foundLeecher != *testLeecher { t.Error("leecher not updated in cache", testLeecher) @@ -322,12 +323,12 @@ func TestSetLeecher(t *testing.T) { func TestIncrementSlots(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) numSlots := testUser.Slots - panicErrNil(tx.IncrementSlots(testUser)) + panicOnErr(tx.IncrementSlots(testUser)) foundUser, _, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if foundUser.Slots != numSlots+1 { t.Error("cached slots not incremented") @@ -340,12 +341,12 @@ func TestIncrementSlots(t *testing.T) { func TestDecrementSlots(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) numSlots := testUser.Slots - panicErrNil(tx.DecrementSlots(testUser)) + panicOnErr(tx.DecrementSlots(testUser)) foundUser, _, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if foundUser.Slots != numSlots-1 { t.Error("cached slots not incremented") @@ -358,15 +359,15 @@ func TestDecrementSlots(t *testing.T) { func TestLeecherFinished(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) testLeecher.Left = 0 - panicErrNil(tx.LeecherFinished(testTorrent, testLeecher)) + panicOnErr(tx.LeecherFinished(testTorrent, testLeecher)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testLeecher)] if foundSeeder != *testLeecher { t.Error("seeder not added to cache", foundSeeder, *testLeecher) @@ -390,17 +391,17 @@ func TestUpdatePeer(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddTorrent(testTorrent)) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) // Update a seeder, set it, then check to make sure it updated r := rand.New(rand.NewSource(time.Now().UnixNano())) testSeeder.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + panicOnErr(tx.SetSeeder(testTorrent, testSeeder)) - panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + panicOnErr(tx.RemoveSeeder(testTorrent, testSeeder)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if seeder, exists := foundTorrent.Seeders[models.PeerMapKey(testSeeder)]; exists { t.Error("seeder not removed from cache", seeder) } @@ -417,16 +418,16 @@ func TestParallelFindUser(t *testing.T) { tx := createTestTx() testUserSuccess := createTestUser() testUserFail := createTestUser() - panicErrNil(tx.AddUser(testUserSuccess)) + panicOnErr(tx.AddUser(testUserSuccess)) for i := 0; i < 10; i++ { foundUser, found, err := tx.FindUser(testUserFail.Passkey) - panicErrNil(err) + panicOnErr(err) if found { t.Error("user found", foundUser) } foundUser, found, err = tx.FindUser(testUserSuccess.Passkey) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("user not found", testUserSuccess) } @@ -444,19 +445,19 @@ func TestParallelFindTorrent(t *testing.T) { tx := createTestTx() testTorrentSuccess := createTestTorrent() testTorrentFail := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrentSuccess)) + panicOnErr(tx.AddTorrent(testTorrentSuccess)) for i := 0; i < 10; i++ { foundTorrent, found, err := tx.FindTorrent(testTorrentSuccess.Infohash) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("torrent not found", testTorrentSuccess) } - if !torrentsEqual(foundTorrent, testTorrentSuccess) { + if !reflect.DeepEqual(foundTorrent, testTorrentSuccess) { t.Error("found torrent mismatch", foundTorrent, testTorrentSuccess) } foundTorrent, found, err = tx.FindTorrent(testTorrentFail.Infohash) - panicErrNil(err) + panicOnErr(err) if found { t.Error("torrent found", foundTorrent) } @@ -470,18 +471,18 @@ func TestParallelSetSeeder(t *testing.T) { } tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) r := rand.New(rand.NewSource(time.Now().UnixNano())) for i := 0; i < 10; i++ { testSeeder.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + panicOnErr(tx.SetSeeder(testTorrent, testSeeder)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if foundSeeder != *testSeeder { t.Error("seeder not updated in cache", foundSeeder, *testSeeder) @@ -500,15 +501,15 @@ func TestParallelAddLeecher(t *testing.T) { } tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) for i := 0; i < 10; i++ { testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if found && foundLeecher != *testLeecher { t.Error("leecher not added to cache", testLeecher) diff --git a/models/models.go b/models/models.go index 1162321..5599c94 100644 --- a/models/models.go +++ b/models/models.go @@ -34,7 +34,7 @@ type Torrent struct { Seeders map[string]Peer `json:"seeders"` Leechers map[string]Peer `json:"leechers"` - Snatches uint `json:"snatches"` + Snatches uint64 `json:"snatches"` UpMultiplier float64 `json:"up_multiplier"` DownMultiplier float64 `json:"down_multiplier"` LastAction int64 `json:"last_action"` @@ -48,5 +48,5 @@ type User struct { DownMultiplier float64 `json:"down_multiplier"` Slots int64 `json:"slots"` SlotsUsed int64 `json:"slots_used"` - Snatches uint `json:"snatches"` + Snatches uint64 `json:"snatches"` }