Merge remote-tracking branch 'origin/main' into atavism/desktop-updates

README.md

@@ -1,7 +1,7 @@
 # Lantern [![Go Actions Status](https://github.com/getlantern/flashlight/actions/workflows/go.yml/badge.svg)](https://github.com/getlantern/flashlight/actions) [![Coverage Status](https://coveralls.io/repos/github/getlantern/flashlight/badge.svg?t=C4SaZX)](https://coveralls.io/github/getlantern/flashlight)
 
 ## Maintainers
-@oxtoacart, @hwh33, @myleshorton
+@hwh33, @myleshorton, @reflog
 
 This repo contains the core Lantern library as well as the Android and iOS bindings.
 

geolookup/geolookup.go

@@ -153,17 +153,38 @@ func init() {
 
 func run() {
 	for range refreshRequest {
-		gi := lookup()
-		if gi.IP == GetIP(0) && !gi.FromDisk {
-			log.Debug("public IP did not change - not notifying watchers")
+		geoInfo := lookup()
+
+		// Check if the IP has changed and if the old IP is simply cached from
+		// disk. If it is cached, we should still notify anyone looking for
+		// a new IP because they won't have been notified of the IP on disk,
+		// as that is loaded very soon on startup.
+		if !isNew(geoInfo) {
+			log.Debug("public IP from network did not change - not notifying watchers")
 			continue
 		}
+		log.Debug("Setting new geolocation info")
 		mx.Lock()
-		setGeoInfo(gi, true)
+		setGeoInfo(geoInfo, true)
 		mx.Unlock()
 	}
 }
 
+func isNew(newGeoInfo *GeoInfo) bool {
+	if newGeoInfo == nil {
+		return false
+	}
+	oldGeoInfo, err := GetGeoInfo(0)
+	if err != nil {
+		return true
+	}
+	if oldGeoInfo == nil {
+		return true
+	}
+	return oldGeoInfo.IP != newGeoInfo.IP ||
+		oldGeoInfo.FromDisk != newGeoInfo.FromDisk
+}
+
 func setGeoInfo(gi *GeoInfo, persist bool) {
 	currentGeoInfo.Set(gi)
 	w := watchers

geolookup/geolookup_test.go

@@ -8,10 +8,9 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/require"
-
 	"github.com/getlantern/eventual/v2"
 	"github.com/getlantern/fronted"
+	"github.com/stretchr/testify/require"
 )
 
 const initialInfo = `
@@ -196,3 +195,29 @@ func TestFronted(t *testing.T) {
 		"persisted geolocation information should have changed",
 	)
 }
+
+func TestIsNew(t *testing.T) {
+	type args struct {
+		newGeoInfo *GeoInfo
+		oldGeoInfo *GeoInfo
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{"nil new should be not new", args{nil, &GeoInfo{FromDisk: true}}, false},
+		{"nil existing should be new", args{&GeoInfo{}, nil}, true},
+		{"old from disk should be new", args{&GeoInfo{IP: "1.1.1.1", FromDisk: false}, &GeoInfo{IP: "1.1.1.1", FromDisk: true}}, true},
+		{"old not from disk should not be new", args{&GeoInfo{IP: "1.1.1.1", FromDisk: false}, &GeoInfo{IP: "1.1.1.1", FromDisk: false}}, false},
+		{"new IP should be new", args{&GeoInfo{IP: "1.1.1.2", FromDisk: false}, &GeoInfo{IP: "1.1.1.1", FromDisk: false}}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			currentGeoInfo.Set(tt.args.oldGeoInfo)
+			if got := isNew(tt.args.newGeoInfo); got != tt.want {
+				t.Errorf("isNew() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}