Skip to content

Feature/relationship insert at index sparse #113

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

Open
tejaede wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Feature/relationship insert at index sparse #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
27276a5
...
tejaede Feb 20, 2026
b147f24
Preferred Next Role fix w/ sparse array + logging
tejaede Feb 24, 2026
cc828c8
Remove logging
tejaede Feb 24, 2026
51d96b8
...
tejaede Feb 24, 2026
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
6 changes: 5 additions & 1 deletion data/converter/raw-foreign-value-to-object-converter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,10 +877,14 @@ exports.RawForeignValueToObjectConverter = RawValueToObjectConverter.specialize(
//forEach skipps over holes of a sparse array
v.forEach(function(value) {
/*
Make sure we have a valid data object anf not null nor undefined before trying to get their primary key
Make sure we have a valid data object anf not null nor undefined before trying to get their primary key
*/
if(value) {
result.push(service.dataIdentifierForObject(value).primaryKey);
} else {
//TODO: Is this safe? It was added because, in some cases, you may want to preserve a sparse array. E.g. The
//array has a fixed length, but not all values are defined
result.push(value);
Copy link
Contributor

@marchant marchant Feb 24, 2026

Choose a reason for hiding this comment

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

The place it could impact would be a toMany that doesn't uses an array column, we should check PG service for that use case of an update. It might already be just looping over and ignoring it, but not certain it does.

Another thing we could do, is add this behavior only if the propertyDescriptor involved has isOrdered == true. But making sure holes in arrays passed to the PG service are ignored when an array column is not involved brings peace of mind and resilience in the face of a mapping / modeling inconsistency

}
});
currentRule = null;
Expand Down
2 changes: 1 addition & 1 deletion data/service/data-operation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ var Montage = require("../../core/core").Montage,

exports.DataOperationType = DataOperationType = new Enum().initWithMembersAndValues(dataOperationTypes,dataOperationTypes);

var dataOperationErrorNames = ["DatabaseMissing", "ObjectDescriptorStoreMissing", "PropertyDescriptorStoreMissing", "InvalidInput", "SyntaxError", "PropertyDescriptorNotFound", "PropertyMappingNotFound", "TransactionDeadlock"];
var dataOperationErrorNames = ["DatabaseMissing", "ObjectDescriptorStoreMissing", "PropertyDescriptorStoreMissing", "InvalidInput", "SyntaxError", "PropertyDescriptorNotFound", "PropertyMappingNotFound", "TransactionDeadlock", "FunctionMissing"];
exports.DataOperationErrorNames = DataOperationErrorNames = new Enum().initWithMembersAndValues(dataOperationErrorNames,dataOperationErrorNames);

// exports.DataOperationError.ObjectDescriptorStoreMissingError = Error.specialize({
Expand Down
8 changes: 6 additions & 2 deletions data/service/data-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4661,6 +4661,7 @@ DataService.addClassProperties(

if (!manyChanges) {
manyChanges = {};
manyChanges.index = changeEvent.index;
changesForDataObject.set(key, manyChanges);
}

Expand Down Expand Up @@ -4692,7 +4693,8 @@ DataService.addClassProperties(
var registeredRemovedValues = manyChanges.removedValues;
if (!registeredRemovedValues) {
if (!isDataObjectBeingMapped) {
manyChanges.removedValues = registeredRemovedValues = new Set(removedValues);
manyChanges.removedValues = removedValues;
manyChanges.removedValuesSet = registeredRemovedValues = new Set(removedValues);
}
self._removeDataObjectPropertyDescriptorValuesForInversePropertyDescriptor(
dataObject,
Expand Down Expand Up @@ -4752,11 +4754,13 @@ DataService.addClassProperties(
*/
if (Array.isArray(manyChanges) && manyChanges.equals(addedValues)) {
manyChanges = {};
manyChanges.index = changeEvent.index;
changesForDataObject.set(key, manyChanges);
}

if (!isDataObjectBeingMapped) {
manyChanges.addedValues = registeredAddedValues = new Set(addedValues);
manyChanges.addedValues = addedValues;
manyChanges.addedValuesSet = registeredAddedValues = new Set(addedValues);
}
self._addDataObjectPropertyDescriptorValuesForInversePropertyDescriptor(
dataObject,
Expand Down
18 changes: 10 additions & 8 deletions data/service/expression-data-mapping.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2011,7 +2011,7 @@ exports.ExpressionDataMapping = DataMapping.specialize(/** @lends ExpressionData
_mapObjectPropertyToRawDataProperty: {
value: function(object, propertyName, data, rawPropertyName, added, removed, _rule, lastReadSnapshot, rawDataSnapshot) {

if((added && added.size > 0) || (removed && removed.size > 0 )) {
if((added && added.length > 0) || (removed && removed.length > 0 )) {
var tmpExtendObject,
//We derived object so we can pretend the value of the property is alternatively added, then removed, to get the mapping done.
//tmpExtendObject = Object.create(object),
Expand All @@ -2026,7 +2026,7 @@ exports.ExpressionDataMapping = DataMapping.specialize(/** @lends ExpressionData

data[rawPropertyName] = aPropertyChanges;

if(added && added.size > 0) {
if(added && added.length > 0) {
/*
Here we have a situation where in the most common case object[propertyName] is not equal to the content of added, like if there were pre-exising values.

Expand All @@ -2038,7 +2038,7 @@ exports.ExpressionDataMapping = DataMapping.specialize(/** @lends ExpressionData
tmpExtendObject = {};
for(i=0, countI = requirements.length; ( i<countI); i++ ) {
//added is a set, regular properties are array, not ideal but we need to convert to be able to map.
tmpExtendObject[requirements[i]] = (requirements[i] === propertyName) ? Array.from(added) : object[requirements[i]];
tmpExtendObject[requirements[i]] = (requirements[i] === propertyName) ? added : object[requirements[i]];
}

//tmpExtendObject[propertyName] = Array.from(added);
Expand All @@ -2054,15 +2054,15 @@ exports.ExpressionDataMapping = DataMapping.specialize(/** @lends ExpressionData
}
}

if(removed && removed.size > 0 ) {
if(removed && removed.length > 0 ) {

requirements = (requirements || _rule.requirements);
// tmpExtendObject[propertyName] = Array.from(result);
tmpExtendObject = (tmpExtendObject || {});

for(i=0, countI = requirements.length; ( i<countI); i++ ) {
//added is a set, regular properties are array, not ideal but we need to convert to be able to map.
tmpExtendObject[requirements[i]] = (requirements[i] === propertyName) ? Array.from(removed) : object[requirements[i]];
tmpExtendObject[requirements[i]] = (requirements[i] === propertyName) ? removed : object[requirements[i]];
}

removedResult = this.__mapObjectToRawDataProperty(tmpExtendObject, diffData, rawPropertyName, _rule, lastReadSnapshot, rawDataSnapshot);
Expand All @@ -2078,11 +2078,11 @@ exports.ExpressionDataMapping = DataMapping.specialize(/** @lends ExpressionData
}

if(addedResultIsPromise && removedResultIsPromise) {
return Promise.all([addedResultIsPromise, removedResultIsPromise]);
return Promise.all([addedResult, removedResult]);
} else if(addedResultIsPromise) {
return addedResultIsPromise;
return addedResult;
} else if(removedResultIsPromise) {
return removedResultIsPromise;
return removedResult;
}

return;
Expand Down Expand Up @@ -2114,6 +2114,8 @@ exports.ExpressionDataMapping = DataMapping.specialize(/** @lends ExpressionData
value;





if (isRelationship && rule.converter) {
this._prepareObjectToRawDataRule(rule);
Expand Down
55 changes: 52 additions & 3 deletions data/service/raw-data-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { last } = require("../../core/frb/operators");

var DataService = require("./data-service").DataService,
assign = require("../../../core/frb/assign"),
compile = require("../../core/frb/compile-evaluator"),
Expand Down Expand Up @@ -1552,6 +1554,7 @@ RawDataService.addClassProperties({
var mapping = this.mappingForType(type);
if(mapping && mapping.rawDataPrimaryKeyCompiledExpressions) {
throw "-dataIdentifierForTypeRawData(): Primary key missing for type '"+type.name+", rawData "+JSON.stringify(rawData);
// console.error("-dataIdentifierForTypeRawData(): Primary key missing for type '"+type.name+", rawData "+JSON.stringify(rawData))
}
}
}
Expand Down Expand Up @@ -1662,7 +1665,9 @@ RawDataService.addClassProperties({
for (i = 0, countI = rawDataKeys.length; (i < countI); i++) {

iUpdatedRawDataValue = rawData[rawDataKeys[i]];
if (isFromUpdate && iUpdatedRawDataValue && ((iHasAddedValues = iUpdatedRawDataValue.hasOwnProperty("addedValues")) || (iHasRemovedValues = iUpdatedRawDataValue.hasOwnProperty("removedValues")))) {
iHasAddedValues = iUpdatedRawDataValue && iUpdatedRawDataValue.hasOwnProperty("addedValues");
iHasRemovedValues = iUpdatedRawDataValue && iUpdatedRawDataValue.hasOwnProperty("removedValues");
if (isFromUpdate && (iHasAddedValues || iHasRemovedValues)) {

canRemoveRawDataKey = true;
iCurrentRawDataValue = snapshot[rawDataKeys[i]];
Expand Down Expand Up @@ -1776,6 +1781,18 @@ RawDataService.addClassProperties({
}
},

/**
* Removes the pending snapshot of the values of record for the DataIdentifier argument
*
* @private
* @argument {DataIdentifier} dataIdentifier
*/
removePendingSnapshot: {
value: function (dataIdentifier) {
this._pendingSnapshot.delete(dataIdentifier);
}
},

/**
* Returns the snapshot associated with the DataIdentifier argument if available
*
Expand Down Expand Up @@ -1845,10 +1862,12 @@ RawDataService.addClassProperties({
pendingSnapshotForDataIdentifier: {
value: function (dataIdentifier) {
let pendingSnapshot = this._pendingSnapshot.get(dataIdentifier);
let tmpPendingSnapshot;
if(!pendingSnapshot) {
let snapshot = this.snapshotForDataIdentifier(dataIdentifier);

if(snapshot) {
tmpPendingSnapshot = Object.assign({}, snapshot);
pendingSnapshot = Object.create(snapshot);
this._pendingSnapshot.set(dataIdentifier, pendingSnapshot);
}
Expand Down Expand Up @@ -4427,7 +4446,35 @@ RawDataService.addClassProperties({
// //when saved, but what if save fails and changes happen in-between?
// operationData[aRawProperty] = aPropertyChanges;

return this._mapObjectPropertyToRawData(object, aProperty, operationData, undefined/*context*/, aPropertyChanges.addedValues, aPropertyChanges.removedValues, lastReadSnapshot, rawDataSnapshot);

let result = this._mapObjectPropertyToRawData(object, aProperty, operationData, undefined/*context*/, aPropertyChanges.addedValues, aPropertyChanges.removedValues, lastReadSnapshot, rawDataSnapshot);
if (this._isAsync(result)) {
return result.then((output) => {
let mapping = this.mappingForObject(object),
rawRules = mapping.rawDataMappingRulesForObjectPropertyName(aProperty),
rawDataProperty;

if(rawRules) {
var iterator = rawRules.values();

while((rule = iterator.next().value)) {
rawDataProperty = rule.targetPath;
if (operationData[rawDataProperty]) {
operationData[rawDataProperty].index = aPropertyChanges.index;
}
if (operationData[rawDataProperty] && operationData[rawDataProperty].removedValues) {
operationData[rawDataProperty].removedValues.index = aPropertyChanges.index;
}
}
}

return output;
})
} else {
return result;
}

// return this._mapObjectPropertyToRawData(object, aProperty, operationData, undefined/*context*/, aPropertyChanges.addedValues, aPropertyChanges.removedValues, lastReadSnapshot, rawDataSnapshot);

}
else {
Expand Down Expand Up @@ -4958,15 +5005,17 @@ RawDataService.addClassProperties({
self.rootService.registerUniqueObjectWithDataIdentifier(iObject, iDataIdentifier);
}
self.recordSnapshot(iDataIdentifier, iOperation.data);

self.removePendingSnapshot(iDataIdentifier);
} else if (iOperation.type === UpdateOperation || iOperation.type === NoOpOperation) {
iDataIdentifier = self.dataIdentifierForObject(iObject);
self.recordSnapshot(iDataIdentifier, iOperation.data, true);
self.removePendingSnapshot(iDataIdentifier);
} else if (iOperation.type === DeleteOperation) {
iDataIdentifier = self.dataIdentifierForObject(iObject);

//Removes the snapshot we have for iDataIdentifier
self.removeSnapshot(iDataIdentifier);
self.removePendingSnapshot(iDataIdentifier);
}

}
Expand Down