OpenWF/SpaceNinjaServer
12
6
Fork 14
Code Issues 42 Pull Requests 1 Actions Packages Projects Releases Activity

chore: move mod upgrading logic into artifactsController #800

Merged
Sainan merged 1 commits from upgrademod into main 2025-01-18 02:12:06 -08:00
Conversation 2 Commits 1 Files Changed 3 +57 -73
3 changed files with 57 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
src/controllers/api/artifactsController.ts
View File

@ -1,19 +1,66 @@
import { getJSONfromString } from "@/src/helpers/stringHelpers"; import { getJSONfromString } from "@/src/helpers/stringHelpers";
import { getAccountIdForRequest } from "@/src/services/loginService"; import { getAccountIdForRequest } from "@/src/services/loginService";
import { upgradeMod } from "@/src/services/inventoryService";
import { IArtifactsRequest } from "@/src/types/requestTypes";
import { RequestHandler } from "express"; import { RequestHandler } from "express";
import { ICrewShipSalvagedWeaponSkin } from "@/src/types/inventoryTypes/inventoryTypes";
import { getInventory } from "@/src/services/inventoryService";
import { config } from "@/src/services/configService";
const artifactsController: RequestHandler = async (req, res) => { export const artifactsController: RequestHandler = async (req, res) => {
const accountId = await getAccountIdForRequest(req); const accountId = await getAccountIdForRequest(req);
const artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest;
try { const { Upgrade, LevelDiff, Cost, FusionPointCost } = artifactsData;
const artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest;
coderabbitai[bot] commented 2025-01-17 19:02:53 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Validate and handle potential parsing errors of request body

Ensure that the request body is properly validated and handle any potential parsing errors to prevent runtime exceptions.

Consider adding error handling for the parsing operation:

 const accountId = await getAccountIdForRequest(req);
-const artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest;
+let artifactsData: IArtifactsRequest;
+try {
+    artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest;
+} catch (error) {
+    res.status(400).send("Invalid request data");
+    return;
+}
📝 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.

    let artifactsData: IArtifactsRequest;
    try {
        artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest;
    } catch (error) {
        res.status(400).send("Invalid request data");
        return;
    }
_:warning: Potential issue_ **Validate and handle potential parsing errors of request body** Ensure that the request body is properly validated and handle any potential parsing errors to prevent runtime exceptions. Consider adding error handling for the parsing operation: ```diff const accountId = await getAccountIdForRequest(req); -const artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest; +let artifactsData: IArtifactsRequest; +try { + artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest; +} catch (error) { + res.status(400).send("Invalid request data"); + return; +} ``` <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. `````suggestion let artifactsData: IArtifactsRequest; try { artifactsData = getJSONfromString(String(req.body)) as IArtifactsRequest; } catch (error) { res.status(400).send("Invalid request data"); return; } ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
const upgradeModId = await upgradeMod(artifactsData, accountId); const inventory = await getInventory(accountId);
res.send(upgradeModId); const { Upgrades, RawUpgrades } = inventory;
} catch (err) { const { ItemType, UpgradeFingerprint, ItemId } = Upgrade;
console.error("Error parsing JSON data:", err);
const safeUpgradeFingerprint = UpgradeFingerprint || '{"lvl":0}';
const parsedUpgradeFingerprint = JSON.parse(safeUpgradeFingerprint) as { lvl: number };
parsedUpgradeFingerprint.lvl += LevelDiff;
const stringifiedUpgradeFingerprint = JSON.stringify(parsedUpgradeFingerprint);
let itemIndex = Upgrades.findIndex(upgrade => upgrade._id?.equals(ItemId!.$oid));
if (itemIndex !== -1) {
Upgrades[itemIndex].UpgradeFingerprint = stringifiedUpgradeFingerprint;
inventory.markModified(`Upgrades.${itemIndex}.UpgradeFingerprint`);
} else {
coderabbitai[bot] commented 2025-01-17 19:02:53 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Add error handling for JSON parsing of 'UpgradeFingerprint'

Parsing UpgradeFingerprint without error handling may lead to exceptions if the input is invalid. Wrap the JSON.parse call in a try-catch block.

Apply this diff to handle potential JSON parsing errors:

 const safeUpgradeFingerprint = UpgradeFingerprint || '{"lvl":0}';
-const parsedUpgradeFingerprint = JSON.parse(safeUpgradeFingerprint) as { lvl: number };
+let parsedUpgradeFingerprint: { lvl: number };
+try {
+    parsedUpgradeFingerprint = JSON.parse(safeUpgradeFingerprint) as { lvl: number };
+} catch (error) {
+    res.status(400).send("Invalid UpgradeFingerprint data");
+    return;
+}

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

_:warning: Potential issue_ **Add error handling for JSON parsing of 'UpgradeFingerprint'** Parsing `UpgradeFingerprint` without error handling may lead to exceptions if the input is invalid. Wrap the `JSON.parse` call in a try-catch block. Apply this diff to handle potential JSON parsing errors: ```diff const safeUpgradeFingerprint = UpgradeFingerprint || '{"lvl":0}'; -const parsedUpgradeFingerprint = JSON.parse(safeUpgradeFingerprint) as { lvl: number }; +let parsedUpgradeFingerprint: { lvl: number }; +try { + parsedUpgradeFingerprint = JSON.parse(safeUpgradeFingerprint) as { lvl: number }; +} catch (error) { + res.status(400).send("Invalid UpgradeFingerprint data"); + return; +} ``` > Committable suggestion skipped: line range outside the PR's diff. <!-- This is an auto-generated comment by CodeRabbit -->
itemIndex =
Upgrades.push({
UpgradeFingerprint: stringifiedUpgradeFingerprint,
ItemType
}) - 1;
coderabbitai[bot] commented 2025-01-17 19:02:53 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Check if 'ItemId' is valid before using it

The ItemId may be undefined or null. Ensure that it is valid before using it in the findIndex function to prevent runtime errors.

Consider adding a check for ItemId:

 const { ItemType, UpgradeFingerprint, ItemId } = Upgrade;

+if (!ItemId || !ItemId.$oid) {
+    res.status(400).send("Invalid ItemId");
+    return;
+}

 let itemIndex = Upgrades.findIndex(upgrade => upgrade._id?.equals(ItemId!.$oid));

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

_:warning: Potential issue_ **Check if 'ItemId' is valid before using it** The `ItemId` may be undefined or null. Ensure that it is valid before using it in the `findIndex` function to prevent runtime errors. Consider adding a check for `ItemId`: ```diff const { ItemType, UpgradeFingerprint, ItemId } = Upgrade; +if (!ItemId || !ItemId.$oid) { + res.status(400).send("Invalid ItemId"); + return; +} let itemIndex = Upgrades.findIndex(upgrade => upgrade._id?.equals(ItemId!.$oid)); ``` > Committable suggestion skipped: line range outside the PR's diff. <!-- This is an auto-generated comment by CodeRabbit -->
const rawItemIndex = RawUpgrades.findIndex(rawUpgrade => rawUpgrade.ItemType === ItemType);
RawUpgrades[rawItemIndex].ItemCount--;
if (RawUpgrades[rawItemIndex].ItemCount > 0) {
inventory.markModified(`RawUpgrades.${rawItemIndex}.UpgradeFingerprint`);
} else {
RawUpgrades.splice(rawItemIndex, 1);
}
} }
coderabbitai[bot] commented 2025-01-17 19:02:53 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Assign 'ItemId' when adding a new upgrade to 'Upgrades'

When pushing a new upgrade to the Upgrades array, the ItemId is not assigned. This may lead to issues when the ItemId is accessed later.

Include ItemId when adding a new upgrade:

             Upgrades.push({
                 UpgradeFingerprint: stringifiedUpgradeFingerprint,
                 ItemType,
+                ItemId: generateNewObjectId(), // Assume a function to generate a new ObjectId
             }) - 1;

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

_:warning: Potential issue_ **Assign 'ItemId' when adding a new upgrade to 'Upgrades'** When pushing a new upgrade to the `Upgrades` array, the `ItemId` is not assigned. This may lead to issues when the `ItemId` is accessed later. Include `ItemId` when adding a new upgrade: ```diff Upgrades.push({ UpgradeFingerprint: stringifiedUpgradeFingerprint, ItemType, + ItemId: generateNewObjectId(), // Assume a function to generate a new ObjectId }) - 1; ``` > Committable suggestion skipped: line range outside the PR's diff. <!-- This is an auto-generated comment by CodeRabbit -->
if (!config.infiniteCredits) {
inventory.RegularCredits -= Cost;
}
coderabbitai[bot] commented 2025-01-17 19:02:53 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Check for valid 'rawItemIndex' before accessing 'RawUpgrades[rawItemIndex]'

If ItemType is not found in RawUpgrades, rawItemIndex will be -1. Accessing RawUpgrades[-1] will cause an error. Ensure that rawItemIndex is valid before proceeding.

Modify the code to handle the case when rawItemIndex is -1:

 const rawItemIndex = RawUpgrades.findIndex(rawUpgrade => rawUpgrade.ItemType === ItemType);

-if (rawItemIndex !== -1) {
-    RawUpgrades[rawItemIndex].ItemCount--;
-    if (RawUpgrades[rawItemIndex].ItemCount > 0) {
-        inventory.markModified(`RawUpgrades.${rawItemIndex}.UpgradeFingerprint`);
-    } else {
-        RawUpgrades.splice(rawItemIndex, 1);
-    }
+if (rawItemIndex === -1) {
+    res.status(400).send("ItemType not found in RawUpgrades");
+    return;
 }
+RawUpgrades[rawItemIndex].ItemCount--;
+if (RawUpgrades[rawItemIndex].ItemCount > 0) {
+    inventory.markModified(`RawUpgrades.${rawItemIndex}.ItemCount`);
+} else {
+    RawUpgrades.splice(rawItemIndex, 1);
+}

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

_:warning: Potential issue_ **Check for valid 'rawItemIndex' before accessing 'RawUpgrades[rawItemIndex]'** If `ItemType` is not found in `RawUpgrades`, `rawItemIndex` will be `-1`. Accessing `RawUpgrades[-1]` will cause an error. Ensure that `rawItemIndex` is valid before proceeding. Modify the code to handle the case when `rawItemIndex` is `-1`: ```diff const rawItemIndex = RawUpgrades.findIndex(rawUpgrade => rawUpgrade.ItemType === ItemType); -if (rawItemIndex !== -1) { - RawUpgrades[rawItemIndex].ItemCount--; - if (RawUpgrades[rawItemIndex].ItemCount > 0) { - inventory.markModified(`RawUpgrades.${rawItemIndex}.UpgradeFingerprint`); - } else { - RawUpgrades.splice(rawItemIndex, 1); - } +if (rawItemIndex === -1) { + res.status(400).send("ItemType not found in RawUpgrades"); + return; } +RawUpgrades[rawItemIndex].ItemCount--; +if (RawUpgrades[rawItemIndex].ItemCount > 0) { + inventory.markModified(`RawUpgrades.${rawItemIndex}.ItemCount`); +} else { + RawUpgrades.splice(rawItemIndex, 1); +} ``` > Committable suggestion skipped: line range outside the PR's diff. <!-- This is an auto-generated comment by CodeRabbit -->
if (!config.infiniteEndo) {
coderabbitai[bot] commented 2025-01-17 19:02:52 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Correct the property name in 'markModified' to 'ItemCount'

You modified ItemCount in RawUpgrades, but the code marks UpgradeFingerprint as modified. Update the property name to reflect the actual modified field.

Apply this diff to correct the property name:

-        inventory.markModified(`RawUpgrades.${rawItemIndex}.UpgradeFingerprint`);
+        inventory.markModified(`RawUpgrades.${rawItemIndex}.ItemCount`);
📝 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.

            inventory.markModified(`RawUpgrades.${rawItemIndex}.ItemCount`);
_:warning: Potential issue_ **Correct the property name in 'markModified' to 'ItemCount'** You modified `ItemCount` in `RawUpgrades`, but the code marks `UpgradeFingerprint` as modified. Update the property name to reflect the actual modified field. Apply this diff to correct the property name: ```diff - inventory.markModified(`RawUpgrades.${rawItemIndex}.UpgradeFingerprint`); + inventory.markModified(`RawUpgrades.${rawItemIndex}.ItemCount`); ``` <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. `````suggestion inventory.markModified(`RawUpgrades.${rawItemIndex}.ItemCount`); ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
inventory.FusionPoints -= FusionPointCost;
}
const changedInventory = await inventory.save();
const itemId = changedInventory.toJSON().Upgrades[itemIndex]?.ItemId?.$oid;
if (!itemId) {
throw new Error("Item Id not found in upgradeMod");
}
res.send(itemId);
coderabbitai[bot] commented 2025-01-17 19:02:53 -08:00 (Migrated from github.com)
Review
Copy Link

💡 Codebase verification

Insufficient balance checks confirmed - Implementation required

The verification confirms that no balance checks exist before deducting RegularCredits and FusionPoints. The suggested implementation is necessary to prevent negative balances and maintain game economy consistency.

🔗 Analysis chain

Ensure sufficient 'RegularCredits' and 'FusionPoints' before deduction

Before deducting Cost and FusionPointCost, verify that the inventory has sufficient RegularCredits and FusionPoints to prevent negative balances.

Add checks to confirm sufficient funds:

 if (!config.infiniteCredits) {
+    if (inventory.RegularCredits < Cost) {
+        res.status(400).send("Insufficient RegularCredits");
+        return;
+    }
     inventory.RegularCredits -= Cost;
 }

 if (!config.infiniteEndo) {
+    if (inventory.FusionPoints < FusionPointCost) {
+        res.status(400).send("Insufficient FusionPoints");
+        return;
+    }
     inventory.FusionPoints -= FusionPointCost;
 }
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found

Script:

#!/bin/bash
# First, let's see the complete artifactsController.ts file
cat src/controllers/api/artifactsController.ts

# Then, let's search for any balance check patterns
rg "RegularCredits.*<" --type ts
rg "FusionPoints.*<" --type ts

# Look for similar patterns in other files
ast-grep --pattern 'if ($inventory.$balance < $cost) { $$$; return; }'

Length of output: 2702

_:bulb: Codebase verification_ **Insufficient balance checks confirmed - Implementation required** The verification confirms that no balance checks exist before deducting `RegularCredits` and `FusionPoints`. The suggested implementation is necessary to prevent negative balances and maintain game economy consistency. <details> <summary>🔗 Analysis chain</summary> **Ensure sufficient 'RegularCredits' and 'FusionPoints' before deduction** Before deducting `Cost` and `FusionPointCost`, verify that the inventory has sufficient `RegularCredits` and `FusionPoints` to prevent negative balances. Add checks to confirm sufficient funds: ```diff if (!config.infiniteCredits) { + if (inventory.RegularCredits < Cost) { + res.status(400).send("Insufficient RegularCredits"); + return; + } inventory.RegularCredits -= Cost; } if (!config.infiniteEndo) { + if (inventory.FusionPoints < FusionPointCost) { + res.status(400).send("Insufficient FusionPoints"); + return; + } inventory.FusionPoints -= FusionPointCost; } ``` </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Error: No shell scripts found --- Script: ```shell #!/bin/bash # First, let's see the complete artifactsController.ts file cat src/controllers/api/artifactsController.ts # Then, let's search for any balance check patterns rg "RegularCredits.*<" --type ts rg "FusionPoints.*<" --type ts # Look for similar patterns in other files ast-grep --pattern 'if ($inventory.$balance < $cost) { $$$; return; }' ``` Length of output: 2702 </details> <!-- This is an auto-generated comment by CodeRabbit -->
}; };
export { artifactsController }; interface IArtifactsRequest {
Upgrade: ICrewShipSalvagedWeaponSkin;
LevelDiff: number;
Cost: number;
FusionPointCost: number;
}

55
src/services/inventoryService.ts
View File

@ -21,7 +21,6 @@ import {
} from "@/src/types/inventoryTypes/inventoryTypes"; } from "@/src/types/inventoryTypes/inventoryTypes";
import { IGenericUpdate } from "../types/genericUpdate"; import { IGenericUpdate } from "../types/genericUpdate";
import { import {
IArtifactsRequest,
IMissionInventoryUpdateRequest, IMissionInventoryUpdateRequest,
IThemeUpdateRequest, IThemeUpdateRequest,
IUpdateChallengeProgressRequest IUpdateChallengeProgressRequest
@ -899,57 +898,3 @@ export const addBooster = async (ItemType: string, time: number, accountId: stri
await inventory.save(); await inventory.save();
}; };
export const upgradeMod = async (artifactsData: IArtifactsRequest, accountId: string): Promise<string | undefined> => {
const { Upgrade, LevelDiff, Cost, FusionPointCost } = artifactsData;
try {
const inventory = await getInventory(accountId);
const { Upgrades, RawUpgrades } = inventory;
const { ItemType, UpgradeFingerprint, ItemId } = Upgrade;
const safeUpgradeFingerprint = UpgradeFingerprint || '{"lvl":0}';
const parsedUpgradeFingerprint = JSON.parse(safeUpgradeFingerprint) as { lvl: number };
parsedUpgradeFingerprint.lvl += LevelDiff;
const stringifiedUpgradeFingerprint = JSON.stringify(parsedUpgradeFingerprint);
let itemIndex = Upgrades.findIndex(upgrade => upgrade._id?.equals(ItemId!.$oid));
if (itemIndex !== -1) {
Upgrades[itemIndex].UpgradeFingerprint = stringifiedUpgradeFingerprint;
inventory.markModified(`Upgrades.${itemIndex}.UpgradeFingerprint`);
} else {
itemIndex =
Upgrades.push({
UpgradeFingerprint: stringifiedUpgradeFingerprint,
ItemType
}) - 1;
const rawItemIndex = RawUpgrades.findIndex(rawUpgrade => rawUpgrade.ItemType === ItemType);
RawUpgrades[rawItemIndex].ItemCount--;
if (RawUpgrades[rawItemIndex].ItemCount > 0) {
inventory.markModified(`RawUpgrades.${rawItemIndex}.UpgradeFingerprint`);
} else {
RawUpgrades.splice(rawItemIndex, 1);
}
}
if (!config.infiniteCredits) {
inventory.RegularCredits -= Cost;
}
if (!config.infiniteEndo) {
inventory.FusionPoints -= FusionPointCost;
}
const changedInventory = await inventory.save();
const itemId = changedInventory.toJSON().Upgrades[itemIndex]?.ItemId?.$oid;
if (!itemId) {
throw new Error("Item Id not found in upgradeMod");
}
return itemId;
} catch (error) {
console.error("Error in upgradeMod:", error);
throw error;
}
};

8
src/types/requestTypes.ts
View File

@ -4,7 +4,6 @@ import {
IBooster, IBooster,
IChallengeProgress, IChallengeProgress,
IConsumable, IConsumable,
ICrewShipSalvagedWeaponSkin,
IEvolutionProgress, IEvolutionProgress,
IMiscItem, IMiscItem,
ITypeCount, ITypeCount,
@ -16,13 +15,6 @@ import {
IFusionTreasure IFusionTreasure
} from "./inventoryTypes/inventoryTypes"; } from "./inventoryTypes/inventoryTypes";
export interface IArtifactsRequest {
Upgrade: ICrewShipSalvagedWeaponSkin;
LevelDiff: number;
Cost: number;
FusionPointCost: number;
}
export interface IThemeUpdateRequest { export interface IThemeUpdateRequest {
Style?: string; Style?: string;
Background?: string; Background?: string;