Skip to content

Commit 772281b

Browse files
jcputneyclaude
jcputney
and
claude
authored
refactor: Extract CMIValueAccessService and RteDataTransferService from god classes (#1323)
* refactor: Extract CMIValueAccessService and RteDataTransferService from god classes - Extract CMIValueAccessService from BaseAPI.ts to handle CMI path traversal logic - Extract RteDataTransferService from TerminationHandler.ts for RTE data transfer - Remove internal wrapper methods from Scorm2004API.ts (methods already delegating to validators) - Update tests to use extracted services directly via internal access - Both services use context pattern for dependency injection This reduces BaseAPI.ts and TerminationHandler.ts complexity by extracting focused services with single responsibilities. * fix: Address code review issues from extracted services Code review fixes for CMIValueAccessService and RteDataTransferService: Critical fixes: - Add safe fallback in getErrorCode helper with warning for unknown keys - Return empty string instead of undefined in getCMIValue error paths (SCORM spec) Type safety improvements: - Replace `any` with `string` in context interface callbacks - Add proper return type `string` to getCMIValue method - Add validation functions for CompletionStatus and SuccessStatus - Use existing SuccessStatus type instead of inline union - Add RteTransferEventData interface for typed event data - Fix null handling type: getCMIData can return null Code quality: - Extract TARGET_ATTRIBUTE_PREFIX constant for magic string - Add getUndefinedDataModelErrorCode helper to consolidate error code lookup - Remove dead code in setFinalAttribute (unused object spread) - Add comprehensive JSDoc documentation for context interfaces - Make errorCodes readonly in context interface Test updates: - Add CHILDREN_ERROR and COUNT_ERROR to BaseAPI test mock --------- Co-authored-by: Claude <[email protected]>
1 parent 6cd631f commit 772281b

File tree

9 files changed

+1176
-755
lines changed

9 files changed

+1176
-755
lines changed

‎src/BaseAPI.ts‎

Lines changed: 33 additions & 282 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import {
2929
} from "./interfaces";
3030
import {
3131
AsynchronousHttpService,
32+
CMIValueAccessService,
33+
CMIValueAccessContext,
3234
createErrorHandlingService,
3335
EventService,
3436
getLoggingService,
@@ -51,6 +53,7 @@ export default abstract class BaseAPI implements IBaseAPI {
5153
private readonly _errorHandlingService: IErrorHandlingService;
5254
private readonly _loggingService: ILoggingService;
5355
private readonly _offlineStorageService?: IOfflineStorageService;
56+
private readonly _cmiValueAccessService: CMIValueAccessService;
5457
private _courseId: string = "";
5558

5659
/**
@@ -237,6 +240,30 @@ export default abstract class BaseAPI implements IBaseAPI {
237240
});
238241
}
239242
}
243+
244+
// Initialize CMI Value Access service
245+
const cmiValueAccessContext: CMIValueAccessContext = {
246+
errorCodes: this._error_codes,
247+
getLastErrorCode: () => this.lastErrorCode,
248+
setLastErrorCode: (errorCode: string) => {
249+
this.lastErrorCode = errorCode;
250+
},
251+
throwSCORMError: (element: string, errorCode: number, message?: string) =>
252+
this.throwSCORMError(element, errorCode, message),
253+
isInitialized: () => this.isInitialized(),
254+
validateCorrectResponse: (CMIElement: string, value: string) =>
255+
this.validateCorrectResponse(CMIElement, value),
256+
checkForDuplicateId: (CMIElement: string, value: string) =>
257+
this._checkForDuplicateId(CMIElement, value),
258+
getChildElement: (CMIElement: string, value: string, foundFirstIndex: boolean) =>
259+
this.getChildElement(CMIElement, value, foundFirstIndex),
260+
apiLog: (methodName: string, message: string, level: LogLevelEnum) =>
261+
this.apiLog(methodName, message, level),
262+
checkObjectHasProperty: (obj: StringKeyMap, attr: string) =>
263+
this._checkObjectHasProperty(obj, attr),
264+
getDataModel: () => this as unknown as StringKeyMap,
265+
};
266+
this._cmiValueAccessService = new CMIValueAccessService(cmiValueAccessContext);
240267
}
241268

242269
public abstract cmi: BaseCMI;
@@ -1164,7 +1191,8 @@ export default abstract class BaseAPI implements IBaseAPI {
11641191
}
11651192

11661193
/**
1167-
* Shared API method to set a valid for a given element.
1194+
* Shared API method to set a value for a given element.
1195+
* Delegates to CMIValueAccessService for the complex traversal logic.
11681196
*
11691197
* @param {string} methodName
11701198
* @param {boolean} scorm2004
@@ -1178,297 +1206,20 @@ export default abstract class BaseAPI implements IBaseAPI {
11781206
CMIElement: string,
11791207
value: any,
11801208
): string {
1181-
if (!CMIElement || CMIElement === "") {
1182-
if (scorm2004) {
1183-
this.throwSCORMError(
1184-
CMIElement,
1185-
this._error_codes.GENERAL_SET_FAILURE,
1186-
"The data model element was not specified",
1187-
);
1188-
}
1189-
return global_constants.SCORM_FALSE;
1190-
}
1191-
1192-
this.lastErrorCode = "0";
1193-
1194-
const structure = CMIElement.split(".");
1195-
let refObject: StringKeyMap | BaseCMI = this as StringKeyMap;
1196-
let returnValue = global_constants.SCORM_FALSE;
1197-
let foundFirstIndex = false;
1198-
1199-
const invalidErrorMessage = `The data model element passed to ${methodName} (${CMIElement}) is not a valid SCORM data model element.`;
1200-
const invalidErrorCode = scorm2004
1201-
? this._error_codes.UNDEFINED_DATA_MODEL
1202-
: this._error_codes.GENERAL;
1203-
1204-
for (let idx = 0; idx < structure.length; idx++) {
1205-
const attribute = structure[idx];
1206-
1207-
if (idx === structure.length - 1) {
1208-
if (scorm2004 && attribute && attribute.substring(0, 8) === "{target=") {
1209-
if (this.isInitialized()) {
1210-
this.throwSCORMError(CMIElement, this._error_codes.READ_ONLY_ELEMENT);
1211-
break;
1212-
} else {
1213-
refObject = {
1214-
...refObject,
1215-
attribute: value,
1216-
};
1217-
}
1218-
} else if (
1219-
typeof attribute === "undefined" ||
1220-
!this._checkObjectHasProperty(refObject as StringKeyMap, attribute)
1221-
) {
1222-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1223-
break;
1224-
} else {
1225-
if (
1226-
stringMatches(CMIElement, "\\.correct_responses\\.\\d+$") &&
1227-
this.isInitialized() &&
1228-
attribute !== "pattern"
1229-
) {
1230-
this.validateCorrectResponse(CMIElement, value);
1231-
if (this.lastErrorCode !== "0") {
1232-
this.throwSCORMError(CMIElement, this._error_codes.TYPE_MISMATCH);
1233-
break;
1234-
}
1235-
}
1236-
1237-
if (!scorm2004 || this._errorHandlingService.lastErrorCode === "0") {
1238-
if (
1239-
typeof attribute === "undefined" ||
1240-
attribute === "__proto__" ||
1241-
attribute === "constructor"
1242-
) {
1243-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1244-
break;
1245-
}
1246-
1247-
// SCORM 2004: Check for duplicate IDs in objectives and interactions arrays
1248-
// Per SCORM 2004 RTE Section 4.1.5/4.1.6: IDs must be unique within their respective arrays
1249-
if (scorm2004 && attribute === "id" && this.isInitialized()) {
1250-
const duplicateError = this._checkForDuplicateId(CMIElement, value);
1251-
if (duplicateError) {
1252-
this.throwSCORMError(CMIElement, this._error_codes.GENERAL_SET_FAILURE);
1253-
break;
1254-
}
1255-
}
1256-
1257-
(refObject as StringKeyMap)[attribute] = value;
1258-
returnValue = global_constants.SCORM_TRUE;
1259-
}
1260-
}
1261-
} else {
1262-
if (
1263-
typeof attribute === "undefined" ||
1264-
!this._checkObjectHasProperty(refObject as StringKeyMap, attribute)
1265-
) {
1266-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1267-
break;
1268-
}
1269-
refObject = (refObject as StringKeyMap)[attribute] as StringKeyMap;
1270-
if (!refObject) {
1271-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1272-
break;
1273-
}
1274-
1275-
if (refObject instanceof CMIArray) {
1276-
const index = parseInt(structure[idx + 1] || "0", 10);
1277-
1278-
// SCO is trying to set an item on an array
1279-
if (!isNaN(index)) {
1280-
const item = refObject.childArray[index];
1281-
1282-
if (item) {
1283-
refObject = item;
1284-
foundFirstIndex = true;
1285-
} else {
1286-
// SCORM spec requires sequential array indices (0, 1, 2, ...)
1287-
// Cannot skip indices when adding to arrays
1288-
if (index > refObject.childArray.length) {
1289-
const errorCode = scorm2004
1290-
? this._error_codes.GENERAL_SET_FAILURE
1291-
: this._error_codes.INVALID_SET_VALUE || this._error_codes.GENERAL_SET_FAILURE;
1292-
this.throwSCORMError(
1293-
CMIElement,
1294-
errorCode,
1295-
`Cannot set array element at index ${index}. Array indices must be sequential. Current array length is ${refObject.childArray.length}, expected index ${refObject.childArray.length}.`,
1296-
);
1297-
break;
1298-
}
1299-
1300-
// Note: SCORM 2004 3rd Edition specifies SPM limits for arrays
1301-
// (objectives: 100, interactions: 250, comments: 250)
1302-
// We intentionally do NOT enforce these limits to maximize
1303-
// content compatibility. Real-world content may exceed these limits.
1304-
1305-
const newChild = this.getChildElement(CMIElement, value, foundFirstIndex);
1306-
foundFirstIndex = true;
1307-
1308-
if (!newChild) {
1309-
if (this.lastErrorCode === "0") {
1310-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1311-
}
1312-
break;
1313-
} else {
1314-
if (refObject.initialized) newChild.initialize();
1315-
refObject.childArray[index] = newChild;
1316-
refObject = newChild;
1317-
}
1318-
}
1319-
1320-
// Have to update idx value to skip the array position
1321-
idx++;
1322-
}
1323-
}
1324-
}
1325-
}
1326-
1327-
if (returnValue === global_constants.SCORM_FALSE) {
1328-
this.apiLog(
1329-
methodName,
1330-
`There was an error setting the value for: ${CMIElement}, value of: ${value}`,
1331-
LogLevelEnum.WARN,
1332-
);
1333-
}
1334-
1335-
return returnValue;
1209+
return this._cmiValueAccessService.setCMIValue(methodName, scorm2004, CMIElement, value);
13361210
}
13371211

13381212
/**
1339-
* Gets a value from the CMI Object
1213+
* Gets a value from the CMI Object.
1214+
* Delegates to CMIValueAccessService for the complex traversal logic.
13401215
*
13411216
* @param {string} methodName
13421217
* @param {boolean} scorm2004
13431218
* @param {string} CMIElement
13441219
* @return {any}
13451220
*/
13461221
_commonGetCMIValue(methodName: string, scorm2004: boolean, CMIElement: string): any {
1347-
if (!CMIElement || CMIElement === "") {
1348-
if (scorm2004) {
1349-
this.throwSCORMError(
1350-
CMIElement,
1351-
this._error_codes.GENERAL_GET_FAILURE,
1352-
"The data model element was not specified",
1353-
);
1354-
}
1355-
return "";
1356-
}
1357-
1358-
// SCORM 2004: Validate ._version keyword usage - only valid on cmi._version
1359-
if (scorm2004 && CMIElement.endsWith("._version") && CMIElement !== "cmi._version") {
1360-
this.throwSCORMError(
1361-
CMIElement,
1362-
this._error_codes.GENERAL_GET_FAILURE,
1363-
"The _version keyword was used incorrectly",
1364-
);
1365-
return "";
1366-
}
1367-
1368-
const structure = CMIElement.split(".");
1369-
let refObject: StringKeyMap = this as StringKeyMap;
1370-
let attribute = null;
1371-
1372-
const uninitializedErrorMessage = `The data model element passed to ${methodName} (${CMIElement}) has not been initialized.`;
1373-
const invalidErrorMessage = `The data model element passed to ${methodName} (${CMIElement}) is not a valid SCORM data model element.`;
1374-
const invalidErrorCode = scorm2004
1375-
? this._error_codes.UNDEFINED_DATA_MODEL
1376-
: this._error_codes.GENERAL;
1377-
1378-
for (let idx = 0; idx < structure.length; idx++) {
1379-
attribute = structure[idx];
1380-
1381-
if (!scorm2004) {
1382-
if (idx === structure.length - 1) {
1383-
if (
1384-
typeof attribute === "undefined" ||
1385-
!this._checkObjectHasProperty(refObject, attribute)
1386-
) {
1387-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1388-
return;
1389-
}
1390-
}
1391-
} else {
1392-
if (
1393-
String(attribute).substring(0, 8) === "{target=" &&
1394-
typeof refObject._isTargetValid == "function"
1395-
) {
1396-
// Extract target from {target=X} format: skip "{target=" (8 chars) and "}" (1 char at end)
1397-
const target = String(attribute).substring(8, String(attribute).length - 1);
1398-
return refObject._isTargetValid(target);
1399-
} else if (
1400-
typeof attribute === "undefined" ||
1401-
!this._checkObjectHasProperty(refObject, attribute)
1402-
) {
1403-
// SCORM 2004: Check for keyword errors with specific diagnostics
1404-
if (attribute === "_children") {
1405-
this.throwSCORMError(
1406-
CMIElement,
1407-
this._error_codes.GENERAL_GET_FAILURE,
1408-
"The data model element does not have children",
1409-
);
1410-
return;
1411-
} else if (attribute === "_count") {
1412-
this.throwSCORMError(
1413-
CMIElement,
1414-
this._error_codes.GENERAL_GET_FAILURE,
1415-
"The data model element is not a collection and therefore does not have a count",
1416-
);
1417-
return;
1418-
}
1419-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1420-
return;
1421-
}
1422-
}
1423-
1424-
if (attribute !== undefined && attribute !== null) {
1425-
refObject = refObject[attribute] as StringKeyMap;
1426-
if (refObject === undefined) {
1427-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1428-
break;
1429-
}
1430-
} else {
1431-
this.throwSCORMError(CMIElement, invalidErrorCode, invalidErrorMessage);
1432-
break;
1433-
}
1434-
1435-
if (refObject instanceof CMIArray) {
1436-
const index = parseInt(structure[idx + 1] || "", 10);
1437-
1438-
// SCO is trying to set an item on an array
1439-
if (!isNaN(index)) {
1440-
const item = refObject.childArray[index];
1441-
1442-
if (item) {
1443-
refObject = item as unknown as StringKeyMap;
1444-
} else {
1445-
this.throwSCORMError(
1446-
CMIElement,
1447-
this._error_codes.VALUE_NOT_INITIALIZED,
1448-
uninitializedErrorMessage,
1449-
);
1450-
return;
1451-
}
1452-
1453-
// Have to update idx value to skip the array position
1454-
idx++;
1455-
}
1456-
}
1457-
}
1458-
1459-
if (refObject === null || refObject === undefined) {
1460-
if (!scorm2004) {
1461-
// SCORM 1.2: Use specific keyword error codes
1462-
if (attribute === "_children") {
1463-
this.throwSCORMError(CMIElement, this._error_codes.CHILDREN_ERROR, undefined);
1464-
} else if (attribute === "_count") {
1465-
this.throwSCORMError(CMIElement, this._error_codes.COUNT_ERROR, undefined);
1466-
}
1467-
}
1468-
// SCORM 2004 keyword errors are handled during traversal (lines 1318-1333)
1469-
} else {
1470-
return refObject;
1471-
}
1222+
return this._cmiValueAccessService.getCMIValue(methodName, scorm2004, CMIElement);
14721223
}
14731224

14741225
/**

0 commit comments

Comments
 (0)