search

loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.96k stars 1.07k forks source link

Support GeoPoint type #1981

Open JorisPalings opened 6 years ago

JorisPalings commented 6 years ago

Description/Steps to reproduce

I'm attempting to start my project by using npm start and get the following output:

> project-api@1.0.0 prestart /Users/joris/Documents/Personal/Projects/project/project-api
> npm run build

> project-api@1.0.0 build /Users/joris/Documents/Personal/Projects/project/project-api
> lb-tsc es2017 --outDir dist

> project-api@1.0.0 start /Users/joris/Documents/Personal/Projects/project/project-api
> node .

Cannot start the application. Error: Unsupported type: geopoint
    at stringTypeToWrapper (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/repository-json-schema/dist/src/build-schema.js:68:19)
    at metaToJsonProperty (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/repository-json-schema/dist/src/build-schema.js:101:25)
    at modelToJsonSchema (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/repository-json-schema/dist/src/build-schema.js:151:32)
    at Object.getJsonSchema (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/repository-json-schema/dist/src/build-schema.js:22:27)
    at generateOpenAPISchema (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/openapi-v3/dist/src/controller-spec.js:181:49)
    at resolveControllerSpec (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/openapi-v3/dist/src/controller-spec.js:128:17)
    at Object.getControllerSpec (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/openapi-v3/dist/src/controller-spec.js:205:16)
    at RestServer._setupHandlerIfNeeded (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/rest/dist/src/rest.server.js:195:42)
    at RestServer.start (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/rest/dist/src/rest.server.js:498:14)
    at _forEachServer.s (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/core/dist/src/application.js:123:42)
    at Promise.all.bindings.map (/Users/joris/Documents/Personal/Projects/project/project-api/node_modules/@loopback/core/dist/src/application.js:145:26)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! project-api@1.0.0 start: `node .`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the project-api@1.0.0 start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/joris/.npm/_logs/2018-11-05T15_06_38_766Z-debug.log

In one of my models, I have a property of type 'geopoint':

  @property({
    type: 'geopoint',
    required: true,
  })
  location: string;

Upon closer inspection, no case is defined for the 'geopoint' type in the switch statement in @loopback/repository-json-schema/dist/src/build-schema.js.

Expected result

For the Loopback 4 project to start.

Additional information

jannyHou commented 6 years ago

The support for GeoPoint is not finished yet. I suggest turn this issue to be a feature.

We can either leverage the GeoPoint class in juggler https://github.com/strongloop/loopback-datasource-juggler/blob/master/lib/geo.js#L164

Or implement a GeoPoint type in loopback 4: see https://github.com/strongloop/loopback-next/tree/master/packages/repository/src/types

bajtos commented 6 years ago

Added "feature" label.

@raymondfeng what's your take on the direction we should take here? I see two options:

  1. A quick fix: keep using the current GeoPoint class from juggler and/or loopback-datatype-geopoint. @loopback/repository should export GeoPoint constructor. We may need to describe GeoPoint in juggler typings.

  2. A rewrite/redesign: create a new implementation of GeoPoint to be used in LB4. While this type can live in @loopback/repository, I'd personally prefer to see it in it's own package, e.g. @loopback/geopoint.

Either way, we need to improve @loopback/repository-json-schema to correctly handle this new type. If GeoPoint is maintained outside of @loopback/repository, then we need to define & implement an extension point allowing GeoPoint to contribute JSON Schema mapping.

Personally, I'd prefer the second option, as it provides a great opportunity to build infrastructure that will allow LB4 community to contribute further types in the future.

bajtos commented 6 years ago

@JorisPalings your model is storing GeoPoint types as string. I find that a bit unusual, shouldn't GeoPoints be represented as objects?

JorisPalings commented 6 years ago

@bajtos I created it through the lb4 model CLI command, which for some reason defined all model properties as strings.

bajtos commented 5 years ago

I created it through the lb4 model CLI command, which for some reason defined all model properties as strings.

Here is the code generating TypeScript types in lb4 model:

https://github.com/strongloop/loopback-next/blob/7966ee8e8a0b9129ed980d79be06a8e8e907b2ee/packages/cli/generators/model/index.js#L297-L319

AFAICT, it falls back to string for types it does not understand (e.g. geopoint).

jannyHou commented 5 years ago

Shall we do a spike story for adding types like GeoPoint?

bajtos commented 5 years ago

IIRC, @hacksparrow is already looking into this issue.

dhmlau commented 5 years ago

@hacksparrow, since you're looking into this issue already, could you please add the acceptance criteria, so that the team can estimate? Thanks!

hacksparrow commented 5 years ago

@dhmlau the acceptance criteria for this issue would be simply, "App should start with GeoPoint support". We will miss the implementation and feature details of the as-yet-non-supportedGeoPoint data type. Maybe we should track the GeoPoint support on a separate dedicated issue?

We also need to decide on the approaches suggested by @bajtos - https://github.com/strongloop/loopback-next/issues/1981#issuecomment-436966284.

bajtos commented 5 years ago

This issue is labelled as a feature, the goal is to add support for GeoPoint to LB4. See my comments above for more details about the problems we need to fix and places to modify.

"App should start with GeoPoint support".

IMO, the app should not only start, but also correctly handle GeoPoint properties in incoming requests and outgoing responses.

In the acceptance criteria, I'd like to see a description of the approach we decided to take and a (high-level) list of places in our code that need to be modified.

dhmlau commented 5 years ago

@strongloop/loopback-maintainers, before we can get to the acceptance criteria, I think we probably need to discuss how we should enable the support of GeoPoint. There are 2 options proposed by @bajtos in https://github.com/strongloop/loopback-next/issues/1981#issuecomment-436966284. PTAL.

ewrayjohnson commented 5 years ago

I have always found the LB3 GeoPoint type limiting, and have often desired support for other geospatial types (e.g. lines, polygons. etc.) and other geospatial relationship comparison operators (e.g. other than just near). I suggest LB4 (insead of / in addtion to) support GeoJSON types (i.e. GeoJsonObject, GeometryObject, or Geometry) as there are many support packages which simplify the implementation (e.g. for the Memory Connector). See:

P.S. Several weeks ago I tried implementing this as a value object and had difficulties because of how arrays of types are not well supported in LB4 (i.e. an array of numbers).

bajtos commented 5 years ago

had difficulties because of how arrays of types are not well supported in LB4 (i.e. an array of numbers).

Could you please open a new issue and provide more details? @b-admike is improving handling of nested properties (including arrays) at the moment, some of the issues may be already fixed by now.

hadasiddhant commented 5 years ago

There is also an issue with type any

Cannot start the application. Error: Unsupported type: any
    at stringTypeToWrapper (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\repository-json-schema\dist\build-schema.js:69:19)
    at metaToJsonProperty (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\repository-json-schema\dist\build-schema.js:102:25)
    at modelToJsonSchema (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\repository-json-schema\dist\build-schema.js:160:32)
    at Object.getJsonSchema (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\repository-json-schema\dist\build-schema.js:23:27)
    at generateOpenAPISchema (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\openapi-v3\dist\controller-spec.js:194:49)
    at resolveTSType (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\openapi-v3\dist\controller-spec.js:161:13)
    at resolveControllerSpec (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\openapi-v3\dist\controller-spec.js:66:17)
    at Object.getControllerSpec (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\openapi-v3\dist\controller-spec.js:218:16)
    at RestServer._setupHandlerIfNeeded (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\rest\dist\rest.server.js:215:42)
    at RestServer.start (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\rest\dist\rest.server.js:530:14)
    at LifeCycleObserverRegistry.invokeObserver (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\core\dist\lifecycle-registry.js:136:34)
    at observers.map (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\core\dist\lifecycle-registry.js:125:25)
    at Array.map (<anonymous>)
    at LifeCycleObserverRegistry.notifyObservers (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\core\dist\lifecycle-registry.js:123:37)
    at LifeCycleObserverRegistry.notifyGroups (C:\Users\user\Desktop\happyfitt\node_modules\@loopback\core\dist\lifecycle-registry.js:162:28)
    at process._tickCallback (internal/process/next_tick.js:68:7)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! happyfitt@1.0.0 start: `node .`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the happyfitt@1.0.0 start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\user\AppData\Roaming\npm-cache\_logs\2019-05-04T21_39_50_979Z-debug.log
bajtos commented 5 years ago

@hadasiddhant please open a new issue to discuss the type any, so that the discussion here can stay focused on GeoPoint. Thanks!

riyastir commented 5 years ago

Guys any update on GeoPoint?

malek0512 commented 4 years ago

Hello, any news on this subject ?

ewrayjohnson commented 4 years ago

I am working on creating tests to show the bug and confirm the fix

malek0512 commented 4 years ago

Hello @ewrayjohnson @bajtos, is there any progress on this feature ?

dhmlau commented 4 years ago

@strongloop/loopback-maintainers, would like to get your input on the approach to support geopoint. Thanks!

To summarize, @bajtos had 2 proposals mentioned in https://github.com/strongloop/loopback-next/issues/1981#issuecomment-436966284, and @ewrayjohnson also mentioned the limitation of the juggler geopoint implementation.

ewrayjohnson commented 4 years ago

Yes

Wray Johnson

On Mar 24, 2020, at 10:38 AM, Malek Mammar notifications@github.com wrote:

 Hello @ewrayjohnson, is there any progress on this feature ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

deepakrkris commented 4 years ago

@JorisPalings @riyastir @malek0512 @ewrayjohnson can you give some details on the usecases you need this feature for ? Is this necessary for a specific backend or db querying ? we need some more details on this issue , to scope it and estimate,

madaky commented 4 years ago

Hi, I anyhow tried to insert the geopont data with mysql-connector. first in your model : add Interface eg:

interface GeoObject {
  lat: number;
  lng: number;
}

than in side your class in property decorator

...Model
 @property({
    type: 'object',
    required: false,
    mysql: {
      dataType: 'point',
    },
  })
  location: GeoObject;
...restModel

Now to insert from rest 'post' endpoint, in our model repository, we will be using 2 hooks 1) persist 2) after save to save the geopoint type data in database. The process is divided in to two steps 1)The basic idea is to pull GeoPoint from 'location' and save in a variable and deleting the node loc from the current context and save the data. using 'persist' hook 2) after that updating the instance with the 'location' by using Update the instance by 'this.datasource.execute()' mentioned in @bajtos loopback-datasource-juggler/pull/1671 using aftersave hook.

Both hook should be called inside the constructor.Example code

...constructor
 let loc: any = null;
    (this.modelClass as any).observe('persist', async (ctx: any) => {
      console.log(ctx.data);
      loc = ctx.data.location;
      delete ctx.data.location;
      console.log(ctx.data);
    });
    (this.modelClass as any).observe('after save', async (ctx: any) => {
      console.log(ctx);
      const id = ctx.instance.id;
      this.dataSource.execute(
        "UPDATE `Store` SET `location` = ST_GeomFromText('Point(" +
          loc.lat +
          ' ' +
          loc.lng +
          ")') WHERE `Store`.`id` = '" +
          id +
          "'",
      );
    });
...restConstructor code
madaky commented 4 years ago

@JorisPalings @riyastir @malek0512 @ewrayjohnson can you give some details on the usecases you need this feature for ? Is this necessary for a specific backend or db querying ? we need some more details on this issue , to scope it and estimate, @deepakrkris : the details are as follows :

Problem Definitation : AFAIK, When we need to create a Spatial data type in db, we would be needing the datatype

Here we are focusing on inserting the data in 'Point' type data type. -For the purpose we need a type named 'geopoint' which may be a object type consisting of two coordinates. { x:number, y:number } -This help in reducing the manual leverage of the developers to define the Spatial data.

Also I have not tried other queries like st_distance to find the distance from PointA to Point B

Another approach can be we can store GeoJson data in data source, and can implement GeoJson schema.

Please let me know if this helps. Also please let me know how to approach this.

vikramkh commented 4 years ago

It says that GeoPoint is support here https://apidocs.strongloop.com/loopback-datasource-juggler/#geopoint

Is it not actually supported? I'm using postgresql. If it is supported now, what packages should I update?

frbuceta commented 4 years ago

I will try to implement this in the next few days. I think it is necessary to have this functionality

GianlucaCesari commented 4 years ago

Hi guys, any news on the support for geopoint?

frbuceta commented 4 years ago

Hi guys, any news on the support for geopoint?

Nothing advanced yet. Sorry for the inconvenience

lakshanmamalgaha commented 4 years ago

@frbuceta Is now the loopback 4 support geopoint?

pktippa commented 4 years ago

@dhmlau @bajtos @frbuceta Any new updates on LB4 with GeoPoint data support.

kdrkrst commented 3 years ago

LB4 docs mention the geopoint as one of the available data types. At least we should update the doc to state that geopoint is not completed, till we have the feature.

bajtos commented 3 years ago

LB4 docs mention the geopoint as one of the available data types. At least we should update the doc to state that geopoint is not completed, till we have the feature.

Makes sense. Would you @kursattokpinar like to open a pull request to update our docs? See https://loopback.io/doc/en/lb4/code-contrib-lb4.html#documentation and https://loopback.io/doc/en/lb4/submitting_a_pr.html to get started.

kdrkrst commented 3 years ago

Yes sure, happy to contribute. I have already gone over the contrib guides and set up my environment (it took while to configure Jekyll stuff and WSL).

I am going to update Loopback Types page to include that development on Geopoint type is not completed yet. All other pages mentioning about Geopoint type is already referring to the Loopback Types page. Thus, it seems no need to update those.

Do you have anything else to add @bajtos ?

daniel-safeup commented 3 years ago

When someone will take care about this issue? I am building a new piece of software and the "GeoPoint" is critical, so from my POV loopback is on "question" if this is not going to come soon.

to2021 commented 3 years ago

Same here, I would like to use Loopback but without geopoints it is no option for the application I wanna code... It’s a basic feature in my opinion.

sskhokhar commented 3 years ago

I think we can simply use {lat:number,lng:number} instead of Geopoints and filter using where near clause

ewrayjohnson commented 3 years ago

The problem is not only one of querying, but the database connector needs to be aware of the correct datatype for proper mapping to the underlying database datatype.

dany-sb commented 2 years ago

Still missing. is missing in "repository-json-schema" in "stringTypeToWrapper" I suppose that is also missing in the opposite direction when we want to save it to the database. This is really a missing important point

ashelman-ec commented 2 years ago

I would like this feature too.

Use case: JSON location data stored & retrieved from MySQL database.

PhilippeCorreges commented 1 year ago

Hello, I hope you are fine. Has this feature been delivered? I see it has been opened a long time ago. Also, please is there a workaround?

Kind Regards

tikimo commented 11 months ago

bump this feature, its been already a couple years in the making. Is the blocker really about being aware of the datatype? couldnt it just be a matter of configuration requirement?

nehemiekoffi commented 4 months ago

Hello here, any feedback about Geopoint support on loopback 4 ?

mathieuBP commented 3 months ago

This is too bad... I hacked my way into the SQLConnector and now it works flawlessly. The hardest part was to circumvent the error thrown by the json-schema thing.

Instead of using geopoint type, I used object. Defining the mysql.dataType to point made the mysql column of the correct type. All that was left was to convert the GeoPoint object from the stringified object (because object types are stored as JSON) back to a correct value. Happens that the Connector is all ready to accept geopoint:

  if (prop.type.name === 'GeoPoint') {
    return new ParameterizedSQL({
      sql: 'Point(?,?)',
      params: [val.lng, val.lat],
    });
  }

But since we cannot use the GoePoint type, I changed it to use the dataType value, while also converting the string back to object.

  if (prop.type.name === 'GeoPoint' || getDataType(prop) === 'point') {
    // Parse the JSON because it is being stringified before.
    if (typeof val === 'string') {
      val = JSON.parse(val);
    }
    return new ParameterizedSQL({
      sql: 'Point(?,?)',
      params: [val.lng, val.lat],
    });
  }

That way I can do without using GeoPoint type.

That's very cumbersome and hackish. I think, since the issue is stale, that the loopback team should at least remove the blocker and just let it work as is, which is way better than telling your users to use another framework...