diff --git a/src/http/Server.zig b/src/http/Server.zig index f71bb1c..5ea0c49 100644 --- a/src/http/Server.zig +++ b/src/http/Server.zig @@ -242,7 +242,7 @@ test "handleWeather: default endpoint uses IP address" { try ht.expectStatus(200); try ht.expectBody( - \\
Weather report: 73.158.64.1
+        \\
Weather report: Union City, California, United States
         \\
         \\    \   /         Clear
         \\     .-.          +68(+68) °F
diff --git a/src/http/handler.zig b/src/http/handler.zig
index 41f6528..479b6d3 100644
--- a/src/http/handler.zig
+++ b/src/http/handler.zig
@@ -372,7 +372,7 @@ test "handler: format v2" {
     try ht.expectStatus(200);
     // Should we have 2 empty lines?
     try ht.expectBody(
-        \\Weather report: 73.158.64.1
+        \\Weather report: Union City, California, United States
         \\
         \\      Current conditions
         \\      Clear
@@ -467,5 +467,5 @@ test "handler: format line 3" {
     try handleWeather(&harness.opts, ht.req, ht.res, client_ip);
 
     try ht.expectStatus(200);
-    try ht.expectBody("Test: ☀️ +20°C");
+    try ht.expectBody("Union City, California, United States: ☀️ +20°C");
 }
diff --git a/src/location/GeoIp.zig b/src/location/GeoIp.zig
index ae4f4f7..accaddf 100644
--- a/src/location/GeoIp.zig
+++ b/src/location/GeoIp.zig
@@ -1,6 +1,6 @@
 const std = @import("std");
-const Coordinates = @import("../Coordinates.zig");
 const Ip2location = @import("Ip2location.zig");
+const Location = @import("resolver.zig").Location;
 
 const c = @cImport({
     @cInclude("maxminddb.h");
@@ -52,7 +52,7 @@ pub fn deinit(self: *GeoIP) void {
     self.allocator.destroy(self.ip2location_client);
 }
 
-pub fn lookup(self: *GeoIP, ip: []const u8) ?Coordinates {
+pub fn lookup(self: *GeoIP, ip: []const u8) ?Location {
     // Try MaxMind first
     const result = lookupInternal(self.mmdb, ip) catch return null;
 
@@ -97,9 +97,7 @@ pub fn isUSIp(self: *GeoIP, ip: []const u8) bool {
     return std.mem.eql(u8, country_code, "US");
 }
 
-fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result_s) ?Coordinates {
-    _ = self;
-
+fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result_s) ?Location {
     if (!result.found_entry) return null;
 
     var entry_copy = result.entry;
@@ -133,9 +131,43 @@ fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result
     const latitude = coords[0];
     const longitude = coords[1];
 
+    // Extract location name parts
+    // SAFETY: value set by MMDB_get_value
+    var city_data: c.MMDB_entry_data_s = undefined;
+    entry_copy = result.entry;
+    const city_status = c.MMDB_get_value(&entry_copy, &city_data, "city", "names", "en", @as([*c]const u8, null));
+    const city = if (city_status == c.MMDB_SUCCESS and city_data.has_data)
+        city_data.unnamed_0.utf8_string[0..city_data.data_size]
+    else
+        "";
+
+    // SAFETY: value set by MMDB_get_value
+    var subdivision_data: c.MMDB_entry_data_s = undefined;
+    entry_copy = result.entry;
+    const subdivision_status = c.MMDB_get_value(&entry_copy, &subdivision_data, "subdivisions", "0", "names", "en", @as([*c]const u8, null));
+    const subdivision = if (subdivision_status == c.MMDB_SUCCESS and subdivision_data.has_data)
+        subdivision_data.unnamed_0.utf8_string[0..subdivision_data.data_size]
+    else
+        "";
+
+    // SAFETY: value set by MMDB_get_value
+    var country_data: c.MMDB_entry_data_s = undefined;
+    entry_copy = result.entry;
+    const country_status = c.MMDB_get_value(&entry_copy, &country_data, "country", "names", "en", @as([*c]const u8, null));
+    const country = if (country_status == c.MMDB_SUCCESS and country_data.has_data)
+        country_data.unnamed_0.utf8_string[0..country_data.data_size]
+    else
+        "";
+
+    const final_name = Location.buildDisplayName(self.allocator, city, subdivision, country, ip);
+
     return .{
-        .latitude = latitude,
-        .longitude = longitude,
+        .allocator = self.allocator,
+        .name = final_name,
+        .coords = .{
+            .latitude = latitude,
+            .longitude = longitude,
+        },
     };
 }
 
@@ -191,10 +223,11 @@ test "lookup works" {
     defer geoip.deinit();
 
     // Test that the function doesn't crash with various IPs
-    const maybe_coords = geoip.lookup("73.158.64.1");
+    const maybe_result = geoip.lookup("73.158.64.1");
 
-    try std.testing.expect(maybe_coords != null);
+    try std.testing.expect(maybe_result != null);
 
-    const coords = maybe_coords.?;
-    try std.testing.expectEqual(@as(f64, 37.5958), coords.latitude);
+    const result = maybe_result.?;
+    defer result.deinit();
+    try std.testing.expectEqual(@as(f64, 37.5958), result.coords.latitude);
 }
diff --git a/src/location/Ip2location.zig b/src/location/Ip2location.zig
index df58e9c..32df3db 100644
--- a/src/location/Ip2location.zig
+++ b/src/location/Ip2location.zig
@@ -1,6 +1,6 @@
 const std = @import("std");
 const Allocator = std.mem.Allocator;
-const Coordinates = @import("../Coordinates.zig");
+const Location = @import("resolver.zig").Location;
 
 const Self = @This();
 
@@ -31,7 +31,7 @@ pub fn deinit(self: *Self) void {
         self.allocator.free(k);
 }
 
-pub fn lookup(self: *Self, ip_str: []const u8) ?Coordinates {
+pub fn lookup(self: *Self, ip_str: []const u8) ?Location {
     // Parse IP to u128 for cache lookup
     const addr = std.net.Address.parseIp(ip_str, 0) catch return null;
     const ip_u128: u128 = switch (addr.any.family) {
@@ -42,24 +42,24 @@ pub fn lookup(self: *Self, ip_str: []const u8) ?Coordinates {
     const family: u8 = if (addr.any.family == std.posix.AF.INET) 4 else 6;
 
     // Check cache first
-    if (self.cache.get(ip_u128)) |coords|
-        return coords;
+    if (self.cache.get(ip_u128)) |result|
+        return result;
 
     // Fetch from API
-    const coords = self.fetch(ip_str) catch |err| {
+    const result = self.fetch(ip_str) catch |err| {
         log.err("API lookup failed: {}", .{err});
         return null;
     };
 
     // Store in cache
-    self.cache.put(ip_u128, family, coords) catch |err| {
+    self.cache.put(ip_u128, family, result) catch |err| {
         log.warn("Failed to cache result: {}", .{err});
     };
 
-    return coords;
+    return result;
 }
 
-fn fetch(self: *Self, ip_str: []const u8) !Coordinates {
+fn fetch(self: *Self, ip_str: []const u8) !Location {
     log.info("Fetching geolocation for IP {s}", .{ip_str});
 
     if (@import("builtin").is_test) return error.LookupUnavailableInUnitTest;
@@ -110,37 +110,47 @@ fn fetch(self: *Self, ip_str: []const u8) !Coordinates {
             "Longitude returned from ip2location.io for ip {s} is not a float: {f}",
             .{ ip_str, std.json.fmt(lon, .{}) },
         );
-    return Coordinates{
-        .latitude = @floatCast(lat.float),
-        .longitude = @floatCast(lon.float),
+
+    const city = getString(obj, "city_name");
+    const region = getString(obj, "region_name");
+    const country = getString(obj, "country_name");
+
+    const display_name = Location.buildDisplayName(
+        self.allocator,
+        city,
+        region,
+        country,
+        ip_str,
+    );
+
+    return Location{
+        .allocator = self.allocator,
+        .name = display_name,
+        .coords = .{
+            .latitude = @floatCast(lat.float),
+            .longitude = @floatCast(lon.float),
+        },
     };
 }
-const CacheEntry = packed struct {
-    family: u8, // 4 or 6
-    _pad0: u8 = 0,
-    _pad1: u8 = 0,
-    _pad2: u8 = 0,
-    _pad3: u8 = 0,
-    _pad4: u8 = 0,
-    _pad5: u8 = 0,
-    _pad6: u8 = 0,
-    ip: u128, // 16 bytes (IPv4 in lower 32 bits)
-    lat: f32, // 4 bytes
-    lon: f32, // 4 bytes
-    // Total: 32 bytes per record
-};
+
+inline fn getString(obj: std.json.ObjectMap, key: []const u8) []const u8 {
+    const maybe_val = obj.get(key);
+    if (maybe_val == null) return "";
+    if (maybe_val.? != .string) return "";
+    return maybe_val.?.string;
+}
 
 pub const Cache = struct {
     allocator: Allocator,
     path: []const u8,
-    entries: std.AutoHashMap(u128, Coordinates),
+    entries: std.AutoHashMap(u128, Location),
     file: ?std.fs.File,
 
     pub fn init(allocator: Allocator, path: []const u8) !Cache {
         var cache = Cache{
             .allocator = allocator,
             .path = try allocator.dupe(u8, path),
-            .entries = std.AutoHashMap(u128, Coordinates).init(allocator),
+            .entries = std.AutoHashMap(u128, Location).init(allocator),
             .file = null,
         };
         errdefer allocator.free(cache.path);
@@ -155,6 +165,8 @@ pub const Cache = struct {
                 const dir = std.fs.path.dirname(path) orelse return error.InvalidPath;
                 try std.fs.cwd().makePath(dir);
                 cache.file = try std.fs.createFileAbsolute(path, .{ .read = true });
+                // Write header
+                try cache.file.?.writeAll("#Ip2location:v2\n");
             },
             else => return err,
         }
@@ -164,6 +176,10 @@ pub const Cache = struct {
 
     pub fn deinit(self: *Cache) void {
         if (self.file) |f| f.close();
+        var it = self.entries.valueIterator();
+        while (it.next()) |loc| {
+            self.allocator.free(loc.name);
+        }
         self.entries.deinit();
         self.allocator.free(self.path);
     }
@@ -173,36 +189,146 @@ pub const Cache = struct {
         const file_size = try file.getEndPos();
         if (file_size == 0) return;
 
-        const bytes = try file.readToEndAlloc(self.allocator, file_size);
-        defer self.allocator.free(bytes);
+        const content = try file.readToEndAlloc(self.allocator, file_size);
+        defer self.allocator.free(content);
 
-        const entries = std.mem.bytesAsSlice(CacheEntry, bytes);
-        for (entries) |entry| {
-            try self.entries.put(entry.ip, .{
-                .latitude = entry.lat,
-                .longitude = entry.lon,
-            });
+        var lines = std.mem.splitScalar(u8, content, '\n');
+
+        // Check for header magic string
+        if (lines.next()) |first_line| {
+            if (!std.mem.eql(u8, first_line, "#Ip2location:v2")) {
+                log.warn("Cache file missing or invalid header, discarding", .{});
+                return;
+            }
+        } else {
+            return; // Empty file
+        }
+
+        while (lines.next()) |line| {
+            if (line.len == 0) continue;
+            const entry = parseCacheLine(self.allocator, line) catch continue;
+            try self.entries.put(entry.ip, entry.location);
         }
     }
 
-    pub fn get(self: *Cache, ip: u128) ?Coordinates {
+    const CacheEntry = struct {
+        ip: u128,
+        location: Location,
+    };
+
+    fn parseCacheLine(allocator: Allocator, line: []const u8) !CacheEntry {
+        // Parse: ip,lat,lon,name
+        var parts = std.mem.splitScalar(u8, line, ',');
+        const ip_str = parts.next() orelse return error.InvalidFormat;
+        const lat_str = parts.next() orelse return error.InvalidFormat;
+        const lon_str = parts.next() orelse return error.InvalidFormat;
+        const name = parts.rest();
+
+        const lat = try std.fmt.parseFloat(f64, lat_str);
+        const lon = try std.fmt.parseFloat(f64, lon_str);
+
+        // Parse IP to u128
+        const addr = try std.net.Address.parseIp(ip_str, 0);
+        const ip_u128: u128 = switch (addr.any.family) {
+            std.posix.AF.INET => @as(u128, @intCast(std.mem.readInt(u32, @ptrCast(&addr.in.sa.addr), .big))),
+            std.posix.AF.INET6 => std.mem.readInt(u128, @ptrCast(&addr.in6.sa.addr), .big),
+            else => return error.InvalidIpFamily,
+        };
+
+        const name_copy = try allocator.dupe(u8, name);
+        return .{
+            .ip = ip_u128,
+            .location = .{
+                .allocator = allocator,
+                .name = name_copy,
+                .coords = .{ .latitude = lat, .longitude = lon },
+            },
+        };
+    }
+
+    pub fn get(self: *Cache, ip: u128) ?Location {
         return self.entries.get(ip);
     }
 
-    pub fn put(self: *Cache, ip: u128, family: u8, coords: Coordinates) !void {
-        // Add to in-memory map
-        try self.entries.put(ip, coords);
+    pub fn put(self: *Cache, ip: u128, _: u8, loc: Location) !void {
+        const name_copy = try self.allocator.dupe(u8, loc.name);
+        try self.entries.put(ip, .{
+            .allocator = self.allocator,
+            .name = name_copy,
+            .coords = loc.coords,
+        });
 
-        // Append to file
+        // Append to file: ip,lat,lon,name
         if (self.file) |file| {
-            const entry = CacheEntry{
-                .family = family,
-                .ip = ip,
-                .lat = @floatCast(coords.latitude),
-                .lon = @floatCast(coords.longitude),
-            };
             try file.seekFromEnd(0);
-            try file.writeAll(std.mem.asBytes(&entry));
+            // Format IP as string for file
+            var buf: [39]u8 = undefined;
+            const ip_str = try std.fmt.bufPrint(&buf, "{}", .{ip});
+            const line = try std.fmt.allocPrint(self.allocator, "{s},{d},{d},{s}\n", .{
+                ip_str,
+                loc.coords.latitude,
+                loc.coords.longitude,
+                loc.name,
+            });
+            defer self.allocator.free(line);
+            try file.writeAll(line);
         }
     }
 };
+
+test "parseCacheLine: valid IPv4 line" {
+    const allocator = std.testing.allocator;
+    const line = "192.168.1.1,37.5,-122.5,San Francisco, California, United States";
+    const entry = try Cache.parseCacheLine(allocator, line);
+    defer allocator.free(entry.location.name);
+
+    try std.testing.expectEqual(@as(u128, 3232235777), entry.ip); // 192.168.1.1 as u128
+    try std.testing.expectEqual(@as(f64, 37.5), entry.location.coords.latitude);
+    try std.testing.expectEqual(@as(f64, -122.5), entry.location.coords.longitude);
+    try std.testing.expectEqualStrings("San Francisco, California, United States", entry.location.name);
+}
+
+test "parseCacheLine: valid IPv6 line" {
+    const allocator = std.testing.allocator;
+    const line = "2001:db8::1,51.5,-0.1,London, United Kingdom";
+    const entry = try Cache.parseCacheLine(allocator, line);
+    defer allocator.free(entry.location.name);
+
+    try std.testing.expect(entry.ip > 0);
+    try std.testing.expectEqual(@as(f64, 51.5), entry.location.coords.latitude);
+    try std.testing.expectEqual(@as(f64, -0.1), entry.location.coords.longitude);
+    try std.testing.expectEqualStrings("London, United Kingdom", entry.location.name);
+}
+
+test "parseCacheLine: empty name" {
+    const allocator = std.testing.allocator;
+    const line = "10.0.0.1,0.0,0.0,";
+    const entry = try Cache.parseCacheLine(allocator, line);
+    defer allocator.free(entry.location.name);
+
+    try std.testing.expectEqualStrings("", entry.location.name);
+}
+
+test "parseCacheLine: missing fields" {
+    const allocator = std.testing.allocator;
+    const line = "192.168.1.1,37.5";
+    try std.testing.expectError(error.InvalidFormat, Cache.parseCacheLine(allocator, line));
+}
+
+test "parseCacheLine: invalid IP" {
+    const allocator = std.testing.allocator;
+    const line = "not.an.ip,37.5,-122.5,Test";
+    try std.testing.expectError(error.InvalidIPAddressFormat, Cache.parseCacheLine(allocator, line));
+}
+
+test "parseCacheLine: invalid latitude" {
+    const allocator = std.testing.allocator;
+    const line = "192.168.1.1,invalid,-122.5,Test";
+    try std.testing.expectError(error.InvalidCharacter, Cache.parseCacheLine(allocator, line));
+}
+
+test "parseCacheLine: invalid longitude" {
+    const allocator = std.testing.allocator;
+    const line = "192.168.1.1,37.5,invalid,Test";
+    try std.testing.expectError(error.InvalidCharacter, Cache.parseCacheLine(allocator, line));
+}
diff --git a/src/location/resolver.zig b/src/location/resolver.zig
index 48afc6d..4c36056 100644
--- a/src/location/resolver.zig
+++ b/src/location/resolver.zig
@@ -14,6 +14,47 @@ pub const Location = struct {
     pub fn deinit(self: Location) void {
         self.allocator.free(self.name);
     }
+
+    /// Build a display name from city, subdivision (state/province), and country
+    /// Returns allocated string that must be freed by caller
+    pub fn buildDisplayName(allocator: std.mem.Allocator, city: []const u8, subdivision: []const u8, country: []const u8, fallback: []const u8) []const u8 {
+        var name_buf: [1024]u8 = undefined;
+        var w = std.Io.Writer.fixed(&name_buf);
+        var oom = false;
+
+        if (city.len > 0) {
+            w.writeAll(city) catch {
+                oom = true;
+            };
+            if (w.buffered().len > 0 and subdivision.len > 0) w.print(", {s}", .{subdivision}) catch {
+                oom = true;
+            };
+            if (w.buffered().len > 0 and country.len > 0) w.print(", {s}", .{country}) catch {
+                oom = true;
+            };
+        } else {
+            if (subdivision.len > 0) {
+                w.writeAll(subdivision) catch {
+                    oom = true;
+                };
+                if (w.buffered().len > 0 and country.len > 0) w.print(", {s}", .{country}) catch {
+                    oom = true;
+                };
+            } else {
+                if (country.len > 0)
+                    w.writeAll(country) catch {
+                        oom = true;
+                    }
+                else
+                    w.writeAll(fallback) catch {
+                        oom = true;
+                    };
+            }
+        }
+
+        if (oom) log.err("Location data overflowed buffer for fallback: {s}", .{fallback});
+        return allocator.dupe(u8, w.buffered()) catch @panic("OOM");
+    }
 };
 
 pub const LocationType = enum {
@@ -76,13 +117,8 @@ pub const Resolver = struct {
 
     fn resolveIP(self: *Resolver, ip: []const u8) !Location {
         if (self.geoip) |geoip| {
-            if (geoip.lookup(ip)) |coords| {
-                return .{
-                    .allocator = self.allocator,
-                    .name = try self.allocator.dupe(u8, ip),
-                    .coords = coords,
-                };
-            }
+            if (geoip.lookup(ip)) |result|
+                return result;
         }
         return error.LocationNotFound;
     }
@@ -282,6 +318,48 @@ test "resolve IP address with GeoIP" {
     const location = try resolver.resolve(test_ip);
     defer location.deinit();
 
-    try std.testing.expectEqualStrings(test_ip, location.name);
+    try std.testing.expectEqualStrings("Union City, California, United States", location.name);
     try std.testing.expect(location.coords.latitude != 0 or location.coords.longitude != 0);
 }
+
+test "buildDisplayName: city + state + country" {
+    const allocator = std.testing.allocator;
+    const name = Location.buildDisplayName(allocator, "Palo Alto", "California", "United States", "fallback");
+    defer allocator.free(name);
+    try std.testing.expectEqualStrings("Palo Alto, California, United States", name);
+}
+
+test "buildDisplayName: city + country (no state)" {
+    const allocator = std.testing.allocator;
+    const name = Location.buildDisplayName(allocator, "London", "", "United Kingdom", "fallback");
+    defer allocator.free(name);
+    try std.testing.expectEqualStrings("London, United Kingdom", name);
+}
+
+test "buildDisplayName: state + country (no city)" {
+    const allocator = std.testing.allocator;
+    const name = Location.buildDisplayName(allocator, "", "California", "United States", "fallback");
+    defer allocator.free(name);
+    try std.testing.expectEqualStrings("California, United States", name);
+}
+
+test "buildDisplayName: country only" {
+    const allocator = std.testing.allocator;
+    const name = Location.buildDisplayName(allocator, "", "", "United States", "fallback");
+    defer allocator.free(name);
+    try std.testing.expectEqualStrings("United States", name);
+}
+
+test "buildDisplayName: fallback when all empty" {
+    const allocator = std.testing.allocator;
+    const name = Location.buildDisplayName(allocator, "", "", "", "192.168.1.1");
+    defer allocator.free(name);
+    try std.testing.expectEqualStrings("192.168.1.1", name);
+}
+
+test "buildDisplayName: city only (no state or country)" {
+    const allocator = std.testing.allocator;
+    const name = Location.buildDisplayName(allocator, "Paris", "", "", "fallback");
+    defer allocator.free(name);
+    try std.testing.expectEqualStrings("Paris", name);
+}
diff --git a/src/render/Line.zig b/src/render/Line.zig
index dbb3786..65bb6fc 100644
--- a/src/render/Line.zig
+++ b/src/render/Line.zig
@@ -40,7 +40,7 @@ pub fn render(writer: *std.Io.Writer, data: types.WeatherData, format: Format, u
         },
         .@"3" => {
             try writer.print("{s}: {s} {s}{d:.0}{s}", .{
-                data.location,
+                data.display_name orelse data.location,
                 emoji.getWeatherEmoji(data.current.weather_code),
                 sign,
                 abs_temp,
@@ -49,7 +49,7 @@ pub fn render(writer: *std.Io.Writer, data: types.WeatherData, format: Format, u
         },
         .@"4" => {
             try writer.print("{s}: {s}  🌡️{s}{d:.0}{s} 🌬️{s}{d:.0}{s}", .{
-                data.location,
+                data.display_name orelse data.location,
                 emoji.getWeatherEmoji(data.current.weather_code),
                 sign,
                 abs_temp,
@@ -114,15 +114,17 @@ test "format 3 with metric units" {
     try std.testing.expectEqualStrings("London: ☀️ +15°C", output);
 }
 
-test "format 3 with imperial units" {
+test "format 3 with imperial units (display name)" {
     var output_buf: [1024]u8 = undefined;
     var writer = std.Io.Writer.fixed(&output_buf);
 
-    try render(&writer, test_data, .@"3", true);
+    var test_data_display = test_data;
+    test_data_display.display_name = "Hello, world";
+    try render(&writer, test_data_display, .@"3", true);
 
     const output = output_buf[0..writer.end];
 
-    try std.testing.expectEqualStrings("London: ☀️ +59°F", output);
+    try std.testing.expectEqualStrings("Hello, world: ☀️ +59°F", output);
 }
 
 test "format 4 with metric units" {