Skip to content

Add a data adapter implementation that infers fields from annotations or properties#1829

Merged
rfecher merged 2 commits intolocationtech:masterfrom
jdgarrett:pojo-adapter
Jul 22, 2021
Merged

Add a data adapter implementation that infers fields from annotations or properties#1829
rfecher merged 2 commits intolocationtech:masterfrom
jdgarrett:pojo-adapter

Conversation

@jdgarrett
Copy link
Copy Markdown
Contributor

No description provided.

… or properties

Signed-off-by: Johnathan Garrett <jd@prominentedge.com>
Copy link
Copy Markdown
Contributor

@rfecher rfecher left a comment

Choose a reason for hiding this comment

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

I had just a couple comments

assertTrue(
adapter.getFieldDescriptor("date").indexHints().contains(TimeField.TIME_DIMENSION_HINT));

// final TestType testEntry = new TestType("id1", 2.5, 8, true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why the commented out block?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, this was an oversight, I will update this test.

SpatialField.LONGITUDE_DIMENSION_HINT);
|| inputFieldDescriptors.get(1).indexHints().contains(
SpatialField.LATITUDE_DIMENSION_HINT)
|| inputFieldDescriptors.get(0).fieldName().equalsIgnoreCase("longitude")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we use inference on field names without hints in the case of an un-annotated POJO? I had assumed with an unannotated POJO you're stuck with default index behaviors as if there are no index hints, but in looking at this addition of checking for longitude field names am curious the reasoning if its not automatically inferring index mapping based on name?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Currently, index mappings are only inferred based on type. Logic could be added to the mapping code to pass all compatible fields to the mapper and let the mapper pick appropriate fields, but that is not currently in place.

…appers

Signed-off-by: Johnathan Garrett <jd@prominentedge.com>
@rfecher rfecher merged commit 42c2af1 into locationtech:master Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants