Settings by value (#45)

* Change interfaces to return values instead of pointers * With updated benchmarks. Not very consequential. Compare against last rev to see prior benchmarks.
efixler · Aug 1, 2024 · e11b2ce · e11b2ce
1 parent 61ab140
commit e11b2ce
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 34 deletions.
diff --git a/internal/server/settings.go b/internal/server/settings.go
@@ -27,7 +27,7 @@ type batchDomainSettingsRequest struct {
 
 type batchDomainSettingsResponse struct {
 	Request  batchDomainSettingsRequest `json:"request"`
-	Settings []*settings.DomainSettings `json:"settings"`
+	Settings []settings.DomainSettings  `json:"settings"`
 }
 
 type dsKey struct{}

diff --git a/internal/server/settings_test.go b/internal/server/settings_test.go
@@ -301,7 +301,7 @@ func TestGetBatchDomainSettings(t *testing.T) {
 	tests := []struct {
 		name         string
 		error        error
-		settings     []*settings.DomainSettings
+		settings     []settings.DomainSettings
 		expectStatus int
 	}{
 		{
@@ -310,14 +310,14 @@ func TestGetBatchDomainSettings(t *testing.T) {
 		},
 		{
 			name: "single",
-			settings: []*settings.DomainSettings{
+			settings: []settings.DomainSettings{
 				{Domain: "example.com", Sitename: "example"},
 			},
 			expectStatus: 200,
 		},
 		{
 			name: "multiple",
-			settings: []*settings.DomainSettings{
+			settings: []settings.DomainSettings{
 				{Domain: "example.com", Sitename: "example"},
 				{Domain: "example2.org", Sitename: "example2"},
 			},
@@ -373,18 +373,18 @@ func TestGetBatchDomainSettings(t *testing.T) {
 }
 
 type mockDomainSettingsStorage struct {
-	settings []*settings.DomainSettings
+	settings []settings.DomainSettings
 	error    error
 }
 
-func (m *mockDomainSettingsStorage) Fetch(domain string) (*settings.DomainSettings, error) {
+func (m *mockDomainSettingsStorage) Fetch(domain string) (settings.DomainSettings, error) {
 	if m.error != nil {
-		return nil, m.error
+		return settings.DomainSettings{}, m.error
 	}
 	return m.settings[0], nil
 }
 
-func (m mockDomainSettingsStorage) FetchRange(offset, limit int, query string) ([]*settings.DomainSettings, error) {
+func (m mockDomainSettingsStorage) FetchRange(offset, limit int, query string) ([]settings.DomainSettings, error) {
 	if m.error != nil {
 		return nil, m.error
 	}

diff --git a/internal/server/version/version.go b/internal/server/version/version.go
@@ -1,7 +1,7 @@
 package version
 
 const (
-	Commit  = "6cd5afa"
+	Commit  = "7d17e66"
 	Tag     = "v0.8.6"
 	RepoURL = "https://github.com/efixler/scrape"
 )
diff --git a/internal/settings/benchmark_test.go b/internal/settings/benchmark_test.go
@@ -39,17 +39,17 @@ func populateTestDB(dbh *database.DBHandle, count int) ([]string, error) {
 // Results: (saving here to reference on testings some possible improvements)
 // BenchmarkLoadDomainSetting
 // BenchmarkLoadDomainSetting/sqlite:memory:100
-// BenchmarkLoadDomainSetting/sqlite:memory:100-10         	  188026	      6101 ns/op	    2517 B/op	      51 allocs/op
+// BenchmarkLoadDomainSetting/sqlite:memory:100-10         	  177280	      5765 ns/op	    2474 B/op	      51 allocs/op
 // BenchmarkLoadDomainSetting/sqlite:memory:1000
-// BenchmarkLoadDomainSetting/sqlite:memory:1000-10        	  201190	      5862 ns/op	    2502 B/op	      51 allocs/op
+// BenchmarkLoadDomainSetting/sqlite:memory:1000-10        	  189783	      5909 ns/op	    2502 B/op	      51 allocs/op
 // BenchmarkLoadDomainSetting/sqlite:memory:10000
-// BenchmarkLoadDomainSetting/sqlite:memory:10000-10       	  184486	      6098 ns/op	    2506 B/op	      51 allocs/op
+// BenchmarkLoadDomainSetting/sqlite:memory:10000-10       	  195798	      6128 ns/op	    2503 B/op	      51 allocs/op
 // BenchmarkLoadDomainSetting/sqlite:tmpfile:100
-// BenchmarkLoadDomainSetting/sqlite:tmpfile:100-10        	  168121	      6753 ns/op	    2492 B/op	      51 allocs/op
+// BenchmarkLoadDomainSetting/sqlite:tmpfile:100-10        	  165363	      6763 ns/op	    2504 B/op	      51 allocs/op
 // BenchmarkLoadDomainSetting/sqlite:tmpfile:1000
-// BenchmarkLoadDomainSetting/sqlite:tmpfile:1000-10       	  161414	      7061 ns/op	    2508 B/op	      51 allocs/op
+// BenchmarkLoadDomainSetting/sqlite:tmpfile:1000-10       	  169336	      6967 ns/op	    2507 B/op	      51 allocs/op
 // BenchmarkLoadDomainSetting/sqlite:tmpfile:10000
-// BenchmarkLoadDomainSetting/sqlite:tmpfile:10000-10      	  166410	      7194 ns/op	    2510 B/op	      51 allocs/op
+// BenchmarkLoadDomainSetting/sqlite:tmpfile:10000-10      	  165699	      7159 ns/op	    2502 B/op	      51 allocs/op
 
 func BenchmarkLoadDomainSetting(b *testing.B) {
 	var tests = []struct {
@@ -91,21 +91,22 @@ func benchmarkLoadDomainSetting(b *testing.B, dss *domainSettingsStorage, domain
 // Results: (saving here to reference on testings some possible improvements)
 // BenchmarkLoadDomainBatch
 // BenchmarkLoadDomainBatch/sqlite:memory:100
-// BenchmarkLoadDomainBatch/sqlite:memory:100-10         	    3500	    332974 ns/op	  174834 B/op	    2426 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:memory:100-10         	    3529	    334731 ns/op	  180754 B/op	    2426 allocs/op
 // BenchmarkLoadDomainBatch/sqlite:memory:500
-// BenchmarkLoadDomainBatch/sqlite:memory:500-10         	     710	   1723392 ns/op	  872429 B/op	   12021 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:memory:500-10         	     712	   1691428 ns/op	  898205 B/op	   12019 allocs/op
 // BenchmarkLoadDomainBatch/sqlite:memory:1000
-// BenchmarkLoadDomainBatch/sqlite:memory:1000-10        	     350	   3495147 ns/op	 1732811 B/op	   24012 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:memory:1000-10        	     343	   3455747 ns/op	 1781367 B/op	   24010 allocs/op
 // BenchmarkLoadDomainBatch/sqlite:memory:10000
-// BenchmarkLoadDomainBatch/sqlite:memory:10000-10       	      31	  37701871 ns/op	17312000 B/op	  239880 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:memory:10000-10       	      32	  38905829 ns/op	17881840 B/op	  239885 allocs/op
 // BenchmarkLoadDomainBatch/sqlite:tmpfile:100
-// BenchmarkLoadDomainBatch/sqlite:tmpfile:100-10        	    3345	    336056 ns/op	  176154 B/op	    2426 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:tmpfile:100-10        	    3344	    332339 ns/op	  177914 B/op	    2428 allocs/op
 // BenchmarkLoadDomainBatch/sqlite:tmpfile:500
-// BenchmarkLoadDomainBatch/sqlite:tmpfile:500-10        	     702	   1693998 ns/op	  870600 B/op	   12021 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:tmpfile:500-10        	     712	   1712945 ns/op	  899061 B/op	   12020 allocs/op
 // BenchmarkLoadDomainBatch/sqlite:tmpfile:1000
-// BenchmarkLoadDomainBatch/sqlite:tmpfile:1000-10       	     348	   3427594 ns/op	 1721624 B/op	   24013 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:tmpfile:1000-10       	     346	   3436954 ns/op	 1782174 B/op	   24013 allocs/op
 // BenchmarkLoadDomainBatch/sqlite:tmpfile:10000
-// BenchmarkLoadDomainBatch/sqlite:tmpfile:10000-10      	      32	  37376171 ns/op	17297943 B/op	  239841 allocs/op
+// BenchmarkLoadDomainBatch/sqlite:tmpfile:10000-10      	      32	  37236333 ns/op	17836775 B/op	  239868 allocs/op
+
 func BenchmarkLoadDomainBatch(b *testing.B) {
 	var tests = []struct {
 		name string

diff --git a/internal/settings/domain.go b/internal/settings/domain.go
@@ -62,8 +62,8 @@ func NewDomainSettings(domain string) (*DomainSettings, error) {
 
 type DomainSettingsStore interface {
 	Delete(string) (bool, error)
-	Fetch(string) (*DomainSettings, error)
-	FetchRange(int, int, string) ([]*DomainSettings, error)
+	Fetch(string) (DomainSettings, error)
+	FetchRange(int, int, string) ([]DomainSettings, error)
 	Save(*DomainSettings) error
 }
 
@@ -110,7 +110,7 @@ func (d *domainSettingsStorage) Delete(domain string) (bool, error) {
 	}
 }
 
-func (d *domainSettingsStorage) Fetch(domain string) (*DomainSettings, error) {
+func (d *domainSettingsStorage) Fetch(domain string) (DomainSettings, error) {
 	stmt, err := d.Statement(fetch, func(ctx context.Context, db *sql.DB) (*sql.Stmt, error) {
 		return db.PrepareContext(
 			ctx,
@@ -119,25 +119,25 @@ func (d *domainSettingsStorage) Fetch(domain string) (*DomainSettings, error) {
 		)
 	})
 	if err != nil {
-		return nil, err
+		return DomainSettings{}, err
 	}
 	rows, err := stmt.QueryContext(d.Ctx, domain)
 	if err != nil {
-		return nil, err
+		return DomainSettings{}, err
 	}
 	defer rows.Close()
 	if !rows.Next() {
-		return nil, store.ErrResourceNotFound
+		return DomainSettings{}, store.ErrResourceNotFound
 	}
 	ds, err := d.loadSettingFromRow(rows)
 	if err != nil {
-		return nil, err
+		return DomainSettings{}, err
 	}
 	return ds, nil
 }
 
-func (d *domainSettingsStorage) loadSettingFromRow(rows *sql.Rows) (*DomainSettings, error) {
-	ds := &DomainSettings{}
+func (d *domainSettingsStorage) loadSettingFromRow(rows *sql.Rows) (DomainSettings, error) {
+	ds := DomainSettings{}
 	var headers string
 	err := rows.Scan(&ds.Domain, &ds.Sitename, &ds.FetchClient, &ds.UserAgent, &headers)
 	if err != nil {
@@ -153,7 +153,7 @@ func (d *domainSettingsStorage) loadSettingFromRow(rows *sql.Rows) (*DomainSetti
 // by the given limit. If query is not empty, it will be used to filter the results.
 // The query string may contain a leading and/or trailing * to match anything before or after the
 // rest of the query. Queries with no asterisks are treated as if they had an asterisk on both sides.
-func (d *domainSettingsStorage) FetchRange(offset int, limit int, query string) ([]*DomainSettings, error) {
+func (d *domainSettingsStorage) FetchRange(offset int, limit int, query string) ([]DomainSettings, error) {
 	switch limit {
 	case 0:
 		limit = DefaultDomainSettingsBatchSize
@@ -200,7 +200,7 @@ func (d *domainSettingsStorage) FetchRange(offset int, limit int, query string)
 		return nil, err
 	}
 	defer rows.Close()
-	dss := make([]*DomainSettings, 0, limit)
+	dss := make([]DomainSettings, 0, limit)
 	for rows.Next() {
 		ds, err := d.loadSettingFromRow(rows)
 		if err != nil {