Regenerating fixtures

[mapsam]

Working on #15

cc @mapbox/core-tech

[springmeyer]

Great work @mapsam. Are you good to keep chugging along here, or would you like help from @mapbox/core-tech in creating some with you? Also, any specific needs for review?

[mapsam]

Thanks @springmeyer - I've just been slowly working on them as I have time. If we're needing them immediately then I'd love some help! I've had a little trouble around working with specific data types, such as uint32's where we should have a string. Right now in the JSON format it's just integer/float ... any ideas how to specify? Should I be doing something in the manipulate function?

[springmeyer]

@mapsam Wow, really enjoying playing around with this fantastic tool. My update:

• Pushed a few things to this branch above. Hope you don't mind
• Added logging for when fixtures are updated (nice to confirm editing takes when iterating)
• Fixed the encoding of Value types on layers. They are message type so needed to be in {} like values: [ {string_value: 'hello' }]

Also, via local testing I confirmed that integer types passed to keys are forced into being strings automatically based on the .proto types:

$ node scripts/build.js  && ./scripts/dump fixtures/010/tile.mvt && ../vector-tile/vtzero/src/vtzero-show fixtures/010/tile.mvt
layers {
  name: "hello"
  features {
    id: 1
    tags: 0
    tags: 0
    type: POINT
    geometry: 9
    geometry: 50
    geometry: 34
  }
  keys: "1"
  values {
    string_value: "hello"
  }
  extent: 4096
  version: 2
}
layer:
  name:    hello
  version: 2
  extent:  4096
  feature:
    id:       1
    geomtype: point
    geometry:
      POINT(25,17)
    tags:
      1="hello"

So, I think we'll need to modify the proto on the fly like we discussed (easy), or manipulate the buffer (harder).

[mapsam]

Great to hear @springmeyer!

Pushed a few things to this branch above. Hope you don't mind

Not one bit

Added logging for when fixtures are updated

gorgeous

Fixed the encoding of Value types on layers.

Thanks for fixing this!

I think we'll need to modify the proto on the fly like we discussed

Sounds like a plan. How about we merge this branch in and start with a fresh PR for this task?

[springmeyer]

@mapsam - dropping some additional thoughts I have after poking last week:

• I feel confident that modifying the .proto (e.g. allowing alternative proto's to be passed in on-the-fly that allow non-compliant types) is a good way to go to be able to support easy generation of more types of (subtly) invalid tiles
• In thinking about ^^ it occurred to me that we are currently hardcoded to the 2.1. spec here:

  mvt-fixtures/index.js

  Line 10 in 2d7630a

  const proto_mvt = schema.parse(fs.readFileSync('vector-tile-spec/2.1/vector_tile.proto', 'utf8'));

  . Which feels wrong in general...
• It feels like a better design would be to make the spec an argument of generateBuffer so that each fixture generator can control its spec. And if the fixture generator wants, it can pass in an alternative one. This would help the design accommodate both the use case of creating bogus fixtures and the usecase of generating fixtures for upcoming/new specs that might have a different scheme.

Thoughts on how to pull this off?

[mapsam]

Good thinking @springmeyer - yes I have some ideas. We can move the schema compile lines into generateBuffer and add an extra argument to this function that can either be

1. a string denoting a version (can do some sort of validation on possible values)
2. or a path to a new (presumably malformed according to the spec) proto file to compile from.

I'd like to avoid creating entirely new protofiles for each malformed fixture, so maybe we can update the concept of manipulate to be a custom proto field. This field can be a version value, like:

proto: '2.1'

Or a string representing a completely new protofile that we can write to disk when generate fixtures and remove it when done generating, like:

proto: `
package vector_tile;
option optimize_for = LITE_RUNTIME;
message Tile {
        enum GeomType {
             UNKNOWN = 0;
             POINT = 1;
             LINESTRING = 2;
             POLYGON = 3;
        }

        message Value {
                optional string string_value = 1;
                optional float float_value = 2;
                optional double double_value = 3;
                optional int64 int_value = 4;
                optional uint64 uint_value = 5;
                optional sint64 sint_value = 6;
                optional bool bool_value = 7;
                extensions 8 to max;
        }

        message Feature {
                optional uint64 id = 1 [ default = 0 ];
                repeated uint32 tags = 2 [ packed = true ];
                optional GeomType type = 3 [ default = UNKNOWN ];
                repeated uint32 geometry = 4 [ packed = true ];
        }

        message Layer {
                required uint32 version = 15 [ default = 1 ];
                required string name = 1;
                repeated Feature features = 2;
                repeated string keys = 3;
                repeated Value values = 4;
                optional uint32 extent = 5 [ default = 4096 ];
                extensions 16 to max;
        }

        repeated Layer layers = 3;
        extensions 16 to 8191;
}
`

The protocol-buffers-schema library allows you to pass in a string to schema.parse which will be nice. I can work on this proof of concept and will send to you for review!

[springmeyer]

@mapsam - great, happy to have you run this out however you think fit.

[mapsam]

👍 going to merge this in and start fresh