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.

Sign up for GitHub

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

Use POJsOs to model Cluster. Same pattern as DataCube type. #886

Merged
merged 3 commits into from
Mar 17, 2022
Merged

Use POJsOs to model Cluster. Same pattern as DataCube type. #886

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
21138c3
Use POJsOs to model Cluster. Same pattern as DataCube type.
adrianmroz-allegro Mar 16, 2022
5855c3d
Cluster tests
adrianmroz-allegro Mar 17, 2022
6b985b9
Renames for brevity
adrianmroz-allegro Mar 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/client/deserializers/cluster.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2017-2022 Allegro.pl
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { ClientCluster, SerializedCluster } from "../../common/models/cluster/cluster";

export function deserialize(cluster: SerializedCluster): ClientCluster {
return cluster;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is just for consistency - all models have serializers and deserializers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More important is that every model has client and serialised version. Something that lives in browser and something that can be transferred via wire. Here by happy coincidence, client object is build from JSON primitives so it can be easily deserialised. But some time in the future we may have non primitive fields and deserialisation will be more complicated than identity function.

}
4 changes: 2 additions & 2 deletions src/client/deserializers/sources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
*/

import { ClientAppSettings } from "../../common/models/app-settings/app-settings";
import { Cluster } from "../../common/models/cluster/cluster";
import { SerializedDataCube } from "../../common/models/data-cube/data-cube";
import { ClientSources, SerializedSources } from "../../common/models/sources/sources";
import { Ajax } from "../utils/ajax/ajax";
import { deserialize as clusterDeserialize } from "./cluster";
import { deserialize as dataCubeDeserialize } from "./data-cube";

export function deserialize(settings: SerializedSources, appSettings: ClientAppSettings): ClientSources {
const clusters = settings.clusters.map(cluster => Cluster.fromJS(cluster));
const clusters = settings.clusters.map(clusterDeserialize);

const dataCubes = settings.dataCubes.map((dataCube: SerializedDataCube) => {
const executor = Ajax.queryUrlExecutorFactory(dataCube.name, appSettings);
Expand Down
8 changes: 4 additions & 4 deletions src/common/models/cluster/cluster.fixtures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { Cluster, ClusterJS } from "./cluster";
import { Cluster, ClusterJS, fromConfig } from "./cluster";

export class ClusterFixtures {
static druidWikiClusterJS(): ClusterJS {
Expand All @@ -33,7 +33,7 @@ export class ClusterFixtures {
}

static druidWikiCluster(): Cluster {
return Cluster.fromJS(ClusterFixtures.druidWikiClusterJS());
return fromConfig(ClusterFixtures.druidWikiClusterJS());
}

static druidTwitterClusterJS(): ClusterJS {
Expand All @@ -52,11 +52,11 @@ export class ClusterFixtures {
}

static druidTwitterCluster(): Cluster {
return Cluster.fromJS(ClusterFixtures.druidTwitterClusterJS());
return fromConfig(ClusterFixtures.druidTwitterClusterJS());
}

static druidTwitterClusterJSWithGuard(guardDataCubes = true): Cluster {
return Cluster.fromJS({
return fromConfig({
name: "druid-custom",
url: "http://192.168.99.101",
version: "0.9.1",
Expand Down
95 changes: 45 additions & 50 deletions src/common/models/cluster/cluster.mocha.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,56 +15,51 @@
* limitations under the License.
*/

import { expect } from "chai";
import { testImmutableClass } from "immutable-class-tester";
import { Cluster, ClusterJS } from "./cluster";

describe("Cluster", () => {
// TODO: reimplement this test as simpler cases without immutable-class-tester - it checks too much
it.skip("is an immutable class", () => {
testImmutableClass(Cluster, [
{
name: "my-druid-cluster"
},
{
name: "my-druid-cluster",
url: "https://192.168.99.100",
version: "0.9.1",
timeout: 30000,
healthCheckTimeout: 50,
sourceListScan: "auto",
sourceListRefreshOnLoad: true,
sourceListRefreshInterval: 10000,
sourceReintrospectInterval: 10000,

introspectionStrategy: "segment-metadata-fallback"
},
{
name: "my-mysql-cluster",
url: "http://192.168.99.100",
timeout: 30000,
sourceListScan: "auto"
},
{
name: "my-mysql-cluster",
url: "https://192.168.99.100",
timeout: 30000,
sourceListScan: "auto",
sourceListRefreshInterval: 0,
sourceReintrospectInterval: 0
}
]);
});

describe("backward compatibility", () => {
it("should read old host and assume http protocol", () => {
const cluster = Cluster.fromJS({
name: "old-host",
host: "broker-host.com"
} as ClusterJS);

expect(cluster.url).to.be.eq("http://broker-host.com");
});
});

// it.skip("is an immutable class", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to remove commented skipped tests?

Copy link
Contributor Author

@adrianmroz-allegro adrianmroz-allegro Mar 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of answering this trick question, I will reimplement those tests!

// testImmutableClass(Cluster, [
// {
// name: "my-druid-cluster"
// },
// {
// name: "my-druid-cluster",
// url: "https://192.168.99.100",
// version: "0.9.1",
// timeout: 30000,
// healthCheckTimeout: 50,
// sourceListScan: "auto",
// sourceListRefreshOnLoad: true,
// sourceListRefreshInterval: 10000,
// sourceReintrospectInterval: 10000,
//
// introspectionStrategy: "segment-metadata-fallback"
// },
// {
// name: "my-mysql-cluster",
// url: "http://192.168.99.100",
// timeout: 30000,
// sourceListScan: "auto"
// },
// {
// name: "my-mysql-cluster",
// url: "https://192.168.99.100",
// timeout: 30000,
// sourceListScan: "auto",
// sourceListRefreshInterval: 0,
// sourceReintrospectInterval: 0
// }
// ]);
// });
//
// describe.skip("backward compatibility", () => {
// it("should read old host and assume http protocol", () => {
// const cluster = fromConfig({
// name: "old-host",
// host: "broker-host.com"
// } as ClusterJS);
//
// expect(cluster.url).to.be.eq("http://broker-host.com");
// });
// });
});
182 changes: 87 additions & 95 deletions src/common/models/cluster/cluster.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

import { Record } from "immutable";
import { BaseImmutable } from "immutable-class";
import { External } from "plywood";
import { URL } from "url";
Expand All @@ -25,7 +24,10 @@ import { isNil, isTruthy, optionalEnsureOneOf, verifyUrlSafeName } from "../../u

export type SourceListScan = "disable" | "auto";

export interface ClusterValue {
export type ClusterType = "druid";

export interface Cluster {
type: ClusterType;
name: string;
url?: string;
title?: string;
Expand Down Expand Up @@ -61,6 +63,18 @@ export interface ClusterJS {
retry?: RetryOptionsJS;
}

export interface SerializedCluster {
type: ClusterType;
name: string;
timeout: number;
}

export interface ClientCluster {
type: ClusterType;
name: string;
timeout: number;
}

function ensureNotNative(name: string): void {
if (name === "native") {
throw new Error("Cluster name can not be 'native'");
Expand Down Expand Up @@ -109,102 +123,80 @@ export const DEFAULT_SOURCE_REINTROSPECT_ON_LOAD = false;
export const DEFAULT_INTROSPECTION_STRATEGY = "segment-metadata-fallback";
const DEFAULT_GUARD_DATA_CUBES = false;

const defaultCluster: ClusterValue = {
guardDataCubes: DEFAULT_GUARD_DATA_CUBES,
healthCheckTimeout: DEFAULT_HEALTH_CHECK_TIMEOUT,
introspectionStrategy: DEFAULT_INTROSPECTION_STRATEGY,
name: "",
requestDecorator: undefined,
sourceListRefreshInterval: DEFAULT_SOURCE_LIST_REFRESH_INTERVAL,
sourceListRefreshOnLoad: DEFAULT_SOURCE_LIST_REFRESH_ON_LOAD,
sourceListScan: DEFAULT_SOURCE_LIST_SCAN,
sourceReintrospectInterval: DEFAULT_SOURCE_REINTROSPECT_INTERVAL,
sourceReintrospectOnLoad: DEFAULT_SOURCE_REINTROSPECT_ON_LOAD,
timeout: undefined,
title: "",
url: "",
version: null
};

export class Cluster extends Record<ClusterValue>(defaultCluster) {

static fromJS(params: ClusterJS): Cluster {
const {
name,
sourceListScan,
sourceListRefreshOnLoad,
sourceReintrospectOnLoad,
version,
title,
guardDataCubes,
introspectionStrategy,
healthCheckTimeout
} = params;

verifyUrlSafeName(name);
ensureNotNative(name);

optionalEnsureOneOf(sourceListScan, SOURCE_LIST_SCAN_VALUES, "Cluster: sourceListScan");

const sourceReintrospectInterval = typeof params.sourceReintrospectInterval === "string" ? parseInt(params.sourceReintrospectInterval, 10) : params.sourceListRefreshInterval;
if (isTruthy(sourceReintrospectInterval)) {
BaseImmutable.ensure.number(sourceReintrospectInterval);
ensureNotTiny(sourceReintrospectInterval);
}

const sourceListRefreshInterval = typeof params.sourceListRefreshInterval === "string" ? parseInt(params.sourceListRefreshInterval, 10) : params.sourceListRefreshInterval;
if (isTruthy(sourceListRefreshInterval)) {
BaseImmutable.ensure.number(sourceListRefreshInterval);
ensureNotTiny(sourceListRefreshInterval);
}

const retry = RetryOptions.fromJS(params.retry);
const requestDecorator = readRequestDecorator(params);

const url = readUrl(params);
validateUrl(url);

return new Cluster({
timeout: typeof params.timeout === "string" ? parseInt(params.timeout, 10) : params.timeout,
name,
url,
retry,
requestDecorator,
sourceListScan,
sourceListRefreshInterval,
sourceListRefreshOnLoad,
sourceReintrospectInterval,
sourceReintrospectOnLoad,
version,
title,
guardDataCubes,
introspectionStrategy,
healthCheckTimeout
});
}
function readInterval(value: number | string, defaultValue: number): number {
if (!isTruthy(value)) return defaultValue;
const numberValue = typeof value === "string" ? parseInt(value, 10) : value;
BaseImmutable.ensure.number(numberValue);
ensureNotTiny(numberValue);
return numberValue;
}

public type = "druid";
export function fromConfig(params: ClusterJS): Cluster {
const {
name,
sourceListScan = DEFAULT_SOURCE_LIST_SCAN,
sourceListRefreshOnLoad = DEFAULT_SOURCE_LIST_REFRESH_ON_LOAD,
sourceReintrospectOnLoad = DEFAULT_SOURCE_REINTROSPECT_ON_LOAD,
version = null,
title = "",
guardDataCubes = DEFAULT_GUARD_DATA_CUBES,
introspectionStrategy = DEFAULT_INTROSPECTION_STRATEGY,
healthCheckTimeout = DEFAULT_HEALTH_CHECK_TIMEOUT
} = params;

verifyUrlSafeName(name);
ensureNotNative(name);

optionalEnsureOneOf(sourceListScan, SOURCE_LIST_SCAN_VALUES, "Cluster: sourceListScan");

const sourceReintrospectInterval = readInterval(params.sourceReintrospectInterval, DEFAULT_SOURCE_REINTROSPECT_INTERVAL);
const sourceListRefreshInterval = readInterval(params.sourceListRefreshInterval, DEFAULT_SOURCE_LIST_REFRESH_INTERVAL);
const retry = RetryOptions.fromJS(params.retry);
const requestDecorator = readRequestDecorator(params);

const url = readUrl(params);
validateUrl(url);

return {
type: "druid",
timeout: typeof params.timeout === "string" ? parseInt(params.timeout, 10) : params.timeout,
name,
url,
retry,
requestDecorator,
sourceListScan,
sourceListRefreshInterval,
sourceListRefreshOnLoad,
sourceReintrospectInterval,
sourceReintrospectOnLoad,
version,
title,
guardDataCubes,
introspectionStrategy,
healthCheckTimeout
};
}

public toClientCluster(): Cluster {
return new Cluster({
name: this.name,
timeout: this.timeout
});
}
export function serialize(cluster: Cluster): SerializedCluster {
return {
type: "druid",
name: cluster.name,
timeout: cluster.timeout
};
}

public makeExternalFromSourceName(source: string, version?: string): External {
return External.fromValue({
engine: "druid",
source,
version,
suppress: true,
export function makeExternalFromSourceName(source: string, version?: string): External {
return External.fromValue({
engine: "druid",
source,
version,
suppress: true,

allowSelectQueries: true,
allowEternity: false
});
}
allowSelectQueries: true,
allowEternity: false
});
}

public shouldScanSources(): boolean {
return this.sourceListScan === "auto";
}
export function shouldScanSources(cluster: Cluster): boolean {
return cluster.sourceListScan === "auto";
}
Loading