Skip to content

Commit

Permalink
feat(owm): only use location for api call
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: location is now required instead of latitude
and longitude
  • Loading branch information
@LNKLEO @JanDeDobbeleer
LNKLEO authored and JanDeDobbeleer committed May 29, 2024
1 parent a4a04ca commit 722073b
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 125 deletions.
55 changes: 8 additions & 47 deletions src/segments/owm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()

Expand All @@ -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")
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 len(apikey) == 0 || len(location) == 0 {
return nil, errors.New("no api key or location found")
}

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
}
units := d.props.GetString(Units, "standard")
httpTimeout := d.props.GetInt(properties.HTTPTimeout, properties.DefaultHTTPTimeout)

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 {
Expand Down
73 changes: 20 additions & 53 deletions src/segments/owm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -33,15 +29,13 @@ 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,
},
{
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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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{
Expand Down Expand Up @@ -232,16 +199,16 @@ 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{}

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{
Expand All @@ -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{
Expand All @@ -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")

Expand All @@ -296,17 +263,17 @@ 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)

assert.Nil(t, o.setStatus())
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{}
Expand All @@ -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)

Expand Down
12 changes: 0 additions & 12 deletions themes/schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2952,18 +2952,6 @@
"description": "Location to use for the API call interpreted only if valid coordinates aren't given. Formatted as <City>,<STATE>,<COUNTRY_CODE>. 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",
Expand Down
20 changes: 7 additions & 13 deletions website/docs/segments/owm.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,STATE,COUNTRY_CODE\>. 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,STATE,COUNTRY_CODE\>. 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])

Expand Down

0 comments on commit 722073b

Please sign in to comment.