[GitHub] [streams] b-hodge commented on a diff in pull request #529: resolves STREAMS-683: upgrade jsonschema2pojo usage to modern version

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

b-hodge commented on code in PR #529:
URL: https://github.com/apache/streams/pull/529#discussion_r970276494


##########
streams-util/src/main/java/org/apache/streams/util/schema/SchemaStoreImpl.java:
##########
@@ -63,7 +63,7 @@ public synchronized Schema create(URI uri) {
       if (uri.toString().contains("#") && !uri.toString().endsWith("#")) {
         Schema newSchema = new Schema(baseUri, baseNode, null, true);
         this.schemas.put(baseUri, newSchema);
-        JsonNode childContent = this.fragmentResolver.resolve(baseNode, '#' + StringUtils.substringAfter(uri.toString(), "#"));
+        JsonNode childContent = this.fragmentResolver.resolve(baseNode, '#' + StringUtils.substringAfter(uri.toString(), "#"), "#");

Review Comment:
   What's going on here? Is this change related to the `jsonschema2pojo` version bump, or something else?



##########
streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/serializer/util/InstagramActivityUtil.java:
##########
@@ -21,18 +21,11 @@
 
 import org.apache.streams.exceptions.ActivityConversionException;
 import org.apache.streams.exceptions.ActivitySerializerException;
-import org.apache.streams.instagram.pojo.Comment;
-import org.apache.streams.instagram.pojo.Comments;
-import org.apache.streams.instagram.pojo.Images;
-import org.apache.streams.instagram.pojo.Media;
-import org.apache.streams.instagram.pojo.UserInfo;
-import org.apache.streams.instagram.pojo.UserInfoCounts;
-import org.apache.streams.instagram.pojo.Videos;
+import org.apache.streams.instagram.pojo.*;

Review Comment:
   What's the reason for not continuing to enumerate imports of specific classes? Are there possible future cases where importing all classes matching the wildcard would be incorrect or dangerous?



##########
streams-contrib/streams-provider-instagram/src/main/java/org/apache/streams/instagram/serializer/util/InstagramActivityUtil.java:
##########
@@ -21,18 +21,11 @@
 
 import org.apache.streams.exceptions.ActivityConversionException;
 import org.apache.streams.exceptions.ActivitySerializerException;
-import org.apache.streams.instagram.pojo.Comment;
-import org.apache.streams.instagram.pojo.Comments;
-import org.apache.streams.instagram.pojo.Images;
-import org.apache.streams.instagram.pojo.Media;
-import org.apache.streams.instagram.pojo.UserInfo;
-import org.apache.streams.instagram.pojo.UserInfoCounts;
-import org.apache.streams.instagram.pojo.Videos;
+import org.apache.streams.instagram.pojo.*;
 import org.apache.streams.pojo.extensions.ExtensionUtil;
 import org.apache.streams.pojo.json.Activity;
 import org.apache.streams.pojo.json.ActivityObject;
 import org.apache.streams.pojo.json.Image;
-import org.apache.streams.pojo.json.ImageParent;

Review Comment:
   How does this change relate to the version upgrade of `jsonschema2pojo` dependency?
   
   I can't find this class anywhere in the `streams` repo, but also don't see evidence of it being removed in this PR. Was this dependency coming from the `org.apache.streams.plugins_streams-plugin-pojo` Maven dependency that's also being removed?



##########
streams-contrib/streams-provider-linkedin/pom.xml:
##########
@@ -138,36 +138,6 @@
             </testResource>
         </testResources>
         <plugins>
-            <plugin>
-                <groupId>org.apache.streams.plugins</groupId>
-                <artifactId>streams-plugin-pojo</artifactId>
-                <version>${project.version}</version>
-                <configuration>
-                    <sourcePaths>
-                        <sourcePath>${project.basedir}/src/main/jsonschema</sourcePath>
-                    </sourcePaths>
-                    <targetDirectory>${project.basedir}/target/generated-sources/pojo</targetDirectory>
-                    <targetPackage>org.apache.streams.linkedin.pojo</targetPackage>
-                </configuration>
-            </plugin>
-            <plugin>
-                <groupId>org.codehaus.mojo</groupId>

Review Comment:
   What's the reason for removing this dependency?



##########
streams-contrib/streams-provider-linkedin/pom.xml:
##########
@@ -138,36 +138,6 @@
             </testResource>
         </testResources>
         <plugins>
-            <plugin>
-                <groupId>org.apache.streams.plugins</groupId>

Review Comment:
   What's the reason for removing this dependency?



##########
streams-plugins/streams-plugin-pojo/src/main/java/org/apache/streams/plugins/StreamsPojoGenerationConfig.java:
##########
@@ -106,10 +106,10 @@ public boolean isIncludeJsr303Annotations() {
     return true;
   }
 
-  @Override
-  public boolean isUseCommonsLang3() {
-    return true;
-  }
+//  @Override

Review Comment:
   1. What _was_ this function being used for? I don't see any references to it elsewhere in the repo.
   
   2. Why comment-out instead of just removing the function entirely?



-- 
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: dev-unsubscribe@streams.apache.org

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