[GitHub] [incubator-baremaps] Drabble commented on a diff in pull request #529: Improve IP to location with a Workflow and a simple web application

[GitBox <gi...@apache.org>]

Drabble commented on code in PR #529:
URL: https://github.com/apache/incubator-baremaps/pull/529#discussion_r1044347203


##########
baremaps-core/src/main/java/org/apache/baremaps/workflow/tasks/CreateIplocIndex.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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.
+ */
+
+package org.apache.baremaps.workflow.tasks;
+
+import org.apache.baremaps.geocoder.geonames.GeonamesGeocoder;
+import org.apache.baremaps.iploc.IpLoc;
+import org.apache.baremaps.iploc.data.IpLocStats;
+import org.apache.baremaps.iploc.database.SqliteUtils;
+import org.apache.baremaps.iploc.nic.NicObject;
+import org.apache.baremaps.iploc.nic.NicParser;
+import org.apache.baremaps.workflow.Task;
+import org.apache.baremaps.workflow.WorkflowContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.stream.Stream;
+
+public record CreateIplocIndex(String geonamesIndexPath, String[] iplocNicPath, String targetIplocIndexPath) implements Task {
+
+  private static final Logger logger = LoggerFactory.getLogger(CreateIplocIndex.class);
+
+  @Override
+  public void execute(WorkflowContext context) throws Exception {
+    logger.info("Generating Iploc from {} {}", geonamesIndexPath, iplocNicPath);
+
+    logger.info("Creating the Geocoder");
+    GeonamesGeocoder geocoder;
+    try {
+      geocoder = new GeonamesGeocoder(Path.of(geonamesIndexPath), null);
+      if (!geocoder.indexExists()) {
+        logger.error("Geocoder index doesn't exist");
+        return;
+      }
+      geocoder.open();
+    } catch(Exception e) {
+      logger.error("Error while creating the geocoder index", e);
+      return;
+    }
+
+    logger.info("Generating NIC objects stream");
+    Stream<NicObject> fetchNicObjectStream = Arrays.stream(iplocNicPath).flatMap(iplocNicPath -> {
+      try {
+        InputStream inputStream = new BufferedInputStream(Files.newInputStream(Path.of(iplocNicPath)));
+        return NicParser.parse(inputStream).onClose(() -> {

Review Comment:
   There is something that doesn't make sense here to me. This is why I changed it from try-with-resources to a simple try.
   
   As defined by oracle `The try-with-resources statement ensures that each resource is closed at the end of the statement.`. However in our case, we want to close the InputStream in the callback from the NicParser which may be Lazy and thus execute after the end of the try. 
   
   What do you think?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org