Skip to content

Commit

Permalink
Fix geo types (#520)
Browse files Browse the repository at this point in the history 
* types fix

* add changeset
  • Loading branch information
@ssanjay1 @ericanderson
ssanjay1 authored and ericanderson committed Jul 26, 2024
1 parent 0d69b95 commit e49a554
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 74 deletions.
6 changes: 6 additions & 0 deletions .changeset/eleven-planets-taste.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@osdk/client.api": patch
"@osdk/client": patch
---

Fixing types for within and intersects so they don't take more than the permitted keys
10 changes: 10 additions & 0 deletions etc/client.api.report.api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,10 @@ export type GeoFilter = GeoFilter_Within | GeoFilter_Intersects;
export type GeoFilter_Intersects = {
"$intersects": {
$bbox: BBox;
$polygon?: never;
} | BBox | {
$polygon: Polygon["coordinates"];
$bbox?: never;
} | Polygon;
};

Expand All @@ -371,10 +373,18 @@ export type GeoFilter_Within = {
"$within": {
$distance: [number, keyof typeof DistanceUnitMapping];
$of: [number, number] | Readonly<Point>;
$bbox?: never;
$polygon?: never;
} | {
$bbox: BBox;
$distance?: never;
$of?: never;
$polygon?: never;
} | BBox | {
$polygon: Polygon["coordinates"];
$bbox?: never;
$distance?: never;
$of?: never;
} | Polygon;
};

Expand Down
10 changes: 10 additions & 0 deletions packages/client.api/src/aggregate/WhereClause.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,21 @@ export type GeoFilter_Within = {
| {
$distance: [number, keyof typeof DistanceUnitMapping];
$of: [number, number] | Readonly<Point>;
$bbox?: never;
$polygon?: never;
}
| {
$bbox: BBox;
$distance?: never;
$of?: never;
$polygon?: never;
}
| BBox
| {
$polygon: Polygon["coordinates"];
$bbox?: never;
$distance?: never;
$of?: never;
}
| Polygon;
};
Expand All @@ -128,10 +136,12 @@ export type GeoFilter_Intersects = {
"$intersects":
| {
$bbox: BBox;
$polygon?: never;
}
| BBox
| {
$polygon: Polygon["coordinates"];
$bbox?: never;
}
| Polygon;
};
Expand Down
199 changes: 128 additions & 71 deletions packages/client/src/internal/conversions/modernToLegacyWhereClause.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
* limitations under the License.
*/

import type { WhereClause } from "@osdk/client.api";
import type { MockOntology } from "@osdk/shared.test";
import type { Point } from "geojson";
import { expectType } from "ts-expect";
import { describe, expect, it } from "vitest";
import { modernToLegacyWhereClause } from "./modernToLegacyWhereClause.js";

Expand Down Expand Up @@ -219,16 +221,70 @@ describe(modernToLegacyWhereClause, () => {
}
`);
});
});
describe("$intersects", () => {
it("properly generates bbox shortcut", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: [-5, 5, -10, 10],
it("check types", async () => {
expectType<WhereClause<ObjAllProps>>({
geoPoint: {
$within: [-5, 5, -10, 10],
},
});

expectType<WhereClause<ObjAllProps>>({
geoPoint: {
$within: { $distance: [2, "centimeter"], $of: [2, 2] },
},
});

expectType<WhereClause<ObjAllProps>>({
geoPoint: {
$within: { $polygon: [[[0, 1], [3, 2], [0, 1]]] },
},
});

expectType<WhereClause<ObjAllProps>>({
geoPoint: {
$within: {
type: "Polygon",
coordinates: [[[0, 1], [3, 2], [0, 1]]],
},
},
)).toMatchInlineSnapshot(`
});

expectType<WhereClause<ObjAllProps>>({
geoPoint: {
$within: { $bbox: [-5, 5, -10, 10] },
},
});

expectType<WhereClause<ObjAllProps>>({
geoPoint: {
$within: {
$bbox: [-5, 5, -10, 10],
// @ts-expect-error
$distance: [2, "centimeter"],
// @ts-expect-error
$of: [2, 2],
},
},
});
expectType<WhereClause<ObjAllProps>>({
geoPoint: {
$within: {
$polygon: [[[0, 1], [3, 2], [0, 1]]],
// @ts-expect-error
$bbox: [2, 2, 2, 2],
},
},
});
});
describe("$intersects", () => {
it("properly generates bbox shortcut", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: [-5, 5, -10, 10],
},
},
)).toMatchInlineSnapshot(`
{
"field": "geoShape",
"type": "intersectsBoundingBox",
Expand All @@ -250,17 +306,17 @@ describe(modernToLegacyWhereClause, () => {
},
}
`);
});
it("properly generates bbox long form", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: {
$bbox: [-5, 5, -10, 10],
});
it("properly generates bbox long form", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: {
$bbox: [-5, 5, -10, 10],
},
},
},
},
)).toMatchInlineSnapshot(`
)).toMatchInlineSnapshot(`
{
"field": "geoShape",
"type": "intersectsBoundingBox",
Expand All @@ -282,16 +338,16 @@ describe(modernToLegacyWhereClause, () => {
},
}
`);
});
});

it("properly generates intersects polygon", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: { $polygon: [[[0, 1], [3, 2], [0, 1]]] },
it("properly generates intersects polygon", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: { $polygon: [[[0, 1], [3, 2], [0, 1]]] },
},
},
},
)).toMatchInlineSnapshot(`
)).toMatchInlineSnapshot(`
{
"field": "geoShape",
"type": "intersectsPolygon",
Expand All @@ -316,19 +372,19 @@ describe(modernToLegacyWhereClause, () => {
},
}
`);
});
});

it("properly generates within polygon geojson", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: {
type: "Polygon",
coordinates: [[[0, 1], [3, 2], [0, 1]]],
it("properly generates within polygon geojson", async () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
geoShape: {
$intersects: {
type: "Polygon",
coordinates: [[[0, 1], [3, 2], [0, 1]]],
},
},
},
},
)).toMatchInlineSnapshot(`
)).toMatchInlineSnapshot(`
{
"field": "geoShape",
"type": "intersectsPolygon",
Expand All @@ -353,13 +409,13 @@ describe(modernToLegacyWhereClause, () => {
},
}
`);
});
});
});

it("inverts ne short hand properly", () => {
expect(modernToLegacyWhereClause<ObjAllProps>({
integer: { $ne: 5 },
})).toMatchInlineSnapshot(`
it("inverts ne short hand properly", () => {
expect(modernToLegacyWhereClause<ObjAllProps>({
integer: { $ne: 5 },
})).toMatchInlineSnapshot(`
{
"type": "not",
"value": {
Expand All @@ -369,17 +425,17 @@ describe(modernToLegacyWhereClause, () => {
},
}
`);
});
});
});

describe("multiple checks", () => {
it("properly handles multiple simple where checks", () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
decimal: 5,
integer: 10,
},
)).toMatchInlineSnapshot(`
describe("multiple checks", () => {
it("properly handles multiple simple where checks", () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
decimal: 5,
integer: 10,
},
)).toMatchInlineSnapshot(`
{
"type": "and",
"value": [
Expand All @@ -396,18 +452,18 @@ describe(modernToLegacyWhereClause, () => {
],
}
`);
});
});

it("properly handles $and", () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
$and: [{
decimal: 5,
}, {
integer: 10,
}],
},
)).toMatchInlineSnapshot(`
it("properly handles $and", () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
$and: [{
decimal: 5,
}, {
integer: 10,
}],
},
)).toMatchInlineSnapshot(`
{
"type": "and",
"value": [
Expand All @@ -424,18 +480,18 @@ describe(modernToLegacyWhereClause, () => {
],
}
`);
});
});

it("properly handles $or", () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
$or: [{
decimal: 5,
}, {
integer: 10,
}],
},
)).toMatchInlineSnapshot(`
it("properly handles $or", () => {
expect(modernToLegacyWhereClause<ObjAllProps>(
{
$or: [{
decimal: 5,
}, {
integer: 10,
}],
},
)).toMatchInlineSnapshot(`
{
"type": "or",
"value": [
Expand All @@ -452,6 +508,7 @@ describe(modernToLegacyWhereClause, () => {
],
}
`);
});
});
});
});
10 changes: 7 additions & 3 deletions packages/client/src/internal/conversions/modernToLegacyWhereClause.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,13 @@ function handleWherePair([field, filter]: [string, any]): SearchJsonQueryV2 {

if (Array.isArray(withinBody)) {
return makeGeoFilterBbox(field, withinBody, firstKey);
} else if ("$bbox" in withinBody) {
} else if ("$bbox" in withinBody && withinBody.$bbox != null) {
return makeGeoFilterBbox(field, withinBody.$bbox, firstKey);
} else if ("$distance" in withinBody && "$of" in withinBody) {
} else if (
("$distance" in withinBody && "$of" in withinBody)
&& withinBody.$distance != null
&& withinBody.$of != null
) {
return {
type: "withinDistanceOf",
field,
Expand Down Expand Up @@ -194,7 +198,7 @@ function handleWherePair([field, filter]: [string, any]): SearchJsonQueryV2 {
filter[firstKey] as GeoFilter_Intersects["$intersects"];
if (Array.isArray(intersectsBody)) {
return makeGeoFilterBbox(field, intersectsBody, firstKey);
} else if ("$bbox" in intersectsBody) {
} else if ("$bbox" in intersectsBody && intersectsBody.$bbox != null) {
return makeGeoFilterBbox(field, intersectsBody.$bbox, firstKey);
} else {
const coordinates = ("$polygon" in intersectsBody)
Expand Down

0 comments on commit e49a554

Please sign in to comment.