fix(lookup): read the fields the providers were already sending
Audit of every field a lookup returns against what OpsLog can store, prompted by a report that HamQTH was not fetching the email. The email was never broken: pinned now against a captured HamQTH answer, it parses and reaches the QSO. The reported callsign simply has no public address on HamQTH, which the empty field could not distinguish from a fault — so the "[email protected]" placeholder is gone. A greyed-out sample address in the one field an operator checks to see whether the lookup found one reads as a value. Real gaps found and closed: - web: the qso table has had the column since migration 0003 and no provider mapping ever read it. HamQTH sends <web>. - picture: Result.ImageURL was documented "QRZ only" because HamQTH's element is <picture>, not <image>. It was there all along. - zip: sent by both providers, read by neither. - adr_name: used only as a fallback when a record carries neither nick nor name. Deliberately NOT preferred over <nick> — a log wants the name the operator goes by on the air, "Igor", not "Igor Vladimirovich Getman". The parse tests use a real captured payload, including the stray <div> advert the server injects into its own XML.
This commit is contained in:
@@ -0,0 +1,7 @@
|
||||
-- The lookup cache gains the two fields the providers were already sending and
|
||||
-- nothing was reading: the operator's own site, and the postal code.
|
||||
--
|
||||
-- `web` matters beyond the cache: the qso table has had a `web` column since
|
||||
-- 0003 and no lookup ever filled it.
|
||||
ALTER TABLE callsign_cache ADD COLUMN web TEXT;
|
||||
ALTER TABLE callsign_cache ADD COLUMN zip TEXT;
|
||||
@@ -94,6 +94,13 @@ func (h *HamQTH) fetch(ctx context.Context, sessionID, callsign string) (Result,
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
return parseHamQTHSearch(body)
|
||||
}
|
||||
|
||||
// parseHamQTHSearch turns one answer into a Result. Split out from the request
|
||||
// so the field mapping can be checked against a captured payload — which is how
|
||||
// the missing full name and picture were found.
|
||||
func parseHamQTHSearch(body []byte) (Result, error) {
|
||||
var resp hamqthRoot
|
||||
if err := xml.Unmarshal(body, &resp); err != nil {
|
||||
return Result{}, fmt.Errorf("hamqth: parse callsign: %w", err)
|
||||
@@ -112,11 +119,20 @@ func (h *HamQTH) fetch(ctx context.Context, sessionID, callsign string) (Result,
|
||||
if s.Callsign == "" {
|
||||
return Result{}, ErrNotFound
|
||||
}
|
||||
// <nick> is the name the operator goes BY on the air, which is what belongs
|
||||
// in a log — "Igor", not "Igor Vladimirovich Getman". adr_name is the postal
|
||||
// name and is used only when there is nothing else, so a record carrying
|
||||
// neither nick nor name no longer resolves to blank.
|
||||
name := strings.TrimSpace(s.Nick + " " + s.LastName)
|
||||
if name == "" {
|
||||
name = strings.TrimSpace(s.AdrName)
|
||||
}
|
||||
r := Result{
|
||||
Callsign: strings.ToUpper(s.Callsign),
|
||||
Name: strings.TrimSpace(s.Nick + " " + s.LastName),
|
||||
Name: name,
|
||||
QTH: firstNonEmpty(s.QTH, s.AdrCity),
|
||||
Address: s.AdrStreet1,
|
||||
Zip: s.AdrZip,
|
||||
State: strings.ToUpper(s.USState),
|
||||
County: s.USCounty,
|
||||
Country: firstNonEmpty(s.AdrCountry, s.Country),
|
||||
@@ -124,6 +140,8 @@ func (h *HamQTH) fetch(ctx context.Context, sessionID, callsign string) (Result,
|
||||
Continent: strings.ToUpper(s.Continent),
|
||||
Email: s.Email,
|
||||
QSLVia: s.QSLVia,
|
||||
Web: s.Web,
|
||||
ImageURL: s.Picture,
|
||||
}
|
||||
r.Lat, _ = strconv.ParseFloat(s.Latitude, 64)
|
||||
r.Lon, _ = strconv.ParseFloat(s.Longitude, 64)
|
||||
@@ -182,4 +200,14 @@ type hamqthSearch struct {
|
||||
Continent string `xml:"continent"`
|
||||
Email string `xml:"email"`
|
||||
QSLVia string `xml:"qsl_via"`
|
||||
// AdrName is the full postal name. Many records carry it and no <name> at
|
||||
// all, so building the name from <nick> + <name> silently kept the first
|
||||
// name and dropped the rest.
|
||||
AdrName string `xml:"adr_name"`
|
||||
AdrZip string `xml:"adr_zip"`
|
||||
// Picture is HamQTH's profile photo. Result.ImageURL was documented as "QRZ
|
||||
// only" because this element was never read — QRZ calls the same thing
|
||||
// <image>.
|
||||
Picture string `xml:"picture"`
|
||||
Web string `xml:"web"`
|
||||
}
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
package lookup
|
||||
|
||||
import (
|
||||
"encoding/xml"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// A real HamQTH answer, captured from the live API.
|
||||
//
|
||||
// Two things it settles. The email IS returned and must reach the QSO. And the
|
||||
// element names are not the ones a reading of the documentation suggests:
|
||||
// there is no <name> here at all, the full name is <adr_name>, and the picture
|
||||
// is <picture> where QRZ calls it <image>. The stray <div> is an advert the
|
||||
// server injects into its own XML; the parser has to shrug it off.
|
||||
const hamqthEU1EU = `<?xml version="1.0"?>
|
||||
<HamQTH xmlns="https://www.hamqth.com" version="2.8">
|
||||
<div id="in-page-channel-node-id" data-channel-name="in_page_channel_Sux7_K"/>
|
||||
<search>
|
||||
<callsign>eu1eu</callsign>
|
||||
<nick>Igor</nick>
|
||||
<qth>Minsk-5</qth>
|
||||
<country>Belarus</country>
|
||||
<adif>27</adif>
|
||||
<itu>29</itu>
|
||||
<cq>16</cq>
|
||||
<grid>KO33SV</grid>
|
||||
<adr_name>Igor Vladimirovich Getman</adr_name>
|
||||
<adr_street1>A/ya 143</adr_street1>
|
||||
<adr_city>Minsk-5</adr_city>
|
||||
<adr_zip>220005</adr_zip>
|
||||
<adr_country>Belarus</adr_country>
|
||||
<adr_adif>27</adr_adif>
|
||||
<district>WAARB-LE</district>
|
||||
<lotw>?</lotw>
|
||||
<qsldirect>?</qsldirect>
|
||||
<qsl>?</qsl>
|
||||
<eqsl>Y</eqsl>
|
||||
<email>[email protected]</email>
|
||||
<latitude>53.88999938964844</latitude>
|
||||
<longitude>27.59000015258789</longitude>
|
||||
<continent>EU</continent>
|
||||
<utc_offset>-2</utc_offset>
|
||||
<picture>https://www.hamqth.com/images/default/ts-930_qith_old_radios.jpg</picture>
|
||||
</search>
|
||||
</HamQTH>`
|
||||
|
||||
func TestHamQTHParsesEveryUsefulField(t *testing.T) {
|
||||
var root hamqthRoot
|
||||
if err := xml.Unmarshal([]byte(hamqthEU1EU), &root); err != nil {
|
||||
t.Fatalf("parse: %v", err)
|
||||
}
|
||||
s := root.Search
|
||||
|
||||
for _, c := range []struct{ name, got, want string }{
|
||||
{"callsign", s.Callsign, "eu1eu"},
|
||||
{"email", s.Email, "[email protected]"},
|
||||
{"grid", s.Grid, "KO33SV"},
|
||||
{"qth", s.QTH, "Minsk-5"},
|
||||
{"street", s.AdrStreet1, "A/ya 143"},
|
||||
{"city", s.AdrCity, "Minsk-5"},
|
||||
{"country", s.AdrCountry, "Belarus"},
|
||||
{"continent", s.Continent, "EU"},
|
||||
{"dxcc", s.DXCC, "27"},
|
||||
{"cq", s.CQ, "16"},
|
||||
{"itu", s.ITU, "29"},
|
||||
} {
|
||||
if c.got != c.want {
|
||||
t.Errorf("%s = %q, want %q", c.name, c.got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The fields the parser was blind to. Each is data OpsLog has somewhere to put.
|
||||
func TestHamQTHParsesTheFieldsThatWereMissing(t *testing.T) {
|
||||
var root hamqthRoot
|
||||
if err := xml.Unmarshal([]byte(hamqthEU1EU), &root); err != nil {
|
||||
t.Fatalf("parse: %v", err)
|
||||
}
|
||||
s := root.Search
|
||||
|
||||
// The postal name, the fallback when a record carries no nick and no name.
|
||||
if s.AdrName != "Igor Vladimirovich Getman" {
|
||||
t.Errorf("adr_name = %q", s.AdrName)
|
||||
}
|
||||
if s.LastName != "" {
|
||||
t.Errorf("this record has no <name>; got %q", s.LastName)
|
||||
}
|
||||
// The profile picture. Result.ImageURL existed and said "QRZ only" — HamQTH
|
||||
// sends one too, under a different element name.
|
||||
if s.Picture == "" {
|
||||
t.Error("picture not parsed")
|
||||
}
|
||||
if s.AdrZip != "220005" {
|
||||
t.Errorf("adr_zip = %q", s.AdrZip)
|
||||
}
|
||||
}
|
||||
|
||||
// End to end: what the provider hands back must carry the email, the full name
|
||||
// and the picture.
|
||||
func TestHamQTHResultCarriesTheLot(t *testing.T) {
|
||||
r, err := parseHamQTHSearch([]byte(hamqthEU1EU))
|
||||
if err != nil {
|
||||
t.Fatalf("parse: %v", err)
|
||||
}
|
||||
if r.Email != "[email protected]" {
|
||||
t.Errorf("Email = %q — this is what was reported missing", r.Email)
|
||||
}
|
||||
// The nick, not the postal name: a log wants the name the operator goes by
|
||||
// on the air. adr_name is only the fallback when there is nothing else.
|
||||
if r.Name != "Igor" {
|
||||
t.Errorf("Name = %q, want the on-air nick", r.Name)
|
||||
}
|
||||
if r.ImageURL == "" {
|
||||
t.Error("ImageURL empty — HamQTH sent a picture")
|
||||
}
|
||||
if r.Grid != "KO33SV" {
|
||||
t.Errorf("Grid = %q", r.Grid)
|
||||
}
|
||||
if r.Lat < 53.8 || r.Lat > 53.9 {
|
||||
t.Errorf("Lat = %v", r.Lat)
|
||||
}
|
||||
if r.DXCC != 27 || r.CQZ != 16 || r.ITUZ != 29 {
|
||||
t.Errorf("dxcc/cq/itu = %d/%d/%d", r.DXCC, r.CQZ, r.ITUZ)
|
||||
}
|
||||
if r.Address != "A/ya 143" {
|
||||
t.Errorf("Address = %q", r.Address)
|
||||
}
|
||||
}
|
||||
@@ -37,7 +37,13 @@ type Result struct {
|
||||
Continent string `json:"cont,omitempty"`
|
||||
Email string `json:"email,omitempty"`
|
||||
QSLVia string `json:"qsl_via,omitempty"`
|
||||
ImageURL string `json:"image_url,omitempty"` // profile picture URL (QRZ only for now)
|
||||
// Web is the operator's own site. The QSO table has had a `web` column all
|
||||
// along and nothing ever filled it, because no provider mapping read the
|
||||
// field.
|
||||
Web string `json:"web,omitempty"`
|
||||
// Zip is the postal code. HamQTH and QRZ both send one.
|
||||
Zip string `json:"zip,omitempty"`
|
||||
ImageURL string `json:"image_url,omitempty"` // profile picture URL
|
||||
Source string `json:"source"` // "qrz", "hamqth", or "cache"
|
||||
FetchedAt time.Time `json:"fetched_at"`
|
||||
}
|
||||
@@ -446,12 +452,13 @@ func (c *Cache) Get(ctx context.Context, callsign string) (Result, bool) {
|
||||
row := c.db.QueryRowContext(ctx, `
|
||||
SELECT callsign, name, qth, address, state, cnty, country, grid,
|
||||
lat, lon, dxcc, cqz, ituz, cont, email, qsl_via, image_url,
|
||||
source, fetched_at
|
||||
web, zip, source, fetched_at
|
||||
FROM callsign_cache WHERE callsign = ?`, callsign)
|
||||
var (
|
||||
r Result
|
||||
name, qth, addr, state, cnty sql.NullString
|
||||
country, grid, cont, email, qslVia, image sql.NullString
|
||||
web, zip sql.NullString
|
||||
src string
|
||||
dxcc, cqz, ituz sql.NullInt64
|
||||
lat, lon sql.NullFloat64
|
||||
@@ -459,7 +466,7 @@ func (c *Cache) Get(ctx context.Context, callsign string) (Result, bool) {
|
||||
)
|
||||
if err := row.Scan(&r.Callsign, &name, &qth, &addr, &state, &cnty,
|
||||
&country, &grid, &lat, &lon,
|
||||
&dxcc, &cqz, &ituz, &cont, &email, &qslVia, &image,
|
||||
&dxcc, &cqz, &ituz, &cont, &email, &qslVia, &image, &web, &zip,
|
||||
&src, &fetched); err != nil {
|
||||
return Result{}, false
|
||||
}
|
||||
@@ -481,6 +488,8 @@ func (c *Cache) Get(ctx context.Context, callsign string) (Result, bool) {
|
||||
r.Lon = lon.Float64
|
||||
r.Continent = cont.String
|
||||
r.Email = email.String
|
||||
r.Web = web.String
|
||||
r.Zip = zip.String
|
||||
r.QSLVia = qslVia.String
|
||||
r.ImageURL = image.String
|
||||
r.DXCC = int(dxcc.Int64)
|
||||
@@ -497,7 +506,7 @@ func (c *Cache) Put(ctx context.Context, r Result) error {
|
||||
updateCols := []string{
|
||||
"name", "qth", "address", "state", "cnty",
|
||||
"country", "grid", "lat", "lon",
|
||||
"dxcc", "cqz", "ituz", "cont", "email", "qsl_via", "image_url",
|
||||
"dxcc", "cqz", "ituz", "cont", "email", "qsl_via", "image_url", "web", "zip",
|
||||
"source", "fetched_at",
|
||||
}
|
||||
// The lookup cache always lives in the local SQLite database, so SQLite
|
||||
@@ -510,8 +519,8 @@ func (c *Cache) Put(ctx context.Context, r Result) error {
|
||||
INSERT INTO callsign_cache(callsign, name, qth, address, state, cnty,
|
||||
country, grid, lat, lon,
|
||||
dxcc, cqz, ituz, cont, email, qsl_via, image_url,
|
||||
source, fetched_at)
|
||||
VALUES(?,?,?,?,?,?, ?,?,?,?, ?,?,?,?,?,?,?, ?,?)
|
||||
web, zip, source, fetched_at)
|
||||
VALUES(?,?,?,?,?,?, ?,?,?,?, ?,?,?,?,?,?,?, ?,?, ?,?)
|
||||
ON CONFLICT(callsign) DO UPDATE SET ` + strings.Join(sets, ", ")
|
||||
_, err := c.db.ExecContext(ctx, q,
|
||||
r.Callsign, nullable(r.Name), nullable(r.QTH), nullable(r.Address),
|
||||
@@ -520,7 +529,7 @@ func (c *Cache) Put(ctx context.Context, r Result) error {
|
||||
nullableFloat(r.Lat), nullableFloat(r.Lon),
|
||||
nullableInt(r.DXCC), nullableInt(r.CQZ), nullableInt(r.ITUZ),
|
||||
nullable(r.Continent), nullable(r.Email), nullable(r.QSLVia),
|
||||
nullable(r.ImageURL),
|
||||
nullable(r.ImageURL), nullable(r.Web), nullable(r.Zip),
|
||||
r.Source, db.NowISO(),
|
||||
)
|
||||
return err
|
||||
|
||||
Reference in New Issue
Block a user