Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Having an uber pom for better build #9

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Having an uber pom for better build #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7dee83
Having an uber pom for better build
bageshwar Jan 15, 2025
c7c3068
Update maven.yml
bageshwar Jan 15, 2025
9291704
Ignore depgraph-maven-plugin temporarily
bageshwar Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/workflows/maven.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ jobs:

steps:
- uses: actions/checkout@v4
- name: Set up JDK 8
- name: Set up JDK 17
uses: actions/setup-java@v4
with:
java-version: '8'
java-version: '17'
distribution: 'temurin'
Comment on lines +24 to 28
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

JDK 17 upgrade requires source/target updates in all modules

The codebase is currently standardized on Java 8 across all modules. Upgrading the GitHub Actions environment to JDK 17 while the project is configured to use Java 8 may lead to build issues. You need to:

  • Update <source>8</source> and <target>8</target> to 17 in all module POMs:
    • pibify-core/pom.xml
    • pibify-maven-plugin/pom.xml
    • test-environments/vertx/pom.xml
    • test-environments/lombok/pom.xml
    • test-environments/dropwizard/test-dropwizard/pom.xml
    • test-environments/pibify-test-vanilla/pom.xml
🔗 Analysis chain

Verify JDK 17 compatibility across all modules.

The upgrade from JDK 8 to 17 is a significant change. While moving to a modern Java version is good, we should ensure all modules and dependencies are compatible with JDK 17.

Let's verify the Java version requirements in the new uber pom:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check Java version requirements in pom files
# Look for maven.compiler.source, maven.compiler.target, and java.version properties

# Check root pom
echo "Checking root pom.xml:"
rg -A 2 '<(maven\.compiler\.(source|target)|java\.version)>' pom.xml

# Check all module poms
echo -e "\nChecking module pom files:"
fd pom.xml --exclude target/ --exec rg -A 2 '<(maven\.compiler\.(source|target)|java\.version)>'

Length of output: 293

Script:

#!/bin/bash
# Search for Java version related configurations more broadly
echo "=== Checking Java version configurations in pom files ==="
fd pom.xml --exclude target/ --exec sh -c 'echo "\nFile: {}"; cat {} | grep -A 5 -E "(<properties>|<plugin>.*compiler|<parent>)"'

# Search for any maven compiler plugin configurations
echo -e "\n=== Checking specific compiler configurations ==="
rg -A 10 '<artifactId>maven-compiler-plugin</artifactId>'

Length of output: 13081

cache: maven
- name: Build with Maven
run: mvn -B package --file pom.xml

# Optional: Uploads the full dependency graph to GitHub to improve the quality of Dependabot alerts this repository can receive
- name: Update dependency graph
uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6
# Disabled temporarily
# - name: Update dependency graph
# uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6
7 changes: 6 additions & 1 deletion pibify-core/src/main/java/com/flipkart/pibify/core/PibifyConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@

package com.flipkart.pibify.core;

import java.util.logging.Logger;

/**
* This class is used for configuring pibify
* Author bageshwar.pn
* Date 24/09/24
*/
public class PibifyConfiguration {

private static final Logger logger = Logger.getLogger(PibifyConfiguration.class.getName());


private static PibifyConfiguration INSTANCE;
private boolean ignoreUnknownFields = true;
private boolean ignoreUnknownEnums = true;
Expand Down Expand Up @@ -61,7 +66,7 @@ public Builder() {
synchronized public void build() {
// A way to keep the static instance pseudo-final
if (PibifyConfiguration.INSTANCE != null) {
throw new IllegalStateException("Re-creating pibify configuration");
logger.warning("Re-creating pibify configuration");
}
PibifyConfiguration.INSTANCE = config;
Comment on lines 68 to 71
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Reconsider replacing exception with warning log

Replacing the exception with a warning log when recreating configuration could lead to silent failures and unexpected behavior. Consider:

  1. Either keeping the exception to prevent reconfiguration
  2. Or implementing a proper reconfiguration mechanism with validation
 if (PibifyConfiguration.INSTANCE != null) {
-    logger.warning("Re-creating pibify configuration");
+    throw new IllegalStateException("Configuration already initialized. Use reset() method for reconfiguration.");
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (PibifyConfiguration.INSTANCE != null) {
throw new IllegalStateException("Re-creating pibify configuration");
logger.warning("Re-creating pibify configuration");
}
PibifyConfiguration.INSTANCE = config;
if (PibifyConfiguration.INSTANCE != null) {
throw new IllegalStateException("Configuration already initialized. Use reset() method for reconfiguration.");
}
PibifyConfiguration.INSTANCE = config;

}
Expand Down
38 changes: 38 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!--
~ /*
~ *Copyright [2025] [Original Author]
~ *
~ * 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.
~ */
-->

<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://maven.apache.org/POM/4.0.0"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>com.flipkart.pibify</groupId>
<artifactId>pibify</artifactId>
<packaging>pom</packaging>
<version>1.6</version>
<name>pibify</name>
<properties>
<protobuf-java.version>4.28.2</protobuf-java.version>
</properties>
<modules>
<module>pibify-core</module>
<module>pibify-maven-plugin</module>
<module>test-environments/pibify-test-vanilla</module>
<module>test-environments/dropwizard/test-dropwizard</module>
<module>test-environments/lombok</module>
<module>test-environments/vertx</module>
</modules>
</project>
4 changes: 2 additions & 2 deletions test-environments/dropwizard/test-dropwizard/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@

<modelVersion>4.0.0</modelVersion>

<groupId>pibify</groupId>
<groupId>com.pibify</groupId>
<artifactId>test-dropwizard</artifactId>
<version>1.0-SNAPSHOT</version>
<packaging>jar</packaging>

<name>PibifyDemo</name>
<name>pibify-test-dropwizard</name>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
Expand Down
10 changes: 6 additions & 4 deletions ...s/dropwizard/test-dropwizard/src/main/java/com/flipkart/pibify/PibifyDemoApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package com.flipkart.pibify;

import com.flipkart.pibify.codegen.stub.AbstractPibifyHandlerCache;
import com.flipkart.pibify.dropwizard.JakartaPibifyMessageBodyWriter;
import com.flipkart.pibify.paritychecker.IParityChecker;
import com.flipkart.pibify.paritychecker.IParityCheckerListener;
Expand All @@ -29,7 +30,6 @@
import io.dropwizard.core.Application;
import io.dropwizard.core.setup.Bootstrap;
import io.dropwizard.core.setup.Environment;
import pibify.generated.pibify.PibifyHandlerCache;

import java.util.Optional;

Expand All @@ -55,10 +55,12 @@ public void initialize(final Bootstrap<PibifyDemoConfiguration> bootstrap) {
public void run(final PibifyDemoConfiguration configuration,
final Environment environment) {

environment.jersey().register(new JakartaPibifyMessageBodyWriter(PibifyHandlerCache.getInstance(), new PibifySampler()));
AbstractPibifyHandlerCache handlerCache = AbstractPibifyHandlerCache.getConcreteInstance("com.pibify.pibify.generated.test.dropwizard.PibifyHandlerCache");
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid hardcoding class names as strings

Using hardcoded class name strings can lead to runtime errors if the class name changes. Consider using Class.getName() instead.

-AbstractPibifyHandlerCache handlerCache = AbstractPibifyHandlerCache.getConcreteInstance("com.pibify.pibify.generated.test.dropwizard.PibifyHandlerCache");
+Class<?> handlerCacheClass = Class.forName("com.pibify.pibify.generated.test.dropwizard.PibifyHandlerCache");
+AbstractPibifyHandlerCache handlerCache = AbstractPibifyHandlerCache.getConcreteInstance(handlerCacheClass.getName());

Committable suggestion skipped: line range outside the PR's diff.


environment.jersey().register(new JakartaPibifyMessageBodyWriter(handlerCache, new PibifySampler()));
environment.jersey().register(new SampleResource());
environment.jersey().register(new JakartaParityCheckerResource(PibifyHandlerCache.getInstance()));
IParityChecker parityChecker = new PibifyParityChecker(PibifyHandlerCache.getInstance(), new ParityCheckerListener(),
environment.jersey().register(new JakartaParityCheckerResource(handlerCache));
IParityChecker parityChecker = new PibifyParityChecker(handlerCache, new ParityCheckerListener(),
Optional.of(() -> null), new PibifySampler());
environment.jersey().register(new JakartaJsonResponseFilter(parityChecker));
}
Expand Down
2 changes: 1 addition & 1 deletion test-environments/lombok/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
<plugin>
<groupId>com.flipkart.pibify</groupId>
<artifactId>pibify-maven-plugin</artifactId>
<version>1.1</version>
<version>1.6</version>
<executions>
<execution>
<id>generate-sources</id>
Expand Down
9 changes: 8 additions & 1 deletion test-environments/vertx/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@
<plugin>
<groupId>com.flipkart.pibify</groupId>
<artifactId>pibify-maven-plugin</artifactId>
<version>1.3</version>
<version>1.6</version>
<dependencies>
<dependency>
<groupId>io.vertx</groupId>
<artifactId>vertx-core</artifactId>
<version>4.5.9</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>generate-sources</id>
Expand Down
13 changes: 12 additions & 1 deletion test-environments/vertx/src/main/java/com/flipkart/pibify/Product.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,25 @@

package com.flipkart.pibify;

import com.flipkart.pibify.core.Pibify;

/**
* This class is used for
* Author bageshwar.pn
* Date 02/10/24
*/
public class Product {
String id, name;

@Pibify(1)
String id;

@Pibify(2)
String name;

@Pibify(3)
int weight;

@Pibify(4)
double price;

public Product(String id, String name, double price, int weight) {
Expand Down
Loading