From a5728767758b8c6b98502f78702c134f90e0e20c Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 7 Dec 2020 14:32:06 +0300 Subject: [PATCH] Pull request: all: fix lint and naming issues Merge in DNS/adguard-home from 2276-fix-lint to master Updates #2276. Squashed commit of the following: commit 433f44cc7b674a20ed60a9d29466ba888b3ef66e Author: Ainar Garipov Date: Mon Dec 7 14:14:28 2020 +0300 querylog: improve code and documentation commit 851df97d2a87de5e7180a502055ee6f1a6defdca Author: Ainar Garipov Date: Fri Dec 4 20:36:32 2020 +0300 all: fix lint and naming issues --- internal/querylog/decode.go | 16 ++++++++++--- .../querylog/{qlog_file.go => qlogfile.go} | 4 ++-- .../{qlog_file_test.go => qlogfile_test.go} | 24 +++++++++---------- .../{qlog_reader.go => qlogreader.go} | 13 ++++------ ...qlog_reader_test.go => qlogreader_test.go} | 12 +++++----- .../{querylog_file.go => querylogfile.go} | 0 internal/querylog/search.go | 4 ++-- .../{search_criteria.go => searchcriteria.go} | 0 .../{search_params.go => searchparams.go} | 0 internal/stats/{stats_http.go => http.go} | 8 ++++++- internal/stats/stats.go | 4 ++-- internal/stats/{stats_unit.go => unit.go} | 0 internal/update/update_test.go | 20 ++++++++-------- internal/update/updater.go | 3 ++- 14 files changed, 61 insertions(+), 47 deletions(-) rename internal/querylog/{qlog_file.go => qlogfile.go} (98%) rename internal/querylog/{qlog_file_test.go => qlogfile_test.go} (95%) rename internal/querylog/{qlog_reader.go => qlogreader.go} (88%) rename internal/querylog/{qlog_reader_test.go => qlogreader_test.go} (97%) rename internal/querylog/{querylog_file.go => querylogfile.go} (100%) rename internal/querylog/{search_criteria.go => searchcriteria.go} (100%) rename internal/querylog/{search_params.go => searchparams.go} (100%) rename internal/stats/{stats_http.go => http.go} (94%) rename internal/stats/{stats_unit.go => unit.go} (100%) diff --git a/internal/querylog/decode.go b/internal/querylog/decode.go index b385f3eb..93a1d265 100644 --- a/internal/querylog/decode.go +++ b/internal/querylog/decode.go @@ -12,7 +12,9 @@ import ( "github.com/miekg/dns" ) -var logEntryHandlers = map[string](func(t json.Token, ent *logEntry) error){ +type logEntryHandler (func(t json.Token, ent *logEntry) error) + +var logEntryHandlers = map[string]logEntryHandler{ "IP": func(t json.Token, ent *logEntry) error { v, ok := t.(string) if !ok { @@ -203,16 +205,24 @@ func decodeLogEntry(ent *logEntry, str string) { if _, ok := keyToken.(json.Delim); ok { continue } - key := keyToken.(string) + + key, ok := keyToken.(string) + if !ok { + log.Debug("decodeLogEntry: keyToken is %T and not string", keyToken) + return + } + handler, ok := logEntryHandlers[key] if !ok { continue } + val, err := dec.Token() if err != nil { return } - if err := handler(val, ent); err != nil { + + if err = handler(val, ent); err != nil { log.Debug("decodeLogEntry err: %s", err) return } diff --git a/internal/querylog/qlog_file.go b/internal/querylog/qlogfile.go similarity index 98% rename from internal/querylog/qlog_file.go rename to internal/querylog/qlogfile.go index 13754427..69a42ed2 100644 --- a/internal/querylog/qlog_file.go +++ b/internal/querylog/qlogfile.go @@ -53,7 +53,7 @@ func NewQLogFile(path string) (*QLogFile, error) { }, nil } -// Seek performs binary search in the query log file looking for a record +// SeekTS performs binary search in the query log file looking for a record // with the specified timestamp. Once the record is found, it sets // "position" so that the next ReadNext call returned that record. // @@ -70,7 +70,7 @@ func NewQLogFile(path string) (*QLogFile, error) { // so that when we call "ReadNext" this line was returned. // * Depth of the search (how many times we compared timestamps). // * If we could not find it, it returns one of the errors described above. -func (q *QLogFile) Seek(timestamp int64) (int64, int, error) { +func (q *QLogFile) SeekTS(timestamp int64) (int64, int, error) { q.lock.Lock() defer q.lock.Unlock() diff --git a/internal/querylog/qlog_file_test.go b/internal/querylog/qlogfile_test.go similarity index 95% rename from internal/querylog/qlog_file_test.go rename to internal/querylog/qlogfile_test.go index b458e071..950eaaf3 100644 --- a/internal/querylog/qlog_file_test.go +++ b/internal/querylog/qlogfile_test.go @@ -61,7 +61,7 @@ func TestQLogFileLarge(t *testing.T) { line, err = q.ReadNext() if err == nil { assert.True(t, len(line) > 0) - read += 1 + read++ } } @@ -96,12 +96,12 @@ func TestQLogFileSeekLargeFile(t *testing.T) { testSeekLineQLogFile(t, q, count) // CASE 5: Seek non-existent (too low) - _, _, err = q.Seek(123) + _, _, err = q.SeekTS(123) assert.NotNil(t, err) // CASE 6: Seek non-existent (too high) ts, _ := time.Parse(time.RFC3339, "2100-01-02T15:04:05Z07:00") - _, _, err = q.Seek(ts.UnixNano()) + _, _, err = q.SeekTS(ts.UnixNano()) assert.NotNil(t, err) // CASE 7: "Almost" found @@ -110,7 +110,7 @@ func TestQLogFileSeekLargeFile(t *testing.T) { // ALMOST the record we need timestamp := readQLogTimestamp(line) - 1 assert.NotEqual(t, uint64(0), timestamp) - _, depth, err := q.Seek(timestamp) + _, depth, err := q.SeekTS(timestamp) assert.NotNil(t, err) assert.True(t, depth <= int(math.Log2(float64(count))+3)) } @@ -142,12 +142,12 @@ func TestQLogFileSeekSmallFile(t *testing.T) { testSeekLineQLogFile(t, q, count) // CASE 5: Seek non-existent (too low) - _, _, err = q.Seek(123) + _, _, err = q.SeekTS(123) assert.NotNil(t, err) // CASE 6: Seek non-existent (too high) ts, _ := time.Parse(time.RFC3339, "2100-01-02T15:04:05Z07:00") - _, _, err = q.Seek(ts.UnixNano()) + _, _, err = q.SeekTS(ts.UnixNano()) assert.NotNil(t, err) // CASE 7: "Almost" found @@ -156,7 +156,7 @@ func TestQLogFileSeekSmallFile(t *testing.T) { // ALMOST the record we need timestamp := readQLogTimestamp(line) - 1 assert.NotEqual(t, uint64(0), timestamp) - _, depth, err := q.Seek(timestamp) + _, depth, err := q.SeekTS(timestamp) assert.NotNil(t, err) assert.True(t, depth <= int(math.Log2(float64(count))+3)) } @@ -168,7 +168,7 @@ func testSeekLineQLogFile(t *testing.T, q *QLogFile, lineNumber int) { assert.NotEqual(t, uint64(0), ts) // try seeking to that line now - pos, _, err := q.Seek(ts) + pos, _, err := q.SeekTS(ts) assert.Nil(t, err) assert.NotEqual(t, int64(0), pos) @@ -249,7 +249,7 @@ func prepareTestFiles(dir string, filesCount, linesCount int) []string { files[filesCount-j-1] = f.Name() for i := 0; i < linesCount; i++ { - lineIP += 1 + lineIP++ lineTime = lineTime.Add(time.Second) ip := make(net.IP, 4) @@ -284,7 +284,7 @@ func TestQLogSeek(t *testing.T) { target, _ := time.Parse(time.RFC3339, "2020-08-31T18:44:25.376690873+03:00") - _, depth, err := q.Seek(target.UnixNano()) + _, depth, err := q.SeekTS(target.UnixNano()) assert.Nil(t, err) assert.Equal(t, 1, depth) } @@ -313,7 +313,7 @@ func TestQLogSeek_ErrTSTooLate(t *testing.T) { target, err := time.Parse(time.RFC3339, "2020-08-31T18:44:25.382540454+03:00") assert.Nil(t, err) - _, depth, err := q.Seek(target.UnixNano() + int64(time.Second)) + _, depth, err := q.SeekTS(target.UnixNano() + int64(time.Second)) assert.Equal(t, ErrTSTooLate, err) assert.Equal(t, 2, depth) } @@ -342,7 +342,7 @@ func TestQLogSeek_ErrTSTooEarly(t *testing.T) { target, err := time.Parse(time.RFC3339, "2020-08-31T18:44:23.911246629+03:00") assert.Nil(t, err) - _, depth, err := q.Seek(target.UnixNano() - int64(time.Second)) + _, depth, err := q.SeekTS(target.UnixNano() - int64(time.Second)) assert.Equal(t, ErrTSTooEarly, err) assert.Equal(t, 1, depth) } diff --git a/internal/querylog/qlog_reader.go b/internal/querylog/qlogreader.go similarity index 88% rename from internal/querylog/qlog_reader.go rename to internal/querylog/qlogreader.go index dbb058c6..19909110 100644 --- a/internal/querylog/qlog_reader.go +++ b/internal/querylog/qlogreader.go @@ -44,16 +44,13 @@ func NewQLogReader(files []string) (*QLogReader, error) { }, nil } -// Seek performs binary search of a query log record with the specified timestamp. -// If the record is found, it sets QLogReader's position to point to that line, -// so that the next ReadNext call returned this line. -// -// Returns nil if the record is successfully found. -// Returns an error if for some reason we could not find a record with the specified timestamp. -func (r *QLogReader) Seek(timestamp int64) (err error) { +// SeekTS performs binary search of a query log record with the specified +// timestamp. If the record is found, it sets QLogReader's position to point to +// that line, so that the next ReadNext call returned this line. +func (r *QLogReader) SeekTS(timestamp int64) (err error) { for i := len(r.qFiles) - 1; i >= 0; i-- { q := r.qFiles[i] - _, _, err = q.Seek(timestamp) + _, _, err = q.SeekTS(timestamp) if err == nil { // Search is finished, and the searched element have // been found. Update currentFile only, position is diff --git a/internal/querylog/qlog_reader_test.go b/internal/querylog/qlogreader_test.go similarity index 97% rename from internal/querylog/qlog_reader_test.go rename to internal/querylog/qlogreader_test.go index f1802a0a..d9dfb3ea 100644 --- a/internal/querylog/qlog_reader_test.go +++ b/internal/querylog/qlogreader_test.go @@ -50,7 +50,7 @@ func TestQLogReaderOneFile(t *testing.T) { line, err = r.ReadNext() if err == nil { assert.True(t, len(line) > 0) - read += 1 + read++ } } @@ -83,7 +83,7 @@ func TestQLogReaderMultipleFiles(t *testing.T) { line, err = r.ReadNext() if err == nil { assert.True(t, len(line) > 0) - read += 1 + read++ } } @@ -147,7 +147,7 @@ func TestQLogReader_Seek(t *testing.T) { timestamp, err := time.Parse(time.RFC3339Nano, tc.time) assert.Nil(t, err) - err = r.Seek(timestamp.UnixNano()) + err = r.SeekTS(timestamp.UnixNano()) assert.True(t, errors.Is(err, tc.want), err) }) } @@ -228,12 +228,12 @@ func TestQLogReaderSeek(t *testing.T) { testSeekLineQLogReader(t, r, count) // CASE 5: Seek non-existent (too low) - err = r.Seek(123) + err = r.SeekTS(123) assert.NotNil(t, err) // CASE 6: Seek non-existent (too high) ts, _ := time.Parse(time.RFC3339, "2100-01-02T15:04:05Z07:00") - err = r.Seek(ts.UnixNano()) + err = r.SeekTS(ts.UnixNano()) assert.NotNil(t, err) } @@ -244,7 +244,7 @@ func testSeekLineQLogReader(t *testing.T, r *QLogReader, lineNumber int) { assert.NotEqual(t, uint64(0), ts) // try seeking to that line now - err = r.Seek(ts) + err = r.SeekTS(ts) assert.Nil(t, err) testLine, err := r.ReadNext() diff --git a/internal/querylog/querylog_file.go b/internal/querylog/querylogfile.go similarity index 100% rename from internal/querylog/querylog_file.go rename to internal/querylog/querylogfile.go diff --git a/internal/querylog/search.go b/internal/querylog/search.go index 0b9aa7d2..f23a42b6 100644 --- a/internal/querylog/search.go +++ b/internal/querylog/search.go @@ -97,7 +97,7 @@ func (l *queryLog) searchFiles(params *searchParams) ([]*logEntry, time.Time, in if params.olderThan.IsZero() { err = r.SeekStart() } else { - err = r.Seek(params.olderThan.UnixNano()) + err = r.SeekTS(params.olderThan.UnixNano()) if err == nil { // Read to the next record right away // The one that was specified in the "oldest" param is not needed, @@ -107,7 +107,7 @@ func (l *queryLog) searchFiles(params *searchParams) ([]*logEntry, time.Time, in } if err != nil { - log.Debug("Cannot Seek() to %v: %v", params.olderThan, err) + log.Debug("Cannot SeekTS() to %v: %v", params.olderThan, err) return entries, oldest, 0 } diff --git a/internal/querylog/search_criteria.go b/internal/querylog/searchcriteria.go similarity index 100% rename from internal/querylog/search_criteria.go rename to internal/querylog/searchcriteria.go diff --git a/internal/querylog/search_params.go b/internal/querylog/searchparams.go similarity index 100% rename from internal/querylog/search_params.go rename to internal/querylog/searchparams.go diff --git a/internal/stats/stats_http.go b/internal/stats/http.go similarity index 94% rename from internal/stats/stats_http.go rename to internal/stats/http.go index ab49a6ae..794bedf1 100644 --- a/internal/stats/stats_http.go +++ b/internal/stats/http.go @@ -37,7 +37,13 @@ func (s *statsCtx) handleStats(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Content-Type", "application/json") - w.Write(data) + + _, err = w.Write(data) + if err != nil { + httpError(r, w, http.StatusInternalServerError, "json encode: %s", err) + + return + } } type config struct { diff --git a/internal/stats/stats.go b/internal/stats/stats.go index 8f77425f..5cd5910d 100644 --- a/internal/stats/stats.go +++ b/internal/stats/stats.go @@ -1,5 +1,5 @@ -// Module for managing statistics for DNS filtering server - +// Package stats provides units for managing statistics of the filtering DNS +// server. package stats import ( diff --git a/internal/stats/stats_unit.go b/internal/stats/unit.go similarity index 100% rename from internal/stats/stats_unit.go rename to internal/stats/unit.go diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 5616104a..ceca2b9d 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -88,16 +88,16 @@ func TestUpdateGetVersion(t *testing.T) { } func TestUpdate(t *testing.T) { - _ = os.Mkdir("aghtest", 0755) + _ = os.Mkdir("aghtest", 0o755) defer func() { _ = os.RemoveAll("aghtest") }() // create "current" files - assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome", []byte("AdGuardHome"), 0755)) - assert.Nil(t, ioutil.WriteFile("aghtest/README.md", []byte("README.md"), 0644)) - assert.Nil(t, ioutil.WriteFile("aghtest/LICENSE.txt", []byte("LICENSE.txt"), 0644)) - assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome.yaml", []byte("AdGuardHome.yaml"), 0644)) + assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome", []byte("AdGuardHome"), 0o755)) + assert.Nil(t, ioutil.WriteFile("aghtest/README.md", []byte("README.md"), 0o644)) + assert.Nil(t, ioutil.WriteFile("aghtest/LICENSE.txt", []byte("LICENSE.txt"), 0o644)) + assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome.yaml", []byte("AdGuardHome.yaml"), 0o644)) // start server for returning package file pkgData, err := ioutil.ReadFile("test/AdGuardHome.tar.gz") @@ -151,16 +151,16 @@ func TestUpdate(t *testing.T) { } func TestUpdateWindows(t *testing.T) { - _ = os.Mkdir("aghtest", 0755) + _ = os.Mkdir("aghtest", 0o755) defer func() { _ = os.RemoveAll("aghtest") }() // create "current" files - assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome.exe", []byte("AdGuardHome.exe"), 0755)) - assert.Nil(t, ioutil.WriteFile("aghtest/README.md", []byte("README.md"), 0644)) - assert.Nil(t, ioutil.WriteFile("aghtest/LICENSE.txt", []byte("LICENSE.txt"), 0644)) - assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome.yaml", []byte("AdGuardHome.yaml"), 0644)) + assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome.exe", []byte("AdGuardHome.exe"), 0o755)) + assert.Nil(t, ioutil.WriteFile("aghtest/README.md", []byte("README.md"), 0o644)) + assert.Nil(t, ioutil.WriteFile("aghtest/LICENSE.txt", []byte("LICENSE.txt"), 0o644)) + assert.Nil(t, ioutil.WriteFile("aghtest/AdGuardHome.yaml", []byte("AdGuardHome.yaml"), 0o644)) // start server for returning package file pkgData, err := ioutil.ReadFile("test/AdGuardHome.zip") diff --git a/internal/update/updater.go b/internal/update/updater.go index 34d66819..b6901889 100644 --- a/internal/update/updater.go +++ b/internal/update/updater.go @@ -1,3 +1,4 @@ +// Package update provides an updater for AdGuardHome. package update import ( @@ -224,7 +225,7 @@ func (u *Updater) clean() { const MaxPackageFileSize = 32 * 1024 * 1024 // Download package file and save it to disk -func (u *Updater) downloadPackageFile(url string, filename string) error { +func (u *Updater) downloadPackageFile(url, filename string) error { resp, err := u.Client.Get(url) if err != nil { return fmt.Errorf("http request failed: %w", err)