From 722073b52f76a7c7d6c53cb6e14edf24444681f0 Mon Sep 17 00:00:00 2001 From: LNKLEO Date: Wed, 29 May 2024 14:16:47 +0800 Subject: [PATCH] feat(owm): only use location for api call BREAKING CHANGE: location is now required instead of latitude and longitude --- src/segments/owm.go | 55 ++++---------------------- src/segments/owm_test.go | 73 ++++++++++------------------------- themes/schema.json | 12 ------ website/docs/segments/owm.mdx | 20 ++++------ 4 files changed, 35 insertions(+), 125 deletions(-) diff --git a/src/segments/owm.go b/src/segments/owm.go index bb2b36ac..e614ee03 100644 --- a/src/segments/owm.go +++ b/src/segments/owm.go @@ -28,10 +28,6 @@ const ( Location properties.Property = "location" // Units openweathermap units Units properties.Property = "units" - // Latitude for the location used in place of location - Latitude properties.Property = "latitude" - // Longitude for the location used in place of location - Longitude properties.Property = "longitude" // CacheKeyResponse key used when caching the response CacheKeyResponse string = "owm_response" // CacheKeyURL key used when caching the url responsible for the response @@ -54,11 +50,6 @@ type owmDataResponse struct { temperature `json:"main"` } -type geoLocation struct { - Lat float64 `json:"lat"` - Lon float64 `json:"lon"` -} - func (d *Owm) Enabled() bool { err := d.setStatus() @@ -77,65 +68,35 @@ func (d *Owm) Template() string { func (d *Owm) getResult() (*owmDataResponse, error) { cacheTimeout := d.props.GetInt(properties.CacheTimeout, properties.DefaultCacheTimeout) response := new(owmDataResponse) + if cacheTimeout > 0 { - // check if data stored in cache val, found := d.env.Cache().Get(CacheKeyResponse) - // we got something from te cache if found { err := json.Unmarshal([]byte(val), response) if err != nil { return nil, err } + d.URL, _ = d.env.Cache().Get(CacheKeyURL) return response, nil } } - apikey := properties.OneOf[string](d.props, ".", APIKey, "apiKey") + apikey := properties.OneOf(d.props, ".", APIKey, "apiKey") if len(apikey) == 0 { apikey = d.env.Getenv(PoshOWMAPIKey) } - if len(apikey) == 0 { - return nil, errors.New("no api key found") + location := d.props.GetString(Location, "De Bilt,NL") + + if len(apikey) == 0 || len(location) == 0 { + return nil, errors.New("no api key or location found") } - location := d.props.GetString(Location, "De Bilt,NL") - latitude := d.props.GetFloat64(Latitude, 91) // This default value is intentionally invalid since there should not be a default for this and 0 is a valid value - longitude := d.props.GetFloat64(Longitude, 181) // This default value is intentionally invalid since there should not be a default for this and 0 is a valid value units := d.props.GetString(Units, "standard") httpTimeout := d.props.GetInt(properties.HTTPTimeout, properties.DefaultHTTPTimeout) - validCoordinates := func(latitude, longitude float64) bool { - // Latitude values are only valid if they are between -90 and 90 - // Longitude values are only valid if they are between -180 and 180 - // https://gisgeography.com/latitude-longitude-coordinates/ - return latitude <= 90 && latitude >= -90 && longitude <= 180 && longitude >= -180 - } - - if !validCoordinates(latitude, longitude) { - var geoResponse []geoLocation - geocodingURL := fmt.Sprintf("http://api.openweathermap.org/geo/1.0/direct?q=%s&limit=1&appid=%s", location, apikey) - - body, err := d.env.HTTPRequest(geocodingURL, nil, httpTimeout) - if err != nil { - return new(owmDataResponse), err - } - - err = json.Unmarshal(body, &geoResponse) - if err != nil { - return new(owmDataResponse), err - } - - if len(geoResponse) == 0 { - return new(owmDataResponse), fmt.Errorf("no coordinates found for %s", location) - } - - latitude = geoResponse[0].Lat - longitude = geoResponse[0].Lon - } - - d.URL = fmt.Sprintf("http://api.openweathermap.org/data/2.5/weather?lat=%v&lon=%v&units=%s&appid=%s", latitude, longitude, units, apikey) + d.URL = fmt.Sprintf("http://api.openweathermap.org/data/2.5/weather?q=%s&units=%s&appid=%s", location, units, apikey) body, err := d.env.HTTPRequest(d.URL, nil, httpTimeout) if err != nil { diff --git a/src/segments/owm_test.go b/src/segments/owm_test.go index 6bc85888..01a96051 100644 --- a/src/segments/owm_test.go +++ b/src/segments/owm_test.go @@ -13,17 +13,13 @@ import ( ) const ( - OWMGEOAPIURL = "http://api.openweathermap.org/geo/1.0/direct?q=AMSTERDAM,NL&limit=1&appid=key" - OWMWEATHERAPIURL = "http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key" + OWMWEATHERAPIURL = "http://api.openweathermap.org/data/2.5/weather?q=%s&units=metric&appid=key" ) func TestOWMSegmentSingle(t *testing.T) { cases := []struct { Case string Location string - Latitude float64 - Longitude float64 - GeoJSONResponse string WeatherJSONResponse string ExpectedString string ExpectedEnabled bool @@ -33,7 +29,6 @@ func TestOWMSegmentSingle(t *testing.T) { { Case: "Sunny Display", Location: "AMSTERDAM,NL", - GeoJSONResponse: `[{"lat": 52.3727598,"lon": 4.8936041}]`, WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`, ExpectedString: "\ue30d (20°C)", ExpectedEnabled: true, @@ -41,7 +36,6 @@ func TestOWMSegmentSingle(t *testing.T) { { Case: "Sunny Display", Location: "AMSTERDAM,NL", - GeoJSONResponse: `[{"lat": 52.3727598,"lon": 4.8936041}]`, WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`, ExpectedString: "\ue30d (20°C)", ExpectedEnabled: true, @@ -50,7 +44,6 @@ func TestOWMSegmentSingle(t *testing.T) { { Case: "Sunny Display", Location: "AMSTERDAM,NL", - GeoJSONResponse: `[{"lat": 52.3727598,"lon": 4.8936041}]`, WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`, ExpectedString: "\ue30d", ExpectedEnabled: true, @@ -59,24 +52,18 @@ func TestOWMSegmentSingle(t *testing.T) { { Case: "Config Skip Geocoding Check With Location", Location: "AMSTERDAM,NL", - Latitude: 52.3727598, - Longitude: 4.8936041, WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`, ExpectedString: "\ue30d (20°C)", ExpectedEnabled: true, }, { Case: "Config Skip Geocoding Check Without Location", - Latitude: 52.3727598, - Longitude: 4.8936041, WeatherJSONResponse: `{"weather":[{"icon":"01d"}],"main":{"temp":20}}`, - ExpectedString: "\ue30d (20°C)", - ExpectedEnabled: true, + ExpectedEnabled: false, }, { Case: "Error in retrieving data", Location: "AMSTERDAM,NL", - GeoJSONResponse: "nonsense", WeatherJSONResponse: "nonsense", Error: errors.New("Something went wrong"), ExpectedEnabled: false, @@ -85,35 +72,15 @@ func TestOWMSegmentSingle(t *testing.T) { for _, tc := range cases { env := &mock.MockedEnvironment{} - var props properties.Map - if tc.Latitude != 0 && tc.Longitude != 0 && tc.Location != "" { //nolint: gocritic - props = properties.Map{ - APIKey: "key", - Location: tc.Location, - Latitude: tc.Latitude, - Longitude: tc.Longitude, - Units: "metric", - properties.CacheTimeout: 0, - } - } else if tc.Location != "" { - props = properties.Map{ - APIKey: "key", - Location: tc.Location, - Units: "metric", - properties.CacheTimeout: 0, - } - } else if tc.Latitude != 0 && tc.Longitude != 0 { - props = properties.Map{ - APIKey: "key", - Latitude: tc.Latitude, - Longitude: tc.Longitude, - Units: "metric", - properties.CacheTimeout: 0, - } + props := properties.Map{ + APIKey: "key", + Location: tc.Location, + Units: "metric", + properties.CacheTimeout: 0, } - env.On("HTTPRequest", OWMGEOAPIURL).Return([]byte(tc.GeoJSONResponse), tc.Error) - env.On("HTTPRequest", OWMWEATHERAPIURL).Return([]byte(tc.WeatherJSONResponse), tc.Error) + url := fmt.Sprintf(OWMWEATHERAPIURL, tc.Location) + env.On("HTTPRequest", url).Return([]byte(tc.WeatherJSONResponse), tc.Error) env.On("Error", mock2.Anything) o := &Owm{ @@ -232,7 +199,8 @@ func TestOWMSegmentIcons(t *testing.T) { ExpectedIconString: "\ue313", }, } - geoResponse := `[{"lat": 52.3727598,"lon": 4.8936041}]` + + url := fmt.Sprintf(OWMWEATHERAPIURL, "AMSTERDAM,NL") for _, tc := range cases { env := &mock.MockedEnvironment{} @@ -240,8 +208,7 @@ func TestOWMSegmentIcons(t *testing.T) { weatherResponse := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20.3}}`, tc.IconID) expectedString := fmt.Sprintf("%s (20°C)", tc.ExpectedIconString) - env.On("HTTPRequest", OWMGEOAPIURL).Return([]byte(geoResponse), nil) - env.On("HTTPRequest", OWMWEATHERAPIURL).Return([]byte(weatherResponse), nil) + env.On("HTTPRequest", url).Return([]byte(weatherResponse), nil) o := &Owm{ props: properties.Map{ @@ -262,10 +229,9 @@ func TestOWMSegmentIcons(t *testing.T) { env := &mock.MockedEnvironment{} weatherResponse := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20.3}}`, tc.IconID) - expectedString := fmt.Sprintf("«%s (20°C)»(http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key)", tc.ExpectedIconString) + expectedString := fmt.Sprintf("«%s (20°C)»(%s)", tc.ExpectedIconString, url) - env.On("HTTPRequest", OWMGEOAPIURL).Return([]byte(geoResponse), nil) - env.On("HTTPRequest", OWMWEATHERAPIURL).Return([]byte(weatherResponse), nil) + env.On("HTTPRequest", url).Return([]byte(weatherResponse), nil) o := &Owm{ props: properties.Map{ @@ -281,7 +247,8 @@ func TestOWMSegmentIcons(t *testing.T) { assert.Equal(t, expectedString, renderTemplate(env, "«{{.Weather}} ({{.Temperature}}{{.UnitIcon}})»({{.URL}})", o), tc.Case) } } -func TestOWMSegmentFromCache(t *testing.T) { + +func TestOWMSegmentFromCacheByGeoName(t *testing.T) { response := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20}}`, "01d") expectedString := fmt.Sprintf("%s (20°C)", "\ue30d") @@ -296,7 +263,7 @@ func TestOWMSegmentFromCache(t *testing.T) { env: env, } cache.On("Get", "owm_response").Return(response, true) - cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key", true) + cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?q=AMSTERDAM,NL&units=metric&appid=key", true) cache.On("Set").Return() env.On("Cache").Return(cache) @@ -304,9 +271,9 @@ func TestOWMSegmentFromCache(t *testing.T) { assert.Equal(t, expectedString, renderTemplate(env, o.Template(), o), "should return the cached response") } -func TestOWMSegmentFromCacheWithHyperlink(t *testing.T) { +func TestOWMSegmentFromCacheWithHyperlinkByGeoName(t *testing.T) { response := fmt.Sprintf(`{"weather":[{"icon":"%s"}],"main":{"temp":20}}`, "01d") - expectedString := fmt.Sprintf("«%s (20°C)»(http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key)", "\ue30d") + expectedString := fmt.Sprintf("«%s (20°C)»(http://api.openweathermap.org/data/2.5/weather?q=AMSTERDAM,NL&units=metric&appid=key)", "\ue30d") env := &mock.MockedEnvironment{} cache := &mock.MockedCache{} @@ -320,7 +287,7 @@ func TestOWMSegmentFromCacheWithHyperlink(t *testing.T) { env: env, } cache.On("Get", "owm_response").Return(response, true) - cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?lat=52.3727598&lon=4.8936041&units=metric&appid=key", true) + cache.On("Get", "owm_url").Return("http://api.openweathermap.org/data/2.5/weather?q=AMSTERDAM,NL&units=metric&appid=key", true) cache.On("Set").Return() env.On("Cache").Return(cache) diff --git a/themes/schema.json b/themes/schema.json index 833902be..72342d89 100644 --- a/themes/schema.json +++ b/themes/schema.json @@ -2952,18 +2952,6 @@ "description": "Location to use for the API call interpreted only if valid coordinates aren't given. Formatted as ,,. City name, state code and country code divided by comma. Please, refer to ISO 3166 for the state codes or country codes.", "default": "De Bilt,NL" }, - "latitude": { - "type": "number", - "title": "Latitude", - "description": "The latitude of the requested location, valid between -90 and 90", - "default": 91 - }, - "longitude": { - "type": "number", - "title": "Longitude", - "description": "The longitude of the requested location, valid between -180 and 180", - "default": 181 - }, "units": { "type": "string", "title": "units", diff --git a/website/docs/segments/owm.mdx b/website/docs/segments/owm.mdx index f755405f..be9ea8e9 100644 --- a/website/docs/segments/owm.mdx +++ b/website/docs/segments/owm.mdx @@ -37,19 +37,13 @@ import Config from "@site/src/components/Config.js"; ## Properties -| Name | Type | Default | Description | -| --------------- | :-------: | :----------: | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `api_key` | `string` | `.` | Your API key from [Open Weather Map][owm]. Can also be set using the `POSH_OWM_API_KEY` environment variable. | -| `latitude` | `float64` | `91` | The latitude of the requested location, valid between -90 and 90 | -| `longitude` | `float64` | `181` | The longitude of the requested location, valid between -180 and 180 | -| `location` | `string` | `De Bilt,NL` | The requested location interpreted only if valid coordinates aren't given. Formatted as \. City name, state code and country code divided by comma. Please, refer to ISO 3166 for the state codes or country codes | -| `units` | `string` | `standard` | Units of measurement. Available values are standard (kelvin), metric (celsius), and imperial (fahrenheit) | -| `http_timeout` | `int` | `20` | in milliseconds, the timeout for http request | -| `cache_timeout` | `int` | `10` | in minutes, the timeout for request caching. A value of 0 disables the cache. | - -### Specifying Location - -The given location can either be specified through the Latitude and Longitude properties or through the Location property. If both Latitude and Longitude are specified and valid then Location will be ignored if it is included. If Latitude or Longitude are not specified or are invalid then Location will be used. +| Name | Type | Default | Description | +| --------------- | :------: | :----------: | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `api_key` | `string` | `.` | Your API key from [Open Weather Map][owm]. Can also be set using the `POSH_OWM_API_KEY` environment variable. | +| `location` | `string` | `De Bilt,NL` | The requested location interpreted only if valid coordinates aren't given. Formatted as \. City name, state code and country code divided by comma. Please, refer to ISO 3166 for the state codes or country codes | +| `units` | `string` | `standard` | Units of measurement. Available values are standard (kelvin), metric (celsius), and imperial (fahrenheit) | +| `http_timeout` | `int` | `20` | in milliseconds, the timeout for http request | +| `cache_timeout` | `int` | `10` | in minutes, the timeout for request caching. A value of 0 disables the cache. | ## Template ([info][templates])