`Location` data should be optional

[llucax]

What happened?

Some fields for Location can be missing, when that happens, there is no way to tell if the values were missing because protobuf will assign them the default. For longitude and latitude that is (0, 0) which is actually a valid coordinate, making the problem worse. For country code, the default is "", which could be easily interpreted as missing, but it is not explicit enough.

What did you expect instead?

All Location fields are required.

Affected version(s)

v1alpha8

Affected part(s)

The protocol buffer definition files (part:protobuf)

Extra information

Some possible solutions are, from more correct and more disruptive, to less correct and less disruptive:

New Coordinate message

message Coordinate {
    float latitude = 1;
    float longitude = 2;
}

message Location {
    optional Coordinates coordinates = 1;
    optional string country_code = 2;
}

Pros:

• Clearly indicates when both lat/long are available or not.
• All fields clearly marked as optional at the level they should.

Cons:

• Breaking change, but it can go easily through a brackwards-compatible (wire-level) deprecation phase, by using this Location definition instead:

  message Location {
      float latitude = 1 [deprecated = true];
      float longitude = 2 [deprecated = true];
      optional string country_code = 3;
      optional Coordinates coordinates = 4;
  }

Plus country code as enum

It was suggested to add a CountryCode enum, so only valid country codes can be used.

Pros:

• Same as before, plus no need to validate country codes

Cons:

• Same as before, plus having to re-deploy all our services each time the ISO changes (which is not often, but it can happen)

Make all optional

message Location {
    optional float latitude = 1;
    optional float longitude = 2;
    optional string country_code = 3;
}

Pros:

• Backwards compatible (wire-level)
• All fields marked as optional

Cons:

• Still room for weird combinations that are not really something that can happen (having latitude but not longitude)

Make latitude / longitude optional

message Location {
    optional float latitude = 1;
    optional float longitude = 2;
    string country_code = 3;
}

Pros:

• Backwards compatible (wire-level)
• Country code still backwards compatible at the source level too

Cons:

• Still room for weird combinations that are not really something that can happen (having latitude but not longitude)
• Optionality of country code needs to be stated in the documentation, less clear

[llucax]

I was mentioned that the Weather API also uses Location but requires only the coordinates part, so if we add a Coordinates message the Weather API could just use Coordinates instead of Location.

[llucax]

@cwasicki also suggested naming Coordinates -> Position. ChatGPT suggests the following options:

Name	When I would use it
GeoPoint	Best general-purpose name. Clear that it represents a geographic point.
LatLng	Good if the codebase/API already uses compact geospatial naming, especially Google-style APIs. Already a common Google type.
Coordinates	Too generic unless the surrounding package is already geographic.
Position	Ambiguous: could mean object position, GPS fix, index, etc.
Location	Often implies more than coordinates, such as address, name, accuracy, timestamp.