Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework instantiation to be more useful and consistent with the spec #10

Closed
nyurik opened this issue May 18, 2022 · 0 comments · Fixed by #14
Closed

Rework instantiation to be more useful and consistent with the spec #10

nyurik opened this issue May 18, 2022 · 0 comments · Fixed by #14

Comments

@nyurik
Copy link
Member

nyurik commented May 18, 2022

There are two usecases for this lib -- with and without defaults:

  • A typically use case with defaults -- a client code that gets a JSON, resolves maxzoom to 30 if missing, and takes actions like loading tiles.
  • Without defaults -- a tile server needs to create and send a minimal tilejson, e.g. if it doesn't support grids value, it shouldn't set it to the default [ ], but instead not set it at all.

Current API actually does the opposite -- the client use-case" serde_json::from_str(str) ignores tilespec defaults, while the server use case with TileJSON::new() sets the defaults.

Also, per specification, several fields must be present from the start, so it would make sense to introduce them as arguments:

"tilejson": "3.0.0",
"tiles": [
    "http://localhost:8888/admin/1.0.0/world-light,broadband/{z}/{x}/{y}.mvt"
  ],

When instantiating, tilejson could still be defaulted to the current version, i.e. 3.0.0. The tiles on the other hand MUST be a non-empty array.

Proposed API (breaking)

impl TileJSONBuilder {
  /// create a builder with tilejson = 3.0.0 and tiles = [ source ]
  fn new(source: String) -> Self {}
  /// create a builder with custom version and possibly multiple sources.
  /// If version is None, will use current default.
  fn new_ext(sources: Vec<String>, version: Option<String>) -> Self {}
  /// generate TileJSON **without** defaults.  Breaking change, so new method name.
  fn build() -> TileJSON {}
  // remove fn tiles(...)
}

impl TileJSON {
  /// consume to ensure each optional value has a proper default per spec. Name is TBD
  fn with_defaults(self) -> Self {}
}

Note that "vector_layers" is required conditionally - depending on the type of the tiles, so in our case we could treat it as optional unless we want to perform validation.

cc: @stepankuzmin @maxammann @Drabble @ka7eh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant