You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@baremaps.apache.org by "bchapuis (via GitHub)" <gi...@apache.org> on 2023/09/04 20:09:56 UTC

[GitHub] [incubator-baremaps] bchapuis opened a new pull request, #772: Expose the tileset and the SQL queries in dev mode

bchapuis opened a new pull request, #772:
URL: https://github.com/apache/incubator-baremaps/pull/772

   Solves #766.


-- 
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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#discussion_r1315783111


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TilesetResource.java:
##########
@@ -18,27 +18,28 @@
 import javax.ws.rs.GET;
 import javax.ws.rs.Produces;
 import javax.ws.rs.core.MediaType;
-import org.apache.baremaps.vectortile.tilejson.TileJSON;
+import org.apache.baremaps.vectortile.tileset.Tileset;
 
 /**
- * A resource that provides access to the tileset.
+ * A resource that provides access to the tileset file. Only suitable for development purposes, as
+ * it exposes SQL queries.

Review Comment:
   Did a draft implementation that reverse the logic in order to allow fields instead of having to disallow them after the facts. 
   https://github.com/apache/incubator-baremaps/pull/773 
   
   Motivation is similar to what was done in https://github.com/apache/incubator-baremaps/pull/658/files#diff-94ecab9df3004fae4fd05c3c610d8ef629f81cb775f72a53a19070c0293bc27cL62 to remove hiding of field with conditions after de-serialization, which could be error prone for newly added field in `tileset.json`.
   
   



-- 
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

[GitHub] [incubator-baremaps] bchapuis commented on a diff in pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#discussion_r1316817690


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TilesetResource.java:
##########
@@ -18,27 +18,28 @@
 import javax.ws.rs.GET;
 import javax.ws.rs.Produces;
 import javax.ws.rs.core.MediaType;
-import org.apache.baremaps.vectortile.tilejson.TileJSON;
+import org.apache.baremaps.vectortile.tileset.Tileset;
 
 /**
- * A resource that provides access to the tileset.
+ * A resource that provides access to the tileset file. Only suitable for development purposes, as
+ * it exposes SQL queries.

Review Comment:
   Sorry, my answer was vague, let's add a bit of context. First, our implementation of tileJSON is partial and I think that adding an extension to the model will slow us down. Second, dev modes are usually [insecure](https://quarkus.io/guides/dev-mode-differences) and I think it is reasonable to assume that our users will not use it in production. Finally, an earlier version of the dev mode offered a maputnik integration and allowed the user to edit the style. I have not completely abandonned this idea (see #561) and we may introduce an API to edit local files when in dev mode (the security aspects associated with this API still need to be discussed).



-- 
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

[GitHub] [incubator-baremaps] sonarcloud[bot] commented on pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#issuecomment-1706181620

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=772)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772) No Coverage information  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [incubator-baremaps] bchapuis merged pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis merged PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772


-- 
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

[GitHub] [incubator-baremaps] sonarcloud[bot] commented on pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#issuecomment-1705696631

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=772)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772) No Coverage information  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [incubator-baremaps] Perdjesk commented on a diff in pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "Perdjesk (via GitHub)" <gi...@apache.org>.
Perdjesk commented on code in PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#discussion_r1315547509


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TilesetResource.java:
##########
@@ -18,27 +18,28 @@
 import javax.ws.rs.GET;
 import javax.ws.rs.Produces;
 import javax.ws.rs.core.MediaType;
-import org.apache.baremaps.vectortile.tilejson.TileJSON;
+import org.apache.baremaps.vectortile.tileset.Tileset;
 
 /**
- * A resource that provides access to the tileset.
+ * A resource that provides access to the tileset file. Only suitable for development purposes, as
+ * it exposes SQL queries.

Review Comment:
   IIRC it exposes the database JDBC URL with credentials as well. Basically anything consumed by baremaps for its internal ends up deserialized in Tileset.



-- 
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

[GitHub] [incubator-baremaps] bchapuis commented on a diff in pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#discussion_r1315823869


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TilesetResource.java:
##########
@@ -18,27 +18,28 @@
 import javax.ws.rs.GET;
 import javax.ws.rs.Produces;
 import javax.ws.rs.core.MediaType;
-import org.apache.baremaps.vectortile.tilejson.TileJSON;
+import org.apache.baremaps.vectortile.tileset.Tileset;
 
 /**
- * A resource that provides access to the tileset.
+ * A resource that provides access to the tileset file. Only suitable for development purposes, as
+ * it exposes SQL queries.

Review Comment:
   @Perdjesk Thank you for the draft, I understand the motivation. However, I would prefer to keep things as simple and small as possible for now, i.e., to expose the tileset in development and the tileJSON in production.



-- 
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

[GitHub] [incubator-baremaps] sonarcloud[bot] commented on pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#issuecomment-1705674980

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=772)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=772&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=772&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772) No Coverage information  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=772&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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

[GitHub] [incubator-baremaps] bchapuis commented on a diff in pull request #772: Expose the tileset and the SQL queries in dev mode

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #772:
URL: https://github.com/apache/incubator-baremaps/pull/772#discussion_r1315568463


##########
baremaps-server/src/main/java/org/apache/baremaps/server/TilesetResource.java:
##########
@@ -18,27 +18,28 @@
 import javax.ws.rs.GET;
 import javax.ws.rs.Produces;
 import javax.ws.rs.core.MediaType;
-import org.apache.baremaps.vectortile.tilejson.TileJSON;
+import org.apache.baremaps.vectortile.tileset.Tileset;
 
 /**
- * A resource that provides access to the tileset.
+ * A resource that provides access to the tileset file. Only suitable for development purposes, as
+ * it exposes SQL queries.

Review Comment:
   @Perdjesk Good point, I just hid the jdbc url. We need to find a good balance between security and usability for the dev mode



-- 
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