Skip to content

Commit

Permalink
Merge pull request #45 from palantir/ea/force-dollar-sign-where
Browse files Browse the repository at this point in the history
Force dollar sign in osdk verbs for where (break)
  • Loading branch information
ericanderson committed Feb 8, 2024
2 parents 51dd1e8 + f0a848f commit 68f373b
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export async function fetchEmployeePageByAdUsername(
adUsername: string,
) {
const result = await client.objects.Employee.where({
$and: [{ adUsername }, { employeeNumber: { ne: 5 } }],
$and: [{ adUsername }, { employeeNumber: { $ne: 5 } }],
}).fetchPageOrThrow();
// for await (const e of client.objects.Employee.asyncIter()) {
// e.__apiName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ export async function fetchEmployeePageByAdUsernameAndLimit(
const result = await client.objects.Employee.where({
$and: [
{ adUsername },
{ employeeNumber: { ne: 5 } },
{ employeeNumber: { gte: 5 } },
{ employeeNumber: { $ne: 5 } },
{ employeeNumber: { $gte: 5 } },
],
}).fetchPageOrThrow({
select: ["adUsername", "employeeNumber", "jobProfile"],
Expand Down
2 changes: 1 addition & 1 deletion examples/basic/sdk/src/syntax.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ async function aggregateThingsWithGroupsAndHandleErrors() {
groupBy: {
text: "exact",
},
where: { id: { gt: 5 } },
where: { id: { $gt: 5 } },
});

if (isOk(result)) {
Expand Down
5 changes: 5 additions & 0 deletions packages/client/changelog/@unreleased/pr-45.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Force dollar sign in osdk verbs for where (break)
links:
- https://github.com/palantir/osdk-ts/pull/45
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ describe(modernToLegacyWhereClause, () => {

it("inverts ne short hand properly", () => {
expect(modernToLegacyWhereClause<ObjAllProps>({
integer: { ne: 5 },
integer: { $ne: 5 },
})).toMatchInlineSnapshot(`
{
"type": "not",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,28 @@ function handleWherePair([field, filter]: [string, any]): SearchJsonQueryV2 {
};
}

const keysOfFilter = Object.keys(filter);

// If any of the keys start with `$` then they must be the only one.
// e.g. `where({ name: { $eq: "foo", $ne: "bar" } })` is invalid currently
const hasDollarSign = keysOfFilter.some((key) => key.startsWith("$"));
invariant(
Object.keys(filter).length === 1,
"WhereClause Filter with multiple properties isn't allowed",
!hasDollarSign
|| keysOfFilter.length === 1,
"WhereClause Filter with multiple clauses isn't allowed",
);
const firstKey = Object.keys(filter)[0] as PossibleWhereClauseFilters;

if (!hasDollarSign) {
// Future case for structs
throw new Error(
"Unsupported filter. Did you forget to use a $-prefixed filter?",
);
}

const firstKey = keysOfFilter[0] as PossibleWhereClauseFilters;
invariant(filter[firstKey] != null);

if (firstKey === "ne") {
if (firstKey === "$ne") {
return {
type: "not",
value: {
Expand Down Expand Up @@ -157,8 +171,11 @@ function handleWherePair([field, filter]: [string, any]): SearchJsonQueryV2 {
}

return {
type: firstKey,
type: firstKey.substring(1) as DropDollarSign<typeof firstKey>,
field,
value: filter[firstKey] as any,
};
}

type DropDollarSign<T extends `$${string}`> = T extends `$${infer U}` ? U
: never;
8 changes: 4 additions & 4 deletions packages/client/src/object/object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ describe("OsdkObject", () => {
// slightly weird request here to hit the existing mocks for employee2
const employees = await client.objects.Employee.where({
$and: [
{ "employeeId": { "gt": 50030 } },
{ "employeeId": { "lt": 50032 } },
{ "employeeId": { "$gt": 50030 } },
{ "employeeId": { "$lt": 50032 } },
],
}).fetchPageOrThrow();
const lead = employees.data[0];
Expand All @@ -90,8 +90,8 @@ describe("OsdkObject", () => {
// slightly weird request here to hit the existing mocks for employee2
const employees = await client.objects.Employee.where({
$and: [
{ "employeeId": { "gt": 50030 } },
{ "employeeId": { "lt": 50032 } },
{ "employeeId": { "$gt": 50030 } },
{ "employeeId": { "$lt": 50032 } },
],
}).fetchPageOrThrow();
const lead = employees.data[0];
Expand Down
22 changes: 11 additions & 11 deletions packages/client/src/query/WhereClause.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ import type {
import type { DistanceUnit } from "@osdk/gateway/types";

export type PossibleWhereClauseFilters =
| "gt"
| "eq"
| "ne"
| "isNull"
| "contains"
| "gte"
| "lt"
| "lte"
| "$gt"
| "$eq"
| "$ne"
| "$isNull"
| "$contains"
| "$gte"
| "$lt"
| "$lte"
| "$within";

// We need to conditional here to force the union to be distributed
Expand All @@ -41,13 +41,13 @@ type MakeFilter<K extends PossibleWhereClauseFilters, V> = K extends string ? {

type BaseFilter<T> =
| T
| MakeFilter<"eq" | "ne" | "contains", T>
| MakeFilter<"isNull", boolean>;
| MakeFilter<"$eq" | "$ne" | "$contains", T>
| MakeFilter<"$isNull", boolean>;

type StringFilter = BaseFilter<string>;
type NumberFilter =
| BaseFilter<number>
| MakeFilter<"gt" | "gte" | "lt" | "lte", number>;
| MakeFilter<"$gt" | "$gte" | "$lt" | "$lte", number>;

export const DistanceUnitMapping = {
"centimeter": "CENTIMETERS",
Expand Down

0 comments on commit 68f373b

Please sign in to comment.