CCSD-676 changes#387
Conversation
nitish-egov
requested review from
kavi-egov,
pradeepkumarcm-egov and
vinothrallapalli-eGov
as code owners
WalkthroughVacant-land handling in EstimationService now selects the first matching billing slab (errors only if none); TranslationService adds null-guards before splitting Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant EstimationService
participant BillingSlabRepo
Client->>EstimationService: getEstimationMap(property)
alt property is Vacant Land
EstimationService->>BillingSlabRepo: query matching slabs
BillingSlabRepo-->>EstimationService: matchingSlabs[]
alt matchingSlabs empty
EstimationService-->>Client: throw "No billing slab found for vacant land"
else matchingSlabs non-empty
EstimationService->>EstimationService: select first slab (index 0)
EstimationService->>EstimationService: tax = unitRate * landArea
EstimationService-->>Client: return estimation with tax
end
else not Vacant Land
EstimationService-->>Client: return estimation per existing non-vacant flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/EstimationService.java(1 hunks)
| if (filteredBillingSlabs.isEmpty()) | ||
| throw new CustomException(PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT, "No billing slab found for vacant land"); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Error code constant looks misspelled and may not exist; verify and standardize.
The code uses PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT (note “VACANCT”). If this constant does not exist, this won’t compile. Recommend PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT (or reuse an existing standard error code used for slab mismatches) and include tenantId/propertyId in the message for observability.
Run the following to verify constants and downstream consumption of billingSlabIds:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check if the error code constants exist
rg -nP '\bPT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT\b|\bPT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT\b' -C2
# 2) Inspect where billingSlabIds is consumed and whether callers expect "id|index" format
rg -nP '\bbillingSlabIds\b' -C3
# 3) Confirm we already define a land-area-null error code (adjust if needed)
rg -nP '\bPT_ESTIMATE_LANDAREA_NULL\b|\bLAND.?AREA.*NULL\b' -C2 --type=javaLength of output: 61575
Fix typo in error code constant & standardize exception
The constant PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT is defined (so it compiles) but its name is misspelled and its value is a generic code. Please:
- Rename the constant in
CalculatorConstants.java(around line 285):- From:
public static final String PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT = "PT_ESTIMATE_BILLINGSLABS_UNMATCH";
- To:
public static final String PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT = "PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT";
- From:
- Update its usage in
EstimationService.java(line 192):- From:
throw new CustomException(PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT, "No billing slab found for vacant land");
- To:
throw new CustomException(PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT, String.format("No billing slab found for vacant land [tenant=%s, propertyId=%s]", tenantId, detail.getPropertyId()));
- From:
- (Optional) If you prefer to keep the generic code string, you can still rename the constant name for clarity and add a new, specific message constant for the vacant‐land case.
- There is currently no
PT_ESTIMATE_LANDAREA_NULLconstant—if you need to guard against null land‐area values, define and throw a dedicated error code (e.g.PT_ESTIMATE_LANDAREA_NULL) before calculating tax.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/TranslationService.java (5)
66-69: Potential NPE: address/locality accessed without null checks
property.getAddress()or.getLocality()can be null; this will crash. Guard and still emit an emptylocalityobject for schema stability.- Map<String, Object> addressMap = new HashMap(); - Map<String, Object> localityMap = new HashMap(); - localityMap.put("area",property.getAddress().getLocality().getArea()); - localityMap.put("code",property.getAddress().getLocality().getCode()); - addressMap.put("locality",localityMap); + Map<String, Object> addressMap = new HashMap<>(); + Map<String, Object> localityMap = new HashMap<>(); + addressMap.put("locality", localityMap); + if (property.getAddress() != null && property.getAddress().getLocality() != null) { + localityMap.put("area", property.getAddress().getLocality().getArea()); + localityMap.put("code", property.getAddress().getLocality().getCode()); + }
134-134: Potential NPE:unit.getConstructionDetail()may be nullNull construction detail will crash when reading built-up area. Guard it.
- unitMap.put("unitArea", unit.getConstructionDetail().getBuiltUpArea()); + if (unit.getConstructionDetail() != null) { + unitMap.put("unitArea", unit.getConstructionDetail().getBuiltUpArea()); + }
120-126: Guard owners iteration against nullIf
property.getOwners()is null, this will crash. Keep payload stable by emitting empty list when absent.- property.getOwners().forEach(ownerInfo -> { - Map<String, Object> owner = mapper.convertValue(ownerInfo, new TypeReference<Map<String, Object>>() {}); - owners.add(owner); - }); + if (!CollectionUtils.isEmpty(property.getOwners())) { + property.getOwners().forEach(ownerInfo -> { + Map<String, Object> owner = mapper.convertValue(ownerInfo, new TypeReference<Map<String, Object>>() {}); + owners.add(owner); + }); + }
162-165: PreferdecimalValue()overdoubleValue()to avoid precision lossMoney amounts should not go through double. Read BigDecimal directly from the JSON node.
- BigDecimal adhocExemption = new BigDecimal(assessment.getAdditionalDetails().get(ADHOC_REBATE_KEY).doubleValue()); + BigDecimal adhocExemption = assessment.getAdditionalDetails().get(ADHOC_REBATE_KEY).decimalValue(); ... - BigDecimal adhocPenalty = new BigDecimal(assessment.getAdditionalDetails().get(ADHOC_PENALTY_KEY).doubleValue()); + BigDecimal adhocPenalty = assessment.getAdditionalDetails().get(ADHOC_PENALTY_KEY).decimalValue();Also applies to: 171-174
61-65: Eliminate rawHashMapusageUse the diamond operator to avoid unchecked-conversion warnings.
- Map<String, Object> propertyMap = new HashMap(); + Map<String, Object> propertyMap = new HashMap<>(); ... - Map<String, Object> addressMap = new HashMap(); - Map<String, Object> localityMap = new HashMap(); + Map<String, Object> addressMap = new HashMap<>(); + Map<String, Object> localityMap = new HashMap<>();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/TranslationService.java(2 hunks)
| if(property.getPropertyType() != null) { | ||
| String[] propertyTypeMasterData = property.getPropertyType().split("\\."); | ||
| propertyType = propertyTypeMasterData[0]; | ||
| if(propertyTypeMasterData.length > 1) | ||
| propertySubType = propertyTypeMasterData[1]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick
Avoid empty subtype on trailing dots; trim and write only non-blank parts
Current split can write "" when value is "FOO." or whitespace. Write only non-blank, trimmed segments.
- if(property.getPropertyType() != null) {
- String[] propertyTypeMasterData = property.getPropertyType().split("\\.");
- propertyType = propertyTypeMasterData[0];
- if(propertyTypeMasterData.length > 1)
- propertySubType = propertyTypeMasterData[1];
- }
+ if (StringUtils.hasText(property.getPropertyType())) {
+ String[] parts = property.getPropertyType().trim().split("\\.", -1);
+ propertyType = StringUtils.hasText(parts[0]) ? parts[0].trim() : null;
+ if (parts.length > 1 && StringUtils.hasText(parts[1])) {
+ propertySubType = parts[1].trim();
+ }
+ }Add import (outside this hunk):
import org.springframework.util.StringUtils;🤖 Prompt for AI Agents
In
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/TranslationService.java
around lines 80 to 85, the current split on property.getPropertyType() can
produce empty or whitespace-only segments (e.g. "FOO."), so trim and only assign
non-blank parts: after splitting, trim each segment and use
StringUtils.hasText(...) to set propertyType and propertySubType only when the
trimmed segment contains text; if the first non-blank segment exists set
propertyType, and if a second non-blank segment exists set propertySubType. Also
add the import "import org.springframework.util.StringUtils;" at the top of the
file (outside this hunk).
🧹 Nitpick
DRY: extract a small helper to split dot-codes safely
Four blocks repeat the same split-and-assign pattern. A tiny helper reduces bugs and keeps behavior consistent.
Example helper (outside this class’ methods):
private static List<String> safeSegments(String code, int maxSegments) {
if (!StringUtils.hasText(code)) return Collections.emptyList();
String[] parts = code.trim().split("\\.", -1);
List<String> out = new ArrayList<>(Math.min(parts.length, maxSegments));
for (int i = 0; i < Math.min(parts.length, maxSegments); i++) {
if (StringUtils.hasText(parts[i])) out.add(parts[i].trim());
else out.add(null);
}
return out;
}Then use safeSegments(code, 2) for property/usage/ownership and safeSegments(code, 4) for units, writing only non-null items to the map.
Also applies to: 88-93, 96-101, 138-152
🤖 Prompt for AI Agents
In
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/TranslationService.java
around lines 80-85 (also applies to 88-93, 96-101, 138-152), the same
dot-splitting and index-safe assignment pattern is duplicated; extract a small
reusable helper (e.g., safeSegments(code, maxSegments)) that trims, handles
null/empty, splits with a limit, and returns a predictable list/array of segment
values (including nulls for empty segments) and replace each duplicated block to
call that helper and assign segments[0], segments[1], etc., only when present;
ensure the helper is static/private at class level, use it with the appropriate
maxSegments for each use-site, and update the assignments to use the helper
output to keep behavior consistent and avoid IndexOutOfBounds or inconsistent
trimming.
| if(property.getUsageCategory() != null) { | ||
| String[] usageCategoryMasterData = property.getUsageCategory().split("\\."); | ||
| usageCategoryMajor = usageCategoryMasterData[0]; | ||
| if(usageCategoryMasterData.length > 1) | ||
| usageCategoryMinor = usageCategoryMasterData[1]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick
Same handling for usageCategory: ignore blanks and trailing dots
Prevent writing empty strings into major/minor.
- if(property.getUsageCategory() != null) {
- String[] usageCategoryMasterData = property.getUsageCategory().split("\\.");
- usageCategoryMajor = usageCategoryMasterData[0];
- if(usageCategoryMasterData.length > 1)
- usageCategoryMinor = usageCategoryMasterData[1];
- }
+ if (StringUtils.hasText(property.getUsageCategory())) {
+ String[] parts = property.getUsageCategory().trim().split("\\.", -1);
+ usageCategoryMajor = StringUtils.hasText(parts[0]) ? parts[0].trim() : null;
+ if (parts.length > 1 && StringUtils.hasText(parts[1])) {
+ usageCategoryMinor = parts[1].trim();
+ }
+ }📝 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.
| if(property.getUsageCategory() != null) { | |
| String[] usageCategoryMasterData = property.getUsageCategory().split("\\."); | |
| usageCategoryMajor = usageCategoryMasterData[0]; | |
| if(usageCategoryMasterData.length > 1) | |
| usageCategoryMinor = usageCategoryMasterData[1]; | |
| } | |
| if (StringUtils.hasText(property.getUsageCategory())) { | |
| String[] parts = property.getUsageCategory().trim().split("\\.", -1); | |
| usageCategoryMajor = StringUtils.hasText(parts[0]) ? parts[0].trim() : null; | |
| if (parts.length > 1 && StringUtils.hasText(parts[1])) { | |
| usageCategoryMinor = parts[1].trim(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/TranslationService.java
around lines 88 to 93, the current usageCategory split can assign empty strings
to usageCategoryMajor/Minor when input contains blanks or trailing/leading dots;
update the logic to ignore empty segments: trim the usageCategory, split on
"\\.", then iterate segments to pick the first non-empty token as
usageCategoryMajor and the next non-empty token as usageCategoryMinor (skip
empty tokens produced by consecutive/trailing/leading dots); ensure you only
assign when the token has text (use StringUtils.hasText or
token.trim().isEmpty() check) so blanks are never written into major/minor.
| if(property.getOwnershipCategory() != null) { | ||
| String[] ownershipCategoryMasterData = property.getOwnershipCategory().split("\\."); | ||
| ownershipCategory = ownershipCategoryMasterData[0]; | ||
| if(ownershipCategoryMasterData.length > 1) | ||
| subOwnershipCategory = ownershipCategoryMasterData[1]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick
Same handling for ownershipCategory/subOwnershipCategory
Avoid empty strings when input has trailing dot or whitespace.
- if(property.getOwnershipCategory() != null) {
- String[] ownershipCategoryMasterData = property.getOwnershipCategory().split("\\.");
- ownershipCategory = ownershipCategoryMasterData[0];
- if(ownershipCategoryMasterData.length > 1)
- subOwnershipCategory = ownershipCategoryMasterData[1];
- }
+ if (StringUtils.hasText(property.getOwnershipCategory())) {
+ String[] parts = property.getOwnershipCategory().trim().split("\\.", -1);
+ ownershipCategory = StringUtils.hasText(parts[0]) ? parts[0].trim() : null;
+ if (parts.length > 1 && StringUtils.hasText(parts[1])) {
+ subOwnershipCategory = parts[1].trim();
+ }
+ }📝 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.
| if(property.getOwnershipCategory() != null) { | |
| String[] ownershipCategoryMasterData = property.getOwnershipCategory().split("\\."); | |
| ownershipCategory = ownershipCategoryMasterData[0]; | |
| if(ownershipCategoryMasterData.length > 1) | |
| subOwnershipCategory = ownershipCategoryMasterData[1]; | |
| } | |
| if (StringUtils.hasText(property.getOwnershipCategory())) { | |
| String[] parts = property.getOwnershipCategory().trim().split("\\.", -1); | |
| ownershipCategory = StringUtils.hasText(parts[0]) ? parts[0].trim() : null; | |
| if (parts.length > 1 && StringUtils.hasText(parts[1])) { | |
| subOwnershipCategory = parts[1].trim(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/TranslationService.java
around lines 96-101, the current split on ownershipCategory can produce empty
strings for trailing dots or surrounding whitespace; trim the raw value first,
return early if empty, then split on "\\.", filter out any empty/blank tokens
(trimming each token) and assign ownershipCategory = first token and
subOwnershipCategory = second token only if it exists and is non-empty; ensure
no empty strings are assigned.
| if(unit.getUsageCategory() != null) { | ||
| String[] masterData = unit.getUsageCategory().split("\\."); | ||
|
|
||
| if(masterData.length >= 1) | ||
| unitMap.put("usageCategoryMajor", masterData[0]); | ||
| if(masterData.length >= 1) | ||
| unitMap.put("usageCategoryMajor", masterData[0]); | ||
|
|
||
| if(masterData.length >= 2) | ||
| unitMap.put("usageCategoryMinor", masterData[1]); | ||
| if(masterData.length >= 2) | ||
| unitMap.put("usageCategoryMinor", masterData[1]); | ||
|
|
||
| if(masterData.length >= 3) | ||
| unitMap.put("usageCategorySubMinor", masterData[2]); | ||
| if(masterData.length >= 3) | ||
| unitMap.put("usageCategorySubMinor", masterData[2]); | ||
|
|
||
| if(masterData.length >= 4) | ||
| unitMap.put("usageCategoryDetail",masterData[3]); | ||
| if(masterData.length >= 4) | ||
| unitMap.put("usageCategoryDetail",masterData[3]); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick
Unit usageCategory: write only non-blank segments; trim input
Prevents empty values when usageCategory has trailing dots or blanks; reduces noisy payloads.
- if(unit.getUsageCategory() != null) {
- String[] masterData = unit.getUsageCategory().split("\\.");
-
- if(masterData.length >= 1)
- unitMap.put("usageCategoryMajor", masterData[0]);
-
- if(masterData.length >= 2)
- unitMap.put("usageCategoryMinor", masterData[1]);
-
- if(masterData.length >= 3)
- unitMap.put("usageCategorySubMinor", masterData[2]);
-
- if(masterData.length >= 4)
- unitMap.put("usageCategoryDetail",masterData[3]);
- }
+ if (StringUtils.hasText(unit.getUsageCategory())) {
+ String[] parts = unit.getUsageCategory().trim().split("\\.", -1);
+ if (parts.length > 0 && StringUtils.hasText(parts[0])) unitMap.put("usageCategoryMajor", parts[0].trim());
+ if (parts.length > 1 && StringUtils.hasText(parts[1])) unitMap.put("usageCategoryMinor", parts[1].trim());
+ if (parts.length > 2 && StringUtils.hasText(parts[2])) unitMap.put("usageCategorySubMinor", parts[2].trim());
+ if (parts.length > 3 && StringUtils.hasText(parts[3])) unitMap.put("usageCategoryDetail", parts[3].trim());
+ }📝 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.
| if(unit.getUsageCategory() != null) { | |
| String[] masterData = unit.getUsageCategory().split("\\."); | |
| if(masterData.length >= 1) | |
| unitMap.put("usageCategoryMajor", masterData[0]); | |
| if(masterData.length >= 1) | |
| unitMap.put("usageCategoryMajor", masterData[0]); | |
| if(masterData.length >= 2) | |
| unitMap.put("usageCategoryMinor", masterData[1]); | |
| if(masterData.length >= 2) | |
| unitMap.put("usageCategoryMinor", masterData[1]); | |
| if(masterData.length >= 3) | |
| unitMap.put("usageCategorySubMinor", masterData[2]); | |
| if(masterData.length >= 3) | |
| unitMap.put("usageCategorySubMinor", masterData[2]); | |
| if(masterData.length >= 4) | |
| unitMap.put("usageCategoryDetail",masterData[3]); | |
| if(masterData.length >= 4) | |
| unitMap.put("usageCategoryDetail",masterData[3]); | |
| } | |
| if (StringUtils.hasText(unit.getUsageCategory())) { | |
| String[] parts = unit.getUsageCategory().trim().split("\\.", -1); | |
| if (parts.length > 0 && StringUtils.hasText(parts[0])) unitMap.put("usageCategoryMajor", parts[0].trim()); | |
| if (parts.length > 1 && StringUtils.hasText(parts[1])) unitMap.put("usageCategoryMinor", parts[1].trim()); | |
| if (parts.length > 2 && StringUtils.hasText(parts[2])) unitMap.put("usageCategorySubMinor", parts[2].trim()); | |
| if (parts.length > 3 && StringUtils.hasText(parts[3])) unitMap.put("usageCategoryDetail", parts[3].trim()); | |
| } |
🤖 Prompt for AI Agents
In
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/TranslationService.java
around lines 138 to 152, the current logic splits usageCategory and writes
segments unconditionally which can produce empty values for trailing dots or
blanks; update it to trim the usageCategory before splitting, iterate over the
split segments and only put non-blank (after trim) segments into unitMap with
the correct keys (usageCategoryMajor, usageCategoryMinor, usageCategorySubMinor,
usageCategoryDetail), skipping any empty segments so no blank values are added
to the payload.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/EstimationService.java(2 hunks)
🔇 Additional comments (1)
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/EstimationService.java (1)
188-195: Vacant-land path: fix misspelled error code, guard null landArea, record billingSlabIds, and make selection deterministic.
- The error code constant is misspelled (VACANCT). Prior review already flagged this. Rename to VACANT and update the message with identifiers.
- Possible NPE when landArea is null.
- Currently not adding the chosen slab to billingSlabIds for the vacant-land path.
- Picking index 0 without ordering can be nondeterministic; sort slabs to ensure stable selection.
Apply this diff:
- if (PT_TYPE_VACANT_LAND.equalsIgnoreCase(detail.getPropertyType())) { - if (filteredBillingSlabs.isEmpty()) - throw new CustomException(PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT, "No billing slab found for vacant land"); - - taxAmt = taxAmt.add(BigDecimal.valueOf(filteredBillingSlabs.get(0).getUnitRate() * detail.getLandArea())); - } else { + if (PT_TYPE_VACANT_LAND.equalsIgnoreCase(detail.getPropertyType())) { + if (filteredBillingSlabs.isEmpty()) { + throw new CustomException(PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT, + String.format("No billing slab found for vacant land [tenant=%s, propertyId=%s]", tenantId, property.getPropertyId())); + } + if (detail.getLandArea() == null) { + throw new CustomException(PT_ESTIMATE_LANDAREA_NULL, PT_ESTIMATE_LANDAREA_NULL_MSG); + } + filteredBillingSlabs.sort(Comparator.comparing(BillingSlab::getFromPlotSize)); + BillingSlab chosen = filteredBillingSlabs.get(0); + taxAmt = taxAmt.add(BigDecimal.valueOf(chosen.getUnitRate() * detail.getLandArea())); + billingSlabIds.add(chosen.getId() + "|0"); + } else {Also add the following constants (in CalculatorConstants.java):
public static final String PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT = "PT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT"; public static final String PT_ESTIMATE_LANDAREA_NULL = "PT_ESTIMATE_LANDAREA_NULL"; public static final String PT_ESTIMATE_LANDAREA_NULL_MSG = "Land area is required for vacant land tax estimation";Run to verify no stale usages and that constants exist:
#!/bin/bash set -euo pipefail rg -nP '\bPT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANCT\b' -C2 || true rg -nP '\bPT_ESTIMATE_BILLINGSLABS_UNMATCH_VACANT\b' -C2 rg -nP '\bPT_ESTIMATE_LANDAREA_NULL(_MSG)?\b' -C2 rg -nP '\bbillingSlabIds\b' -C3
| if(property.getUsageCategory() != null) { | ||
| String[] usageCategoryMasterData = property.getUsageCategory().split("\\."); | ||
| usageCategoryMajor = usageCategoryMasterData[0]; | ||
| if(usageCategoryMasterData.length > 1) | ||
| usageCategoryMinor = usageCategoryMasterData[1]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick
Harden null-guard to also skip blanks for usageCategory.
Use isNotBlank to avoid empty token arrays for values like "".
- if(property.getUsageCategory() != null) {
+ if (StringUtils.isNotBlank(property.getUsageCategory())) {
String[] usageCategoryMasterData = property.getUsageCategory().split("\\.");
usageCategoryMajor = usageCategoryMasterData[0];
if(usageCategoryMasterData.length > 1)
usageCategoryMinor = usageCategoryMasterData[1];
}📝 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.
| if(property.getUsageCategory() != null) { | |
| String[] usageCategoryMasterData = property.getUsageCategory().split("\\."); | |
| usageCategoryMajor = usageCategoryMasterData[0]; | |
| if(usageCategoryMasterData.length > 1) | |
| usageCategoryMinor = usageCategoryMasterData[1]; | |
| } | |
| if (StringUtils.isNotBlank(property.getUsageCategory())) { | |
| String[] usageCategoryMasterData = property.getUsageCategory().split("\\."); | |
| usageCategoryMajor = usageCategoryMasterData[0]; | |
| if(usageCategoryMasterData.length > 1) | |
| usageCategoryMinor = usageCategoryMasterData[1]; | |
| } |
| String[] propertyTypeCollection = property.getPropertyType().split("\\."); | ||
| propertyType = propertyTypeCollection[0]; | ||
| if(propertyTypeCollection.length > 1) | ||
| propertySubType = propertyTypeCollection[1]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick
Same improvement for propertyType.
- if(property.getPropertyType() != null) {
+ if (StringUtils.isNotBlank(property.getPropertyType())) {
String[] propertyTypeCollection = property.getPropertyType().split("\\.");
propertyType = propertyTypeCollection[0];
if(propertyTypeCollection.length > 1)
propertySubType = propertyTypeCollection[1];
}📝 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.
| String[] propertyTypeCollection = property.getPropertyType().split("\\."); | |
| propertyType = propertyTypeCollection[0]; | |
| if(propertyTypeCollection.length > 1) | |
| propertySubType = propertyTypeCollection[1]; | |
| } | |
| if (StringUtils.isNotBlank(property.getPropertyType())) { | |
| String[] propertyTypeCollection = property.getPropertyType().split("\\."); | |
| propertyType = propertyTypeCollection[0]; | |
| if (propertyTypeCollection.length > 1) | |
| propertySubType = propertyTypeCollection[1]; | |
| } |
🤖 Prompt for AI Agents
In
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/EstimationService.java
around lines 1160 to 1164, the split handling sets propertyType from
propertyTypeCollection[0] without ensuring the original value is preserved when
no split occurs; mirror the earlier improvement used for propertySubType by
guarding and falling back: ensure propertyType is set to the full
property.getPropertyType() when the split produces no parts or the first part is
empty (and trim values), and only assign propertyTypeCollection[1] to
propertySubType when length > 1; update the logic to validate null/empty input
and use a safe fallback to avoid losing the original type.
| String[] ownership = property.getOwnershipCategory().split("\\."); | ||
| ownershipCategory = ownership[0]; | ||
| if(ownership.length > 1) | ||
| subownershipCategory = ownership[1]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick
Same improvement for ownershipCategory.
- if(property.getOwnershipCategory() != null) {
+ if (StringUtils.isNotBlank(property.getOwnershipCategory())) {
String[] ownership = property.getOwnershipCategory().split("\\.");
ownershipCategory = ownership[0];
if(ownership.length > 1)
subownershipCategory = ownership[1];
}📝 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.
| String[] ownership = property.getOwnershipCategory().split("\\."); | |
| ownershipCategory = ownership[0]; | |
| if(ownership.length > 1) | |
| subownershipCategory = ownership[1]; | |
| } | |
| if (StringUtils.isNotBlank(property.getOwnershipCategory())) { | |
| String[] ownership = property.getOwnershipCategory().split("\\."); | |
| ownershipCategory = ownership[0]; | |
| if(ownership.length > 1) | |
| subownershipCategory = ownership[1]; | |
| } |
🤖 Prompt for AI Agents
In
property-tax/pt-calculator-v2/src/main/java/org/egov/pt/calculator/service/EstimationService.java
around lines 1173-1177, the code assigns ownershipCategory directly from the
split result which can cause NPEs or retain surrounding whitespace; update the
block to first null-check property.getOwnershipCategory(), then split and guard
indexes and trim values before assignment (e.g., if ownership.length > 0 set
ownershipCategory = ownership[0].trim(); if ownership.length > 1 set
subownershipCategory = ownership[1].trim()), ensuring safe handling of empty or
whitespace-only strings.
Refactor Dockerfile for Maven and runtime images
build the multi arc yml for backend service
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @.github/workflows/backendbuild.yaml:
- Around line 123-151: The matrix job "build-matrix" currently computes the
"db_folder_exists" output in its "check-db-folder" step (id: check-db-folder),
which is non-deterministic across matrix legs; move this directory existence
check into the single-run job "resolve-config" so it runs once and add an
outputs entry (e.g., outputs.db_folder_exists) alongside the existing outputs
like dockerfile_path and work_dir, populate that output from the new check in
resolve-config, and remove the "Check for DB folder" step and the
db_folder_exists outputs block from "build-matrix" so downstream jobs consume
needs.resolve-config.outputs.db_folder_exists deterministically.
- Around line 157-160: Update the docker/login-action version from v2 to v3 in
the workflow; locate the steps that use "uses: docker/login-action@v2" inside
the "build-matrix" and "create-manifest" jobs and change them to
"docker/login-action@v3" so both occurrences (the step under build-matrix and
the step under create-manifest) are updated to v3 to resolve the Node 16
deprecation issue.
- Around line 65-68: Remove the extra spaces after the colon in the outputs
mapping so each key uses a single space before its value; update the outputs
block (keys dockerfile_path, work_dir, tag) to use the form "key: ${{ ... }}"
instead of aligning with multiple spaces (e.g., change "work_dir: ${{
steps.pick_dockerfile.outputs.work_dir }}" to "work_dir: ${{
steps.pick_dockerfile.outputs.work_dir }}" and similarly for dockerfile_path and
tag).
- Around line 71-73: Replace occurrences of the checkout action pinned to
actions/checkout@v3 with actions/checkout@v4 in the workflow jobs that use it:
update the checkout step inside the resolve-config job and the checkout step
inside the build-matrix job so both reference actions/checkout@v4 (ensure the
`uses:` lines that currently say actions/checkout@v3 are changed to
actions/checkout@v4).
In `@build/maven/Dockerfile`:
- Line 19: The COPY line "COPY --from=build /app/target/*.jar /app/start.sh
/opt/egov/" is too broad and may grab -sources, -javadoc, or other jars; update
the Dockerfile to copy a specific artifact name instead (e.g., the Spring Boot
fat JAR) or a tighter glob, or change the Maven build to set a deterministic
finalName via spring-boot-maven-plugin and then reference that exact filename in
the COPY command so only the intended runnable jar is copied.
- Line 22: The Dockerfile currently runs "dos2unix /opt/egov/start.sh" in an
Alpine-based image which lacks dos2unix; fix by either installing it before use
(add an apk add --no-cache dos2unix step prior to the RUN that references
dos2unix) or replace the dos2unix invocation with an equivalent sed conversion
(e.g., a sed command to remove CRLFs) so that the RUN line referencing
/opt/egov/start.sh succeeds; update the RUN that contains "dos2unix
/opt/egov/start.sh && chmod +x /opt/egov/start.sh" accordingly.
- Around line 24-25: Remove the build-time architecture check by deleting the
RUN uname -m instruction from the Dockerfile (the line that executes "RUN uname
-m"); this command only prints the build agent's architecture during image
build, creates an extra layer, and serves no runtime verification purpose, so
remove that RUN line to simplify the image and reduce layers.
- Around line 14-27: The runtime Dockerfile currently leaves the container
running as root; add a non-root user and switch to it in the runtime stage by
creating a group/user (e.g., with addgroup/adduser or adduser -D on alpine),
chowning /opt/egov and the copied artifacts (so /opt/egov/start.sh and the
jar(s) are owned by that user), and then add a USER <username> directive before
CMD ["/opt/egov/start.sh"]; ensure the user has execute permission on start.sh
(chmod +x already present) and that any required runtime ports/dirs under
/opt/egov are writable by the new user.
| outputs: | ||
| dockerfile_path: ${{ steps.pick_dockerfile.outputs.dockerfile_path }} | ||
| work_dir: ${{ steps.pick_dockerfile.outputs.work_dir }} | ||
| tag: ${{ steps.tag.outputs.tag }} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fix YAML formatting: extra spaces after colons.
YAMLlint flags Lines 67–68 for excessive whitespace. This is a minor lint issue but worth cleaning up for consistency.
Proposed fix
- dockerfile_path: ${{ steps.pick_dockerfile.outputs.dockerfile_path }}
- work_dir: ${{ steps.pick_dockerfile.outputs.work_dir }}
- tag: ${{ steps.tag.outputs.tag }}
+ dockerfile_path: ${{ steps.pick_dockerfile.outputs.dockerfile_path }}
+ work_dir: ${{ steps.pick_dockerfile.outputs.work_dir }}
+ tag: ${{ steps.tag.outputs.tag }}📝 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.
| outputs: | |
| dockerfile_path: ${{ steps.pick_dockerfile.outputs.dockerfile_path }} | |
| work_dir: ${{ steps.pick_dockerfile.outputs.work_dir }} | |
| tag: ${{ steps.tag.outputs.tag }} | |
| outputs: | |
| dockerfile_path: ${{ steps.pick_dockerfile.outputs.dockerfile_path }} | |
| work_dir: ${{ steps.pick_dockerfile.outputs.work_dir }} | |
| tag: ${{ steps.tag.outputs.tag }} |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 67-67: too many spaces after colon
(colons)
[error] 68-68: too many spaces after colon
(colons)
🤖 Prompt for AI Agents
In @.github/workflows/backendbuild.yaml around lines 65 - 68, Remove the extra
spaces after the colon in the outputs mapping so each key uses a single space
before its value; update the outputs block (keys dockerfile_path, work_dir, tag)
to use the form "key: ${{ ... }}" instead of aligning with multiple spaces
(e.g., change "work_dir: ${{ steps.pick_dockerfile.outputs.work_dir }}"
to "work_dir: ${{ steps.pick_dockerfile.outputs.work_dir }}" and similarly for
dockerfile_path and tag).
| uses: actions/checkout@v3 | ||
| with: | ||
| ref: ${{ github.event.inputs.branch }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
actions/checkout v3 Node 16 EOL GitHub Actions 2025
💡 Result:
actions/checkout@v3is a JavaScript action built for the Node.js 16 runtime. GitHub Actions ended support for “Node16 actions” by removing Node 16 from the hosted runner image on November 12, 2024, meaning workflows that still rely on Node16-based actions (likecheckout@v3) can break and must be updated. [1]- GitHub began the enforcement/migration earlier: they announced the Node16→Node20 transition in September 2023 (warnings starting Oct 23, 2023) [2], and then announced that runners would enforce Node20 instead of Node16 starting June 3, 2024 (with temporary opt-out env vars). [3]
- The practical fix is to upgrade to
actions/checkout@v4(Node20-based) if you haven’t already. [2][3] - Looking ahead into 2025+: GitHub has also already started the Node20 deprecation process (plan to move actions to Node24; Node24 default begins March 4, 2026). So staying current on major versions (v4 → future v5, etc.) is the safe path. [4]
Sources: [1] [2] [3] [4]
🌐 Web query:
actions/checkout v4 release date Node version
💡 Result:
- actions/checkout v4.0.0 release date: October 17, 2023 (tag v4.0.0 was published “17 Oct” on the official GitHub release page). [1][2]
- Node version used by actions/checkout v4: Node.js 20 (the v4.0.0 release notes say “Update default runtime to node20”). [1][3]
Sources:
[1] (github.com)
[2] (releasealert.dev)
[3] (newreleases.io)
Citations:
- 1: https://github.com/actions/checkout/releases/tag/v4.0.0
- 2: https://releasealert.dev/github/actions/checkout?utm_source=openai
- 3: https://newreleases.io/project/github/actions/checkout/release/v4.0.0
🌐 Web query:
actionlint actions/checkout v3 deprecated warning
💡 Result:
actionlint is warning because actions/checkout@v3 is a JavaScript action built for the Node.js 16 runtime, and GitHub Actions has deprecated Node 16 and moved workflows toward newer Node runtimes (Node 20 became the default in mid‑2024). This is why checkout@v3 is flagged as “deprecated/outdated” in workflows. [1][2][3]
Fix: upgrade to actions/checkout@v4 (or pin to the v4 commit SHA).
steps:
- uses: actions/checkout@v4If you still see the warning after updating checkout, check other @v3 actions in the workflow (e.g., upload-artifact@v3, cache@v3, etc.) and bump them to their Node 20+ major versions too. [1]
Sources: [1] GitHub Changelog (Node16→Node20 transition), [2] actions/checkout@v3 uses Node16 discussion, [3] updated Node20 default date.
Update actions/checkout from v3 to v4 — Node 16 support was removed from runners.
actions/checkout@v3 uses Node.js 16, which GitHub removed from hosted runners on November 12, 2024. Workflows relying on v3 are already broken. Update to actions/checkout@v4 (Node 20) in both the resolve-config (Line 71) and build-matrix (Line 140) jobs.
Proposed fix
- uses: actions/checkout@v3
+ uses: actions/checkout@v4Apply in both locations (Lines 71 and 140).
📝 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.
| uses: actions/checkout@v3 | |
| with: | |
| ref: ${{ github.event.inputs.branch }} | |
| uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ github.event.inputs.branch }} |
🧰 Tools
🪛 actionlint (1.7.10)
[error] 71-71: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/backendbuild.yaml around lines 71 - 73, Replace
occurrences of the checkout action pinned to actions/checkout@v3 with
actions/checkout@v4 in the workflow jobs that use it: update the checkout step
inside the resolve-config job and the checkout step inside the build-matrix job
so both reference actions/checkout@v4 (ensure the `uses:` lines that currently
say actions/checkout@v3 are changed to actions/checkout@v4).
| build-matrix: | ||
| needs: resolve-config | ||
| # expose whether a DB folder was present | ||
| outputs: | ||
| db_folder_exists: ${{ steps.check-db-folder.outputs.folder_exists }} | ||
| strategy: | ||
| matrix: | ||
| include: | ||
| - arch: amd64 | ||
| platform: linux/amd64 | ||
| runner: ubuntu-latest | ||
| - arch: arm64 | ||
| platform: linux/arm64 | ||
| runner: ubuntu-24.04-arm | ||
| runs-on: ${{ matrix.runner }} | ||
| steps: | ||
| - name: Checkout Code | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| ref: ${{ github.event.inputs.branch }} | ||
|
|
||
| - name: Check for DB folder | ||
| id: check-db-folder | ||
| run: | | ||
| if [ -d "${{ needs.resolve-config.outputs.work_dir }}/src/main/resources/db" ]; then | ||
| echo "folder_exists=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "folder_exists=false" >> "$GITHUB_OUTPUT" | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Matrix job output db_folder_exists is non-deterministic.
The build-matrix job runs as a 2×1 matrix (amd64 + arm64). When both matrix legs set the db_folder_exists output, GitHub Actions picks the value from the last leg to complete, which is non-deterministic. In practice this works because both legs check the same source tree, but it's fragile — if matrix composition changes, this could silently break.
Consider moving the DB-folder check into resolve-config (which runs once) and passing it as an output alongside dockerfile_path and work_dir.
🧰 Tools
🪛 actionlint (1.7.10)
[error] 140-140: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/backendbuild.yaml around lines 123 - 151, The matrix job
"build-matrix" currently computes the "db_folder_exists" output in its
"check-db-folder" step (id: check-db-folder), which is non-deterministic across
matrix legs; move this directory existence check into the single-run job
"resolve-config" so it runs once and add an outputs entry (e.g.,
outputs.db_folder_exists) alongside the existing outputs like dockerfile_path
and work_dir, populate that output from the new check in resolve-config, and
remove the "Check for DB folder" step and the db_folder_exists outputs block
from "build-matrix" so downstream jobs consume
needs.resolve-config.outputs.db_folder_exists deterministically.
| uses: docker/login-action@v2 | ||
| with: | ||
| username: ${{ env.DOCKER_USERNAME }} | ||
| password: ${{ env.DOCKER_PASSWORD }} |
There was a problem hiding this comment.
Update docker/login-action from v2 to v3.
Same Node 16 deprecation issue. docker/login-action@v2 should be updated to v3 in both the build-matrix (Line 157) and create-manifest (Line 194) jobs.
Proposed fix
- uses: docker/login-action@v2
+ uses: docker/login-action@v3Apply in both locations (Lines 157 and 194).
Also applies to: 194-197
🧰 Tools
🪛 actionlint (1.7.10)
[error] 157-157: the runner of "docker/login-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/backendbuild.yaml around lines 157 - 160, Update the
docker/login-action version from v2 to v3 in the workflow; locate the steps that
use "uses: docker/login-action@v2" inside the "build-matrix" and
"create-manifest" jobs and change them to "docker/login-action@v3" so both
occurrences (the step under build-matrix and the step under create-manifest) are
updated to v3 to resolve the Node 16 deprecation issue.
| FROM amazoncorretto:17-alpine | ||
|
|
||
| WORKDIR /opt/egov | ||
|
|
||
| # Copy artifacts from the build stage | ||
| COPY --from=build /app/target/*.jar /app/start.sh /opt/egov/ | ||
|
|
||
| RUN chmod +x /opt/egov/start.sh | ||
| # Ensure the start script has correct line endings and is executable | ||
| RUN dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh | ||
|
|
||
| # Verify architecture inside the container | ||
| RUN uname -m | ||
|
|
||
| CMD ["/opt/egov/start.sh"] |
There was a problem hiding this comment.
Container runs as root — add a non-root USER.
The runtime stage has no USER directive, so the container runs as root. This is flagged by both Trivy (DS002) and Checkov (CKV_DOCKER_3). Create a non-root user for the runtime stage.
🛡️ Proposed fix
FROM amazoncorretto:17-alpine
WORKDIR /opt/egov
# Copy artifacts from the build stage
COPY --from=build /app/target/*.jar /app/start.sh /opt/egov/
-# Ensure the start script has correct line endings and is executable
-RUN dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh
+# Ensure the start script has correct line endings and is executable, and create non-root user
+RUN sed -i 's/\r$//' /opt/egov/start.sh && chmod +x /opt/egov/start.sh \
+ && addgroup -S egov && adduser -S egov -G egov
-# Verify architecture inside the container
-RUN uname -m
+USER egov
CMD ["/opt/egov/start.sh"]🧰 Tools
🪛 Checkov (3.2.334)
[low] 1-27: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-27: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 Trivy (0.58.1)
[warning] 18-18: ':latest' tag used
Specify a tag in the 'FROM' statement for image 'egovio/8-openjdk-alpine'
Rule: DS001
(IaC/Dockerfile)
🤖 Prompt for AI Agents
In `@build/maven/Dockerfile` around lines 14 - 27, The runtime Dockerfile
currently leaves the container running as root; add a non-root user and switch
to it in the runtime stage by creating a group/user (e.g., with addgroup/adduser
or adduser -D on alpine), chowning /opt/egov and the copied artifacts (so
/opt/egov/start.sh and the jar(s) are owned by that user), and then add a USER
<username> directive before CMD ["/opt/egov/start.sh"]; ensure the user has
execute permission on start.sh (chmod +x already present) and that any required
runtime ports/dirs under /opt/egov are writable by the new user.
| WORKDIR /opt/egov | ||
|
|
||
| # Copy artifacts from the build stage | ||
| COPY --from=build /app/target/*.jar /app/start.sh /opt/egov/ |
There was a problem hiding this comment.
Wildcard JAR copy may pull in unintended artifacts.
/app/target/*.jar will copy every JAR in the target directory (including -sources.jar, -javadoc.jar, or dependency JARs copied by certain plugins). Consider using the Maven spring-boot-maven-plugin final name or a more specific glob to ensure only the intended fat JAR is copied.
🤖 Prompt for AI Agents
In `@build/maven/Dockerfile` at line 19, The COPY line "COPY --from=build
/app/target/*.jar /app/start.sh /opt/egov/" is too broad and may grab -sources,
-javadoc, or other jars; update the Dockerfile to copy a specific artifact name
instead (e.g., the Spring Boot fat JAR) or a tighter glob, or change the Maven
build to set a deterministic finalName via spring-boot-maven-plugin and then
reference that exact filename in the COPY command so only the intended runnable
jar is copied.
|
|
||
| RUN chmod +x /opt/egov/start.sh | ||
| # Ensure the start script has correct line endings and is executable | ||
| RUN dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh |
There was a problem hiding this comment.
dos2unix is not available in Alpine-based images by default — build will fail.
amazoncorretto:17-alpine is Alpine-based and does not ship dos2unix. This RUN step will fail with a "not found" error. Either install it first (apk add --no-cache dos2unix) or use sed as a lightweight alternative.
🐛 Proposed fix
Option A — install dos2unix:
-RUN dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh
+RUN apk add --no-cache dos2unix \
+ && dos2unix /opt/egov/start.sh \
+ && chmod +x /opt/egov/start.shOption B — use sed (no extra package):
-RUN dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh
+RUN sed -i 's/\r$//' /opt/egov/start.sh && chmod +x /opt/egov/start.sh📝 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.
| RUN dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh | |
| RUN apk add --no-cache dos2unix \ | |
| && dos2unix /opt/egov/start.sh \ | |
| && chmod +x /opt/egov/start.sh |
| RUN dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh | |
| RUN sed -i 's/\r$//' /opt/egov/start.sh && chmod +x /opt/egov/start.sh |
🤖 Prompt for AI Agents
In `@build/maven/Dockerfile` at line 22, The Dockerfile currently runs "dos2unix
/opt/egov/start.sh" in an Alpine-based image which lacks dos2unix; fix by either
installing it before use (add an apk add --no-cache dos2unix step prior to the
RUN that references dos2unix) or replace the dos2unix invocation with an
equivalent sed conversion (e.g., a sed command to remove CRLFs) so that the RUN
line referencing /opt/egov/start.sh succeeds; update the RUN that contains
"dos2unix /opt/egov/start.sh && chmod +x /opt/egov/start.sh" accordingly.
| # Verify architecture inside the container | ||
| RUN uname -m |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Build-time uname -m serves no runtime purpose — remove it.
This only prints the architecture of the build agent during image build. It doesn't verify the running container's architecture and adds an unnecessary layer.
Proposed fix
-# Verify architecture inside the container
-RUN uname -m
-
CMD ["/opt/egov/start.sh"]📝 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.
| # Verify architecture inside the container | |
| RUN uname -m | |
| CMD ["/opt/egov/start.sh"] |
🤖 Prompt for AI Agents
In `@build/maven/Dockerfile` around lines 24 - 25, Remove the build-time
architecture check by deleting the RUN uname -m instruction from the Dockerfile
(the line that executes "RUN uname -m"); this command only prints the build
agent's architecture during image build, creates an extra layer, and serves no
runtime verification purpose, so remove that RUN line to simplify the image and
reduce layers.
2 participants
Summary by CodeRabbit
Bug Fixes
Chores