Improving repo linting

topfreegames · Jul 10, 2020 · 5c6df6b · 5c6df6b
1 parent 9a9e42d
commit 5c6df6b
Show file tree

Hide file tree

Showing 55 changed files with 254 additions and 237 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -47,4 +47,19 @@ linters:
     - gomnd
     - interfacer
     - wsl
+    - depguard
+    - errcheck
+    - goerr113
+    - dupl
+    - gosec
+    - prealloc
+    - lll
+    - godot
+    - scopelint
+    - goconst
+    - unparam
+    - staticcheck
+    - stylecheck
+    - gosimple
+    - testpackage
 
diff --git a/README.md b/README.md
@@ -41,6 +41,12 @@ make build
 
 We're using [Ginkgo](https://onsi.github.io/ginkgo) and [Gomega](https://onsi.github.io/gomega) for testing our code. Since we're making extensive use of interfaces, external dependencies are mocked for all unit tests.
 
+### Linter
+
+```bash
+make lint
+```
+
 #### Unit Tests
 We'll try to keep testing coverage as high as possible. To run unit tests simply use:
 
@@ -70,6 +76,9 @@ To run integration tests run:
 make integration
 ```
 
+If you are running integration tests locally with the most recent librdkafka version (such as installed by brew) some of them will fail due to incompatible librdkafka version. 
+Tests should work for librdkafka v0.11.5
+
 ### Benchmark
 
 #### Create fake push data

diff --git a/cmd/apns.go b/cmd/apns.go
@@ -32,8 +32,6 @@ import (
 	"github.com/topfreegames/pusher/util"
 )
 
-var app string
-
 func startApns(
 	debug, json, production bool,
 	config *viper.Viper,

diff --git a/cmd/apns_test.go b/cmd/apns_test.go
@@ -38,21 +38,21 @@ var _ = Describe("APNS", func() {
 
 	var config *viper.Viper
 	var mockPushQueue *mocks.APNSPushQueueMock
-	var mockDb *mocks.PGMock
+	var mockDB *mocks.PGMock
 	var mockStatsDClient *mocks.StatsDClientMock
 
 	BeforeEach(func() {
 		var err error
 		config, err = util.NewViperWithConfigFile(cfg)
 		Expect(err).NotTo(HaveOccurred())
-		mockDb = mocks.NewPGMock(0, 1)
+		mockDB = mocks.NewPGMock(0, 1)
 		mockPushQueue = mocks.NewAPNSPushQueueMock()
 		mockStatsDClient = mocks.NewStatsDClientMock()
 	})
 
 	Describe("[Unit]", func() {
 		It("Should return apnsPusher without errors", func() {
-			apnsPusher, err := startApns(false, false, false, config, mockStatsDClient, mockDb, mockPushQueue)
+			apnsPusher, err := startApns(false, false, false, config, mockStatsDClient, mockDB, mockPushQueue)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(apnsPusher).NotTo(BeNil())
 			Expect(apnsPusher.Config).NotTo(BeNil())
@@ -62,21 +62,21 @@ var _ = Describe("APNS", func() {
 		})
 
 		It("Should set log to json format", func() {
-			apnsPusher, err := startApns(false, true, false, config, mockStatsDClient, mockDb, mockPushQueue)
+			apnsPusher, err := startApns(false, true, false, config, mockStatsDClient, mockDB, mockPushQueue)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(apnsPusher).NotTo(BeNil())
 			Expect(fmt.Sprintf("%T", apnsPusher.Logger.Formatter)).To(Equal(fmt.Sprintf("%T", &logrus.JSONFormatter{})))
 		})
 
 		It("Should set log to debug", func() {
-			apnsPusher, err := startApns(true, false, false, config, mockStatsDClient, mockDb, mockPushQueue)
+			apnsPusher, err := startApns(true, false, false, config, mockStatsDClient, mockDB, mockPushQueue)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(apnsPusher).NotTo(BeNil())
 			Expect(apnsPusher.Logger.Level).To(Equal(logrus.DebugLevel))
 		})
 
 		It("Should set log to production", func() {
-			apnsPusher, err := startApns(false, false, true, config, mockStatsDClient, mockDb, mockPushQueue)
+			apnsPusher, err := startApns(false, false, true, config, mockStatsDClient, mockDB, mockPushQueue)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(apnsPusher).NotTo(BeNil())
 			Expect(apnsPusher.IsProduction).To(BeTrue())

diff --git a/cmd/gcm_test.go b/cmd/gcm_test.go
@@ -40,21 +40,21 @@ var _ = Describe("GCM", func() {
 
 	var config *viper.Viper
 	var mockClient *mocks.GCMClientMock
-	var mockDb *mocks.PGMock
+	var mockDB *mocks.PGMock
 	var mockStatsDClient *mocks.StatsDClientMock
 
 	BeforeEach(func() {
 		var err error
 		config, err = util.NewViperWithConfigFile(cfg)
 		Expect(err).NotTo(HaveOccurred())
-		mockDb = mocks.NewPGMock(0, 1)
+		mockDB = mocks.NewPGMock(0, 1)
 		mockClient = mocks.NewGCMClientMock()
 		mockStatsDClient = mocks.NewStatsDClientMock()
 	})
 
 	Describe("[Unit]", func() {
 		It("Should return gcmPusher without errors", func() {
-			gcmPusher, err := startGcm(false, false, false, senderID, apiKey, config, mockStatsDClient, mockDb, mockClient)
+			gcmPusher, err := startGcm(false, false, false, senderID, apiKey, config, mockStatsDClient, mockDB, mockClient)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(gcmPusher).NotTo(BeNil())
 			Expect(gcmPusher.Config).NotTo(BeNil())
@@ -64,21 +64,21 @@ var _ = Describe("GCM", func() {
 		})
 
 		It("Should set log to json format", func() {
-			gcmPusher, err := startGcm(false, true, false, senderID, apiKey, config, mockStatsDClient, mockDb, mockClient)
+			gcmPusher, err := startGcm(false, true, false, senderID, apiKey, config, mockStatsDClient, mockDB, mockClient)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(gcmPusher).NotTo(BeNil())
 			Expect(fmt.Sprintf("%T", gcmPusher.Logger.Formatter)).To(Equal(fmt.Sprintf("%T", &logrus.JSONFormatter{})))
 		})
 
 		It("Should set log to debug", func() {
-			gcmPusher, err := startGcm(true, false, false, senderID, apiKey, config, mockStatsDClient, mockDb, mockClient)
+			gcmPusher, err := startGcm(true, false, false, senderID, apiKey, config, mockStatsDClient, mockDB, mockClient)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(gcmPusher).NotTo(BeNil())
 			Expect(gcmPusher.Logger.Level).To(Equal(logrus.DebugLevel))
 		})
 
 		It("Should set log to production", func() {
-			gcmPusher, err := startGcm(false, false, true, senderID, apiKey, config, mockStatsDClient, mockDb, mockClient)
+			gcmPusher, err := startGcm(false, false, true, senderID, apiKey, config, mockStatsDClient, mockDB, mockClient)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(gcmPusher).NotTo(BeNil())
 			Expect(gcmPusher.IsProduction).To(BeTrue())

diff --git a/errors/push.go b/errors/push.go
@@ -24,21 +24,21 @@ package errors
 
 import "fmt"
 
-//PushError reports an error sending a Push Message
+//PushError reports an error sending a Push Message.
 type PushError struct {
 	Key         string
 	Description string
 }
 
-//NewPushError creates a new instance
+//NewPushError creates a new instance.
 func NewPushError(key, description string) *PushError {
 	return &PushError{
 		Key:         key,
 		Description: description,
 	}
 }
 
-//Error returns a string
+//Error returns a string.
 func (e *PushError) Error() string {
 	return fmt.Sprintf("Sending push notification failed with error %s (%s).", e.Key, e.Description)
 }
diff --git a/extensions/apns_message_handler.go b/extensions/apns_message_handler.go
@@ -30,7 +30,6 @@ import (
 
 	uuid "github.com/satori/go.uuid"
 	"github.com/sideshow/apns2"
-	token "github.com/sideshow/apns2/token"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/viper"
 	"github.com/topfreegames/pusher/errors"
@@ -40,44 +39,41 @@ import (
 
 var apnsResMutex sync.Mutex
 
-// Notification is the notification base struct
+// Notification is the notification base struct.
 type Notification struct {
 	DeviceToken string
 	Payload     interface{}
 	Metadata    map[string]interface{} `json:"metadata,omitempty"`
 	PushExpiry  int64                  `json:"push_expiry,omitempty"`
 }
 
-// APNSMessageHandler implements the messagehandler interface
+// APNSMessageHandler implements the messagehandler interface.
 type APNSMessageHandler struct {
+	feedbackReporters            []interfaces.FeedbackReporter
+	StatsReporters               []interfaces.StatsReporter
 	authKeyPath                  string
 	keyID                        string
 	teamID                       string
-	token                        *token.Token
 	appName                      string
+	PushQueue                    interfaces.APNSPushQueue
+	Topic                        string
 	Config                       *viper.Viper
-	clients                      chan *apns2.Client
-	failuresReceived             int64
-	feedbackReporters            []interfaces.FeedbackReporter
+	FailuresReceived             int64
 	InflightMessagesMetadata     map[string]interface{}
-	IsProduction                 bool
 	Logger                       *log.Logger
 	LogStatsInterval             time.Duration
 	pendingMessagesWG            *sync.WaitGroup
 	inflightMessagesMetadataLock *sync.Mutex
-	PushQueue                    interfaces.APNSPushQueue
 	responsesReceived            int64
-	run                          bool
 	sentMessages                 int64
 	ignoredMessages              int64
-	StatsReporters               []interfaces.StatsReporter
 	successesReceived            int64
-	Topic                        string
 	requestsHeap                 *TimeoutHeap
 	CacheCleaningInterval        int
+	IsProduction                 bool
 }
 
-// NewAPNSMessageHandler returns a new instance of a APNSMessageHandler
+// NewAPNSMessageHandler returns a new instance of a APNSMessageHandler.
 func NewAPNSMessageHandler(
 	authKeyPath, keyID, teamID, topic, appName string,
 	isProduction bool,
@@ -95,7 +91,7 @@ func NewAPNSMessageHandler(
 		Topic:                        topic,
 		appName:                      appName,
 		Config:                       config,
-		failuresReceived:             0,
+		FailuresReceived:             0,
 		feedbackReporters:            feedbackReporters,
 		InflightMessagesMetadata:     map[string]interface{}{},
 		IsProduction:                 isProduction,
@@ -200,14 +196,14 @@ func (a *APNSMessageHandler) sendMessage(message interfaces.KafkaMessage) error
 	return nil
 }
 
-// HandleResponses from apns
+// HandleResponses from apns.
 func (a *APNSMessageHandler) HandleResponses() {
 	for response := range a.PushQueue.ResponseChannel() {
 		a.handleAPNSResponse(response)
 	}
 }
 
-// CleanMetadataCache clears expired requests from memory
+// CleanMetadataCache clears expired requests from memory.
 func (a *APNSMessageHandler) CleanMetadataCache() {
 	var deviceToken string
 	var hasIndeed bool
@@ -230,7 +226,7 @@ func (a *APNSMessageHandler) CleanMetadataCache() {
 	}
 }
 
-// HandleMessages get messages from msgChan and send to APNS
+// HandleMessages get messages from msgChan and send to APNS.
 func (a *APNSMessageHandler) HandleMessages(message interfaces.KafkaMessage) {
 	a.sendMessage(message)
 }
@@ -265,7 +261,7 @@ func (a *APNSMessageHandler) handleAPNSResponse(responseWithMetadata *structs.Re
 
 	if responseWithMetadata.Reason != "" {
 		apnsResMutex.Lock()
-		a.failuresReceived++
+		a.FailuresReceived++
 		apnsResMutex.Unlock()
 		reason := responseWithMetadata.Reason
 		pErr := errors.NewPushError(a.mapErrorReason(reason), reason)
@@ -327,7 +323,7 @@ func (a *APNSMessageHandler) handleAPNSResponse(responseWithMetadata *structs.Re
 	return nil
 }
 
-// LogStats from time to time
+// LogStats from time to time.
 func (a *APNSMessageHandler) LogStats() {
 	l := a.Logger.WithFields(log.Fields{
 		"method":       "logStats",
@@ -337,19 +333,19 @@ func (a *APNSMessageHandler) LogStats() {
 	ticker := time.NewTicker(a.LogStatsInterval)
 	for range ticker.C {
 		apnsResMutex.Lock()
-		if a.sentMessages > 0 || a.responsesReceived > 0 || a.ignoredMessages > 0 || a.successesReceived > 0 || a.failuresReceived > 0 {
+		if a.sentMessages > 0 || a.responsesReceived > 0 || a.ignoredMessages > 0 || a.successesReceived > 0 || a.FailuresReceived > 0 {
 			l.WithFields(log.Fields{
 				"sentMessages":      a.sentMessages,
 				"ignoredMessages":   a.ignoredMessages,
 				"responsesReceived": a.responsesReceived,
 				"successesReceived": a.successesReceived,
-				"failuresReceived":  a.failuresReceived,
+				"failuresReceived":  a.FailuresReceived,
 			}).Info("flushing stats")
 			a.sentMessages = 0
 			a.responsesReceived = 0
 			a.ignoredMessages = 0
 			a.successesReceived = 0
-			a.failuresReceived = 0
+			a.FailuresReceived = 0
 		}
 		apnsResMutex.Unlock()
 	}
@@ -414,7 +410,7 @@ func (a *APNSMessageHandler) mapErrorReason(reason string) string {
 	}
 }
 
-//Cleanup closes connections to APNS
+//Cleanup closes connections to APNS.
 func (a *APNSMessageHandler) Cleanup() error {
 	a.PushQueue.Close()
 	return nil