Skip to content

Commit

Permalink
fix: handle values with multiple types (including null) [SCH-1643] (#57)
Browse files Browse the repository at this point in the history
This PR extends our test suite to cover JSON Schemas with properties that specify union types (`type: ["string", "number"]`), including union types with a `null` type (`type: ["string", "null"]`).

Specifically, this resolves an issue with the Android client not generating enums (and instead crashing during `typewriter gen-android`).

------

While testing the example Android app against Turo's inferred spec, I also noticed that there were collisions between property key names, because similar properties (like `optional any` and `optional_any`) would map to the same key identifier (`OPTIONAL_ANY_KEY`). This was fixed by inlining these values.

-------

I also identified an issue with event name collisions, specifically if you have two event names that differ by delimiter characters (f.e. `check_in_event` and `checkin_event`). They both map to the same filename (causing one of the files to be overwritten). In this example case, `CheckInEvent.java` and `CheckinEvent.java` (note: file systems are case-insensitive). Java will end up complaining because the class name (either `CheckInEvent` or `CheckinEvent`) differs from the filename.

This can be solved by an upstream PR to QuickType ([see our conversation in Slack](https://quicktype.slack.com/archives/C68E91E8J/p1551486779009600)), but I'm not going to block this PR on that. The fix would be to make the `Namer` case-insensitive so that it uses a suffix in both the class and file names to differentiate between these events.

The simplest solution for affected users is to remove all but one of the colliding events. Usually these events appear due to inferring on bad data, so it usually won't be a problem.

I've added a warning that notifies users when their Tracking Plan contains events that collide in Android.

------

I also made a few refactoring changes:

- The logic to generate QuickType's `inputData` is duplicated across all QuickType-based clients, so I moved that out to `utils/rules.ts`
- Removed `PropertiesSerializable` because it wasn't necessary
  • Loading branch information
colinking authored Mar 6, 2019
1 parent 326e8cc commit 6cce0f7
Show file tree
Hide file tree
Showing 31 changed files with 2,358 additions and 635 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This code is auto-generated by Segment Typewriter. Do not edit.
package com.segment.analytics;

import java.util.*;
import com.segment.analytics.Analytics;
import com.segment.analytics.Options;
import android.content.Context;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
// This code is auto-generated by Segment Typewriter. Do not edit.
package com.segment.analytics;

import java.util.*;
import android.support.annotation.NonNull;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

public final class OrderCompleted extends PropertiesSerializable {
public final class OrderCompleted {
private Properties properties;

private OrderCompleted(Properties properties) {
Expand All @@ -23,18 +19,6 @@ protected Properties toProperties() {
* Builder for {@link OrderCompleted}
*/
public static class Builder {
private static String AFFILIATION_KEY = "affiliation";
private static String CHECKOUT_ID_KEY = "checkout_id";
private static String COUPON_KEY = "coupon";
private static String CURRENCY_KEY = "currency";
private static String DISCOUNT_KEY = "discount";
private static String ORDER_ID_KEY = "order_id";
private static String PRODUCTS_KEY = "products";
private static String REVENUE_KEY = "revenue";
private static String SHIPPING_KEY = "shipping";
private static String TAX_KEY = "tax";
private static String TOTAL_KEY = "total";

private Properties properties;

/**
Expand All @@ -49,7 +33,7 @@ public Builder() {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder affiliation(final @NonNull String affiliation) {
properties.putValue(AFFILIATION_KEY, affiliation);
properties.putValue("affiliation", affiliation);
return this;
}

Expand All @@ -58,7 +42,7 @@ public Builder affiliation(final @NonNull String affiliation) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder checkoutID(final @NonNull String checkoutID) {
properties.putValue(CHECKOUT_ID_KEY, checkoutID);
properties.putValue("checkout_id", checkoutID);
return this;
}

Expand All @@ -67,7 +51,7 @@ public Builder checkoutID(final @NonNull String checkoutID) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder coupon(final @NonNull String coupon) {
properties.putValue(COUPON_KEY, coupon);
properties.putValue("coupon", coupon);
return this;
}

Expand All @@ -76,7 +60,7 @@ public Builder coupon(final @NonNull String coupon) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder currency(final @NonNull String currency) {
properties.putValue(CURRENCY_KEY, currency);
properties.putValue("currency", currency);
return this;
}

Expand All @@ -85,7 +69,7 @@ public Builder currency(final @NonNull String currency) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder discount(final @NonNull Double discount) {
properties.putValue(DISCOUNT_KEY, discount);
properties.putValue("discount", discount);
return this;
}

Expand All @@ -94,7 +78,7 @@ public Builder discount(final @NonNull Double discount) {
* This property is required to generate a valid OrderCompleted object
*/
public Builder orderID(final @NonNull String orderID) {
properties.putValue(ORDER_ID_KEY, orderID);
properties.putValue("order_id", orderID);
return this;
}

Expand All @@ -103,7 +87,7 @@ public Builder orderID(final @NonNull String orderID) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder products(final @NonNull List<Product> products) {
properties.putValue(PRODUCTS_KEY, PropertiesSerializable.toPropertyList(products));
properties.putValue("products", products);
return this;
}

Expand All @@ -112,7 +96,7 @@ public Builder products(final @NonNull List<Product> products) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder revenue(final @NonNull Double revenue) {
properties.putValue(REVENUE_KEY, revenue);
properties.putValue("revenue", revenue);
return this;
}

Expand All @@ -121,7 +105,7 @@ public Builder revenue(final @NonNull Double revenue) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder shipping(final @NonNull Double shipping) {
properties.putValue(SHIPPING_KEY, shipping);
properties.putValue("shipping", shipping);
return this;
}

Expand All @@ -130,7 +114,7 @@ public Builder shipping(final @NonNull Double shipping) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder tax(final @NonNull Double tax) {
properties.putValue(TAX_KEY, tax);
properties.putValue("tax", tax);
return this;
}

Expand All @@ -142,7 +126,7 @@ public Builder tax(final @NonNull Double tax) {
* This property is optional and not required to generate a valid OrderCompleted object
*/
public Builder total(final @NonNull Double total) {
properties.putValue(TOTAL_KEY, total);
properties.putValue("total", total);
return this;
}

Expand All @@ -152,7 +136,7 @@ public Builder total(final @NonNull Double total) {
* - orderID
*/
public OrderCompleted build() {
if (properties.get(ORDER_ID_KEY) == null) {
if (properties.get("order_id") == null) {
throw new IllegalArgumentException("OrderCompleted missing required property: orderID");
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
// This code is auto-generated by Segment Typewriter. Do not edit.
package com.segment.analytics;

import java.util.*;
import android.support.annotation.NonNull;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

public final class Product extends PropertiesSerializable {
public final class Product {
private Properties properties;

private Product(Properties properties) {
Expand All @@ -23,19 +19,6 @@ protected Properties toProperties() {
* Builder for {@link Product}
*/
public static class Builder {
private static String BRAND_KEY = "brand";
private static String CATEGORY_KEY = "category";
private static String COUPON_KEY = "coupon";
private static String IMAGE_URL_KEY = "image_url";
private static String NAME_KEY = "name";
private static String POSITION_KEY = "position";
private static String PRICE_KEY = "price";
private static String PRODUCT_ID_KEY = "product_id";
private static String QUANTITY_KEY = "quantity";
private static String SKU_KEY = "sku";
private static String URL_KEY = "url";
private static String VARIANT_KEY = "variant";

private Properties properties;

/**
Expand All @@ -50,7 +33,7 @@ public Builder() {
* This property is optional and not required to generate a valid Product object
*/
public Builder brand(final @NonNull String brand) {
properties.putValue(BRAND_KEY, brand);
properties.putValue("brand", brand);
return this;
}

Expand All @@ -59,7 +42,7 @@ public Builder brand(final @NonNull String brand) {
* This property is optional and not required to generate a valid Product object
*/
public Builder category(final @NonNull String category) {
properties.putValue(CATEGORY_KEY, category);
properties.putValue("category", category);
return this;
}

Expand All @@ -68,7 +51,7 @@ public Builder category(final @NonNull String category) {
* This property is optional and not required to generate a valid Product object
*/
public Builder coupon(final @NonNull String coupon) {
properties.putValue(COUPON_KEY, coupon);
properties.putValue("coupon", coupon);
return this;
}

Expand All @@ -77,7 +60,7 @@ public Builder coupon(final @NonNull String coupon) {
* This property is optional and not required to generate a valid Product object
*/
public Builder imageURL(final @NonNull String imageURL) {
properties.putValue(IMAGE_URL_KEY, imageURL);
properties.putValue("image_url", imageURL);
return this;
}

Expand All @@ -86,7 +69,7 @@ public Builder imageURL(final @NonNull String imageURL) {
* This property is optional and not required to generate a valid Product object
*/
public Builder name(final @NonNull String name) {
properties.putValue(NAME_KEY, name);
properties.putValue("name", name);
return this;
}

Expand All @@ -95,7 +78,7 @@ public Builder name(final @NonNull String name) {
* This property is optional and not required to generate a valid Product object
*/
public Builder position(final @NonNull Long position) {
properties.putValue(POSITION_KEY, position);
properties.putValue("position", position);
return this;
}

Expand All @@ -104,7 +87,7 @@ public Builder position(final @NonNull Long position) {
* This property is optional and not required to generate a valid Product object
*/
public Builder price(final @NonNull Double price) {
properties.putValue(PRICE_KEY, price);
properties.putValue("price", price);
return this;
}

Expand All @@ -113,7 +96,7 @@ public Builder price(final @NonNull Double price) {
* This property is optional and not required to generate a valid Product object
*/
public Builder productID(final @NonNull String productID) {
properties.putValue(PRODUCT_ID_KEY, productID);
properties.putValue("product_id", productID);
return this;
}

Expand All @@ -122,7 +105,7 @@ public Builder productID(final @NonNull String productID) {
* This property is optional and not required to generate a valid Product object
*/
public Builder quantity(final @NonNull Double quantity) {
properties.putValue(QUANTITY_KEY, quantity);
properties.putValue("quantity", quantity);
return this;
}

Expand All @@ -131,7 +114,7 @@ public Builder quantity(final @NonNull Double quantity) {
* This property is optional and not required to generate a valid Product object
*/
public Builder sku(final @NonNull String sku) {
properties.putValue(SKU_KEY, sku);
properties.putValue("sku", sku);
return this;
}

Expand All @@ -140,7 +123,7 @@ public Builder sku(final @NonNull String sku) {
* This property is optional and not required to generate a valid Product object
*/
public Builder url(final @NonNull String url) {
properties.putValue(URL_KEY, url);
properties.putValue("url", url);
return this;
}

Expand All @@ -149,7 +132,7 @@ public Builder url(final @NonNull String url) {
* This property is optional and not required to generate a valid Product object
*/
public Builder variant(final @NonNull String variant) {
properties.putValue(VARIANT_KEY, variant);
properties.putValue("variant", variant);
return this;
}

Expand Down

This file was deleted.

Loading

0 comments on commit 6cce0f7

Please sign in to comment.