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.

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 the feature geom instead of the pixel clicked #117

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
File renamed without changes.
2 changes: 1 addition & 1 deletion src/maps/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { AggregationType } from '@/data/operations/aggregation';
import { uuidv4 } from '@/core/utils/uuid';
import { Credentials } from '../auth';
import errorHandlers from './errors';
import { encodeParameter, getRequest, postRequest } from './utils';
import { encodeParameter, getRequest, postRequest } from '../core/utils/request-utils';

const REQUEST_GET_MAX_URL_LENGTH = 2048;
const VECTOR_EXTENT = 2048;
Expand Down
82 changes: 82 additions & 0 deletions src/sql/SQLClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { DEFAULT_ID_PROPERTY } from '@/viz/source/SQLSource';
import { Credentials } from '../auth';
import { encodeParameter, getRequest, postRequest } from '../core/utils/request-utils';
import errorHandlers from '../maps/errors';

const REQUEST_GET_MAX_URL_LENGTH = 2048;
export const GEOMETRY_WKT_ALIAS = 'the_geom_wkt';

export class SQLClient {
private _credentials: Credentials;

constructor(credentials: Credentials) {
this._credentials = credentials;
}

public async getRowById(id: string, origin: string) {
const sql = `SELECT *, ST_AsText(the_geom) as ${GEOMETRY_WKT_ALIAS} FROM (${origin}) as __cdb_orig WHERE ${DEFAULT_ID_PROPERTY} = '${id}'`;
return this.execute(sql);
}

/**
*
* @param sql
*/
public async execute(sql: string) {
if (!sql) {
throw new Error('Please provide an SQL query');
}

let response;

try {
response = await fetch(this.makeSQLApiRequest(sql));
} catch (error) {
throw new Error(
`Failed to connect to Maps API with the user ('${this._credentials.username}'): ${error}`
);
}

const result = (await response.json()) as never;

if (!response.ok) {
this.dealWithSQLApiErrors(response, result);
}

return result;
}

private makeSQLApiRequest(sql: string) {
const encodedApiKey = encodeParameter('api_key', this._credentials.apiKey);
const parameters = [encodedApiKey];
const url = this.generateSQLApiUrl(parameters);

const getUrl = `${url}&${encodeParameter('q', sql)}`;

if (getUrl.length < REQUEST_GET_MAX_URL_LENGTH) {
return getRequest(getUrl);
}

return postRequest(url, sql);
}

private dealWithSQLApiErrors(
response: { status: number },
// eslint-disable-next-line @typescript-eslint/no-explicit-any
result: any
) {
const errorForCode = errorHandlers[response.status];

if (errorForCode) {
errorForCode(this._credentials);
return;
}

throw new Error(`${JSON.stringify(result.errors)}`);
}

private generateSQLApiUrl(parameters: string[] = []) {
const base = `${this._credentials.serverURL}api/v1/sql`;
return `${base}?${parameters.join('&')}`;
}
}
42 changes: 35 additions & 7 deletions src/viz/__tests__/interactivity-popups.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { InteractivityEvent, LayerInteractivity } from '../layer/LayerInteractiv
import { Popup } from '../popups/Popup';

const FAKE_COORDS = [0, 0];
const FEATURE = { type: 'feature', properties: { cartodb_id: '15', pop: 10435000 } };
const FEATURE = {
type: 'feature',
properties: { cartodb_id: '15', pop: 10435000 },
geometry: { type: 'Point', coordinates: [0, 0] }
};

let layer: Layer;
let setContentMockClick: jasmine.Spy;
Expand All @@ -14,12 +18,36 @@ beforeEach(() => {

const sourceData = [
FEATURE,
{ type: 'feature', properties: { cartodb_id: '3', pop: 30 } },
{ type: 'feature', properties: { cartodb_id: '4', pop: 40 } },
{ type: 'feature', properties: { cartodb_id: '5', pop: null } },
{ type: 'feature', properties: { cartodb_id: '6', pop: undefined } },
{ type: 'feature', properties: { cartodb_id: '7', pop: 50 } },
{ type: 'feature', properties: { cartodb_id: '8', pop: 90 } }
{
type: 'feature',
properties: { cartodb_id: '3', pop: 30 },
geometry: { type: 'Point', coordinates: [0, 0] }
},
{
type: 'feature',
properties: { cartodb_id: '4', pop: 40 },
geometry: { type: 'Point', coordinates: [0, 0] }
},
{
type: 'feature',
properties: { cartodb_id: '5', pop: null },
geometry: { type: 'Point', coordinates: [0, 0] }
},
{
type: 'feature',
properties: { cartodb_id: '6', pop: undefined },
geometry: { type: 'Point', coordinates: [0, 0] }
},
{
type: 'feature',
properties: { cartodb_id: '7', pop: 50 },
geometry: { type: 'Point', coordinates: [0, 0] }
},
{
type: 'feature',
properties: { cartodb_id: '8', pop: 90 },
geometry: { type: 'Point', coordinates: [0, 0] }
}
];

spyOn(layer, 'getViewportFeatures').and.returnValue(Promise.resolve(sourceData));
Expand Down
37 changes: 36 additions & 1 deletion src/viz/layer/LayerInteractivity.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Deck, RGBAColor } from '@deck.gl/core';
import { Popup, PopupElement } from '../popups/Popup';
import { Popup, PopupElement, REMOTE_COORD_FLAG } from '../popups/Popup';
import { Style, StyleProperties } from '../style/Style';
import { StyledLayer } from '../style/layer-style';
import { DEFAULT_ID_PROPERTY } from '../source/SQLSource';

export class LayerInteractivity {
private _deckInstance?: Deck;
Expand All @@ -28,6 +29,12 @@ export class LayerInteractivity {
private _clickPopupHandler?: InteractionHandler;
private _hoverPopupHandler?: InteractionHandler;

/**
* Remote coordinates cache in order to prevent
* requesting a feature multiple times
*/
private _remoteCoordinates: Record<string, number[]>;

constructor(options: LayerInteractivityOptions) {
this._layer = options.layer;

Expand Down Expand Up @@ -55,6 +62,8 @@ export class LayerInteractivity {
}

this._layerEmitFn = options.layerEmitFn;

this._remoteCoordinates = {};
}

public onClick(info: any, event: HammerInput) {
Expand All @@ -75,6 +84,12 @@ export class LayerInteractivity {

if (eventType === InteractivityEvent.CLICK) {
this._clickFeature = object;

// to be more accurate on points we use the real feature
// coordinates instead of the coordinates where the user clicked
if (this._clickFeature && this._clickFeature.geometry.type === 'Point') {
this.calculateRemoteCoordinates(this._clickFeature);
}
} else if (eventType === InteractivityEvent.HOVER) {
this._hoverFeature = object;
this._setStyleCursor(info);
Expand Down Expand Up @@ -292,6 +307,26 @@ export class LayerInteractivity {

return new Style(defaultHighlightProps);
}

private calculateRemoteCoordinates(feature: Record<string, any>) {
const remoteCoords = this._remoteCoordinates[feature.properties[DEFAULT_ID_PROPERTY]];

if (remoteCoords) {
// eslint-disable-next-line no-param-reassign
feature.geometry.coordinates = remoteCoords;
// eslint-disable-next-line no-param-reassign
feature.properties[REMOTE_COORD_FLAG] = true;
} else {
this._layer.source.getRemoteFeatureCoordinates(feature).then(coords => {
this._remoteCoordinates[feature.properties[DEFAULT_ID_PROPERTY]] = coords;

if (this._clickPopup) {
// prevent rendering to avoid "jumping" effect
this._clickPopup.setCoordinates(coords, false);
}
});
}
}
}

/**
Expand Down
28 changes: 14 additions & 14 deletions src/viz/popups/Popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ const defaultOptions: PopupOptions = {
position: 'top-right'
};

/**
* Attribute name for the flag used to check if
* the feature coordinates were already calculated
* from the SQL API
*/
export const REMOTE_COORD_FLAG = '__cdb_remote_coords';

/**
* @class
* This class wraps the popup based on the
Expand Down Expand Up @@ -65,14 +72,14 @@ export class Popup {
*
* @param coordinates with long lat.
*/
public setCoordinates(coordinates: number[]) {
public setCoordinates(coordinates: number[], render = true) {
if (!coordinates || coordinates.length !== 2) {
throw new CartoPopupError('Popup coordinates invalid', popupErrorTypes.COORDINATE_INVALID);
}

this._coordinates = coordinates;

if (this._deckInstance) {
if (render && this._deckInstance) {
this._render();
}
}
Expand Down Expand Up @@ -154,23 +161,16 @@ export class Popup {
* content.
*/
public createHandler(elements: PopupElement[] | string[] | null = []) {
return ([features, coordinates]: [Record<string, any>[], number[], HammerInput]) => {
return async ([features, coordinates]: [Record<string, any>[], number[], HammerInput]) => {
if (features.length > 0) {
const popupContent: string = generatePopupContent(elements, features);
this.open();
this.setContent(popupContent);

// to be more accurate on points we use the feature
// coordinates instead of the coordinates where the user clicked
// if (features[0].geometry.type === 'Point') {
// const featureCoordinates = pixels2coordinates(
// features[0].geometry.coordinates,
// this._deckInstance
// );
// this.setCoordinates(featureCoordinates);
// } else {
this.setCoordinates(coordinates);
// }
const popupCoordinates = features[0].properties[REMOTE_COORD_FLAG]
? features[0].geometry.coordinates
: coordinates;
this.setCoordinates(popupCoordinates);
} else {
this.close();
}
Expand Down
5 changes: 5 additions & 0 deletions src/viz/source/DOSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ export class DOSource extends Source {
return this._credentials;
}

// eslint-disable-next-line class-methods-use-this, @typescript-eslint/no-unused-vars
public getRemoteFeatureCoordinates(_feature: Record<string, unknown>): Promise<number[]> {
throw new Error('Method not implemented.');
}

public async init(): Promise<boolean> {
// Get geography from metadata
const variable = await this._getVariable(this._variable);
Expand Down
5 changes: 5 additions & 0 deletions src/viz/source/GeoJSONSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ export class GeoJSONSource extends Source {
.flat();
}

// eslint-disable-next-line class-methods-use-this
public getRemoteFeatureCoordinates(feature: Record<string, unknown>): Promise<number[]> {
return new Promise(resolve => resolve((feature.geometry as any).coordinates));
}

public async init(): Promise<boolean> {
if (!this.needsInitialization) {
return true;
Expand Down
17 changes: 16 additions & 1 deletion src/viz/source/SQLSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Credentials, defaultCredentials } from '@/auth';
import { MapInstance, MapOptions, Client } from '@/maps/Client';
import { uuidv4 } from '@/core/utils/uuid';
import { SQLFilterApplicator } from '@/viz/filters/SQLFilterApplicator';
import { SQLClient, GEOMETRY_WKT_ALIAS } from '@/sql/SQLClient';
import {
Source,
SourceProps,
Expand All @@ -21,7 +22,7 @@ export interface SourceOptions {
mapOptions?: MapOptions;
}

const DEFAULT_ID_PROPERTY = 'cartodb_id';
export const DEFAULT_ID_PROPERTY = 'cartodb_id';

export const defaultMapOptions: MapOptions = {
vectorExtent: 2048,
Expand Down Expand Up @@ -93,6 +94,20 @@ export class SQLSource extends Source {
return `SELECT * FROM (${this._value}) as originalQuery ${whereClause}`.trim();
}

public async getRemoteFeatureCoordinates(feature: Record<string, unknown>): Promise<number[]> {
const sqlClient = new SQLClient(this._credentials);
const id = (feature.properties as any)[DEFAULT_ID_PROPERTY] as string;
const element = await sqlClient.getRowById(id, this._value);
const remoteFeature = (element as any).rows[0];
const remoteCoordinatesWkt = remoteFeature[GEOMETRY_WKT_ALIAS] as string;
const remoteCoords = remoteCoordinatesWkt
.replace(/POINT\s*\(([^\s]+)\s([^\s]+)\)/gi, '$1,$2')
.split(',')
.map(coordStr => Number.parseFloat(coordStr));

return remoteCoords;
}

/**
* It returns the props of the source:
* - URL of the tiles provided by MAPs API
Expand Down
2 changes: 2 additions & 0 deletions src/viz/source/Source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export abstract class Source extends WithEvents {

abstract getFeatures(excludedFilters: string[]): Record<string, unknown>[];

abstract getRemoteFeatureCoordinates(feature: Record<string, unknown>): Promise<number[]>;

// eslint-disable-next-line @typescript-eslint/no-unused-vars, class-methods-use-this
addFilter(_filterId: string, _filter: ColumnFilters) {
throw new Error(`Method not implemented`);
Expand Down