V3 casl 1220 migrate dh connector tov3 #518
Conversation
Signed-off-by: Marcin-Here <[email protected]>
Signed-off-by: Marcin-Here <[email protected]>
| } | ||
|
|
||
| @JsonView(ExcludeFromHash.class) | ||
| @JsonProperty("streamId") |
There was a problem hiding this comment.
Do you use this class (com.here.naksha.lib.core.models.payload.Event) somehow?
Afaik most if not all of its subclasses are effectively dead I was strongly considering removing it.
There was a problem hiding this comment.
as discussed - check possibility of moving these events to storage-http module
There was a problem hiding this comment.
Those are coupled with other classes from lib.core.payload packages. Which are coupled with other classes from lib.core. That probably is doable but it would take longer time.
|
|
||
| public enum HttpInterface { | ||
| ffwAdapter, | ||
| dataHubConnector |
There was a problem hiding this comment.
I think it design decision from v2. Because we get that interface from storage properties like: "properties": { "httpInterface" : "dataHubConnector", so to not translate camelCase to UPPPER_SNAKE.
| public PropertyQuery() {} | ||
|
|
||
| public PropertyQuery(@NotNull String key, @NotNull QueryOperation op) {} | ||
| public PropertyQuery(@NotNull String key, @NotNull QueryOperation op) { |
There was a problem hiding this comment.
this class might also be moved to the http module as it serves only "dto" purposes
There was a problem hiding this comment.
Those are coupled with other classes from lib.core.payload packages. Which are coupled with other classes from lib.core. That probably is doable but it would take longer time.
| setQueryParameters(Map.of(key, parameters)); | ||
| return this; | ||
| } | ||
| getQueryParameters().put(key, parameters); |
There was a problem hiding this comment.
nth: getQueryParameters could be called once in this method
...naksha-storage-http/src/jvmMain/java/com/here/naksha/storage/http/HttpStorageProperties.java
Show resolved
Hide resolved
| event.setQuadkey(tileAddress.asQuadkey()); | ||
| return event; | ||
| } else { | ||
| throw new NotImplementedException("Tile type other than " + TILE_TYPE_QUADKEY); |
There was a problem hiding this comment.
Maybe let's stick to NakshaException
| private void assertNoUuid(NakshaFeature feature) { | ||
| String id = feature.getId(); | ||
| if (feature.getProperties().getXyz().getUuid() != null) { | ||
| throw new IllegalArgumentException("The feature with id " + id + " cannot be created. " |
There was a problem hiding this comment.
Are we recapturing these exceptions? If not, I would use NakshaException if there's no bigger issue with that
| } | ||
| } | ||
|
|
||
| private NakshaFeatureList getFeaturesFromDb(List<String> featureIds) { |
There was a problem hiding this comment.
So in getFeaturesFromDb we actually execute a HTTP call to a REST service? 🙃
| private final WriteRequest request; | ||
| private final RequestSender sender; | ||
| private final String endpoint; | ||
| private final Map<String, NakshaFeature> databaseFeaturesCache = new HashMap<>(); |
There was a problem hiding this comment.
Are we sure that in V3, with TupleCache and all, this cache is required as well? It's worth checking if we're not storing the same stuff in memory twice
There was a problem hiding this comment.
I think TupleCache store to cache only on PG calls. In HttpStorage we have constant 21 features in cache, which are defaults from admin storage and storage, handler and space that we are using for DH connector.
|
|
||
| private IPropertyQueryToPropertiesQuery() {} | ||
|
|
||
| public static PropertyQueryOr toPopQueryOr(@NotNull IPropertyQuery query) { |
There was a problem hiding this comment.
This public utility method has no tests
Signed-off-by: Marcin-Here <[email protected]>
* CASL-1220: migrated DH connector to v3. Signed-off-by: Marcin-Here <[email protected]> * CASL-1220: added empty tags handling removed unused imports Signed-off-by: Marcin-Here <[email protected]> * CASL-1220: review comments fixes. Signed-off-by: Marcin-Here <[email protected]> --------- Signed-off-by: Marcin-Here <[email protected]>
2 participants
No description provided.