Skip to content

Commit 7b117d4

Browse files
authored
[#186] Fix "Internal Error" in mapping Sample Source / Single Record Reconciliation when first property has no source (#188)
1 parent 23c68ee commit 7b117d4

7 files changed

Lines changed: 183 additions & 15 deletions

File tree

openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/mapping/association/DataAssociationManagementView.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,15 @@ define([
7070
this.mapping = this.getCurrentMapping();
7171
this.mappingSync = this.getSyncNow();
7272
this.data.numRepresentativeProps = this.getNumRepresentativeProps();
73-
this.data.sourceProps = _.pluck(this.mapping.properties,"source").slice(0,this.data.numRepresentativeProps);
74-
this.data.targetProps = _.pluck(this.mapping.properties,"target").slice(0,this.data.numRepresentativeProps);
73+
// Only keep properties that actually define a "source"/"target"; a leading
74+
// undefined value would break the sample search query. See discussion #186.
75+
// Use native Array filter/slice (lodash 3 _.first ignores the count arg).
76+
this.data.sourceProps = _.pluck(this.mapping.properties, "source")
77+
.filter(function(source){ return !!source; })
78+
.slice(0, this.data.numRepresentativeProps);
79+
this.data.targetProps = _.pluck(this.mapping.properties, "target")
80+
.filter(function(target){ return !!target; })
81+
.slice(0, this.data.numRepresentativeProps);
7582
this.data.hideSingleRecordReconButton = mappingUtils.readOnlySituationalPolicy(this.mapping.policies);
7683

7784
this.data.reconAvailable = false;

openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/mapping/behaviors/SingleRecordReconciliationView.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* information: "Portions copyright [year] [name of copyright owner]".
1313
*
1414
* Copyright 2015-2016 ForgeRock AS.
15+
* Portions copyright 2026 3A Systems, LLC.
1516
*/
1617

1718
define([
@@ -79,7 +80,13 @@ define([
7980
},
8081

8182
setupSearch: function(){
82-
var autocompleteProps = _.pluck(this.data.mapping.properties,"source").slice(0,this.getNumRepresentativeProps());
83+
// Only keep properties that actually define a "source"; otherwise the
84+
// first representative property may be undefined and break the sample
85+
// search query (empty _sortKeys= => HTTP 500). See discussion #186.
86+
// Use native Array filter/slice (lodash 3 _.first ignores the count arg).
87+
var autocompleteProps = _.pluck(this.data.mapping.properties, "source")
88+
.filter(function(source){ return !!source; })
89+
.slice(0, this.getNumRepresentativeProps());
8390

8491
mappingUtils.setupSampleSearch($("#findSampleSource",this.$el), this.data.mapping, autocompleteProps, _.bind(function(item) {
8592
conf.globalData.testSyncSource = item;

openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/mapping/properties/AttributesGridView.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* information: "Portions copyright [year] [name of copyright owner]".
1313
*
1414
* Copyright 2016 ForgeRock AS.
15+
* Portions copyright 2026 3A Systems, LLC.
1516
*/
1617

1718
define([
@@ -111,7 +112,13 @@ define([
111112
this.data.mapProps = mapProps;
112113

113114
if (this.data.usesDynamicSampleSource) {
114-
let autocompleteProps = _.pluck(this.model.mapping.properties,"source").slice(0, this.model.numRepresentativeProps);
115+
// Only keep properties that actually define a "source"; otherwise the
116+
// first representative property may be undefined and break the sample
117+
// search query (empty _sortKeys= => HTTP 500). See discussion #186.
118+
// Use native Array filter/slice (lodash 3 _.first ignores the count arg).
119+
let autocompleteProps = _.pluck(this.model.mapping.properties, "source")
120+
.filter((source) => !!source)
121+
.slice(0, this.model.numRepresentativeProps);
115122

116123
mappingUtils.setupSampleSearch($("#findSampleSource", this.$el), this.model.mapping, autocompleteProps, (item) => {
117124
item.IDMSampleMappingName = this.model.mapping.name;

openidm-ui/openidm-ui-admin/src/main/js/org/forgerock/openidm/ui/admin/mapping/util/MappingUtils.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,14 @@ define([
6565

6666
obj.setupSampleSearch = function(el, mapping, autocompleteProps, selectSuccessCallback){
6767
var searchList,
68-
selectedItem;
68+
selectedItem,
69+
// Guard against empty/undefined entries (e.g. properties without a "source").
70+
cleanProps = _.compact(autocompleteProps),
71+
primaryProp = cleanProps[0];
6972

7073
el.selectize({
71-
valueField: autocompleteProps[0],
72-
searchField: autocompleteProps,
74+
valueField: primaryProp,
75+
searchField: cleanProps,
7376
maxOptions: 10,
7477
create: false,
7578
onChange: function() {
@@ -96,18 +99,20 @@ define([
9699
item: function(item, escape) {
97100
selectedItem = item;
98101

99-
return "<div>" +escape(item[autocompleteProps[0]]) +"</div>";
102+
return "<div>" +escape(item[primaryProp]) +"</div>";
100103
}
101104
},
102105
load: function(query, callback) {
103-
if (!query.length || query.length < 2 || !autocompleteProps.length) {
106+
if (!query.length || query.length < 2 || !cleanProps.length) {
104107
return callback();
105108
}
106109

107-
searchDelegate.searchResults(mapping.source, autocompleteProps, query).then(function(response) {
108-
if(response) {
110+
searchDelegate.searchResults(mapping.source, cleanProps, query).then(function(response) {
111+
if(response && response.length) {
109112
searchList = response;
110-
callback([response]);
113+
// searchResults already returns an array of records; pass it
114+
// through as-is (selectize expects a flat array of options).
115+
callback(response);
111116
} else {
112117
searchList = [];
113118

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,63 @@
11
define([
2-
"org/forgerock/openidm/ui/admin/mapping/util/MappingUtils"
3-
], function (MappingUtils) {
2+
"jquery",
3+
"sinon",
4+
"org/forgerock/openidm/ui/admin/mapping/util/MappingUtils",
5+
"org/forgerock/openidm/ui/common/delegates/SearchDelegate"
6+
], function ($, sinon, MappingUtils, SearchDelegate) {
47
QUnit.module('MappingUtils Tests');
8+
9+
QUnit.test("setupSampleSearch passes a flat array of records to the selectize load callback", function (assert) {
10+
var done = assert.async(),
11+
records = [{ email: "jsanchez@example.com", lastName: "Sanchez", firstName: "Jane" }],
12+
capturedConfig,
13+
selectizeStub = sinon.stub($.fn, "selectize", function (config) {
14+
capturedConfig = config;
15+
return this;
16+
}),
17+
searchStub = sinon.stub(SearchDelegate, "searchResults", function () {
18+
return $.Deferred().resolve(records).promise();
19+
});
20+
21+
try {
22+
MappingUtils.setupSampleSearch(
23+
$("<input>"),
24+
{ source: "system/hr/account" },
25+
["email", "lastName", "firstName"],
26+
function () {}
27+
);
28+
29+
assert.equal(capturedConfig.valueField, "email", "valueField is the first non-empty prop");
30+
31+
capturedConfig.load("Sanchez", function (options) {
32+
// Regression test: options must be the flat array of records, not [[...]].
33+
assert.deepEqual(options, records, "load callback receives a flat array of records");
34+
done();
35+
});
36+
} finally {
37+
selectizeStub.restore();
38+
searchStub.restore();
39+
}
40+
});
41+
42+
QUnit.test("setupSampleSearch ignores props without a source (compacts valueField/searchField)", function (assert) {
43+
var capturedConfig,
44+
selectizeStub = sinon.stub($.fn, "selectize", function (config) {
45+
capturedConfig = config;
46+
return this;
47+
});
48+
49+
try {
50+
MappingUtils.setupSampleSearch(
51+
$("<input>"),
52+
{ source: "managed/user" },
53+
[undefined, "userName", "sn"],
54+
function () {}
55+
);
56+
57+
assert.equal(capturedConfig.valueField, "userName", "valueField falls back to first non-empty prop");
58+
assert.deepEqual(capturedConfig.searchField, ["userName", "sn"], "searchField has no undefined entries");
59+
} finally {
60+
selectizeStub.restore();
61+
}
62+
});
563
});

openidm-ui/openidm-ui-common/src/main/js/org/forgerock/openidm/ui/common/delegates/SearchDelegate.js

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,41 @@ define([
2323

2424
var obj = new AbstractDelegate(constants.host + "/" + constants.context);
2525

26+
/**
27+
* Builds the search URL for a resource query.
28+
*
29+
* Note: only the first non-empty property is used for "_sortKeys". When the
30+
* supplied props array starts with empty/undefined values (e.g. a mapping
31+
* whose first property has no "source"), an empty "_sortKeys=" must NOT be
32+
* emitted, otherwise the backend (CREST/IDM) returns an HTTP 500 error.
33+
*
34+
* In addition, "_sortKeys" is omitted for system resources ("system/..."),
35+
* because many connectors (e.g. the CSV connector) do not support
36+
* server-side sorting and would fail the whole query. Sample searches are
37+
* capped at a handful of results anyway, so the sort order is not required.
38+
*/
39+
obj.buildSearchUrl = function (resource, props, searchString, comparisonOperator, additionalQuery, maxPageSize) {
40+
var sortKey = _.find(props, function (p) { return !!p; }),
41+
sortableResource = !/^\/?system\//.test(resource),
42+
url = "/" + resource + "?";
43+
44+
if (sortKey && sortableResource) {
45+
url += "_sortKeys=" + sortKey + "&";
46+
}
47+
48+
// [a,b] => "a or (b)"; [a,b,c] => "a or (b or (c))"
49+
url += "_pageSize=" + maxPageSize +
50+
"&_queryFilter=" + obj.generateQueryFilter(props, searchString, additionalQuery, comparisonOperator);
51+
52+
return url;
53+
};
54+
2655
obj.searchResults = function (resource, props, searchString, comparisonOperator, additionalQuery) {
2756
var maxPageSize = 10;
2857

2958
return this.serviceCall({
3059
"type": "GET",
31-
"url": "/" + resource + "?_sortKeys=" + props[0] + "&_pageSize=" + maxPageSize + "&_queryFilter=" + obj.generateQueryFilter(props, searchString, additionalQuery, comparisonOperator)// [a,b] => "a or (b)"; [a,b,c] => "a or (b or (c))"
60+
"url": obj.buildSearchUrl(resource, props, searchString, comparisonOperator, additionalQuery, maxPageSize)
3261
}).then(
3362
function (qry) {
3463
return _.take(qry.result, maxPageSize);//we never want more than 10 results from search in case _pageSize does not work

openidm-ui/openidm-ui-common/src/test/qunit/tests/org/forgerock/openidm/ui/common/delegates/SearchDelegateTest.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
/**
2+
* The contents of this file are subject to the terms of the Common Development and
3+
* Distribution License (the License). You may not use this file except in compliance with the
4+
* License.
5+
*
6+
* You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
7+
* specific language governing permission and limitations under the License.
8+
*
9+
* When distributing Covered Software, include this CDDL Header Notice in each file and include
10+
* the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
11+
* Header, with the fields enclosed by brackets [] replaced by your own identifying
12+
* information: "Portions copyright [year] [name of copyright owner]".
13+
*
14+
* Portions copyright 2026 3A Systems, LLC.
15+
*/
116
define([
217
"org/forgerock/openidm/ui/common/delegates/SearchDelegate"
318
], function (SearchDelegate) {
@@ -12,4 +27,44 @@ define([
1227
assert.equal(SearchDelegate.generateQueryFilter(props, search), '(userName sw "test" or (givenName sw "test" or (sn sw "test")))', "Basic Query Filter Generated");
1328
assert.equal(SearchDelegate.generateQueryFilter(props, search, additionalQuery, comparisonOperator), '((userName sw "test" or (givenName sw "test" or (sn sw "test"))) and (true))', "Complex Query Filter Generated");
1429
});
30+
31+
QUnit.test("buildSearchUrl uses the first property as _sortKeys", function (assert) {
32+
var url = SearchDelegate.buildSearchUrl("managed/user", ["userName", "sn"], "test", null, null, 10);
33+
34+
assert.equal(
35+
url,
36+
'/managed/user?_sortKeys=userName&_pageSize=10&_queryFilter=(userName sw "test" or (sn sw "test"))',
37+
"_sortKeys is set to the first property"
38+
);
39+
});
40+
41+
QUnit.test("buildSearchUrl skips empty _sortKeys when the first property is missing (discussion #186)", function (assert) {
42+
// Mapping whose first property has no "source" => leading undefined value.
43+
var url = SearchDelegate.buildSearchUrl("managed/user", [undefined, "sn"], "test", null, null, 10);
44+
45+
assert.equal(url.indexOf("_sortKeys=&"), -1, "no empty _sortKeys= is emitted");
46+
assert.equal(
47+
url,
48+
'/managed/user?_sortKeys=sn&_pageSize=10&_queryFilter=(sn sw "test")',
49+
"_sortKeys falls back to the first non-empty property"
50+
);
51+
});
52+
53+
QUnit.test("buildSearchUrl omits _sortKeys entirely when no property is usable", function (assert) {
54+
var url = SearchDelegate.buildSearchUrl("managed/user", [undefined, ""], "test", null, null, 10);
55+
56+
assert.equal(url.indexOf("_sortKeys"), -1, "no _sortKeys parameter is present at all");
57+
assert.equal(url.indexOf("?_pageSize=10"), "/managed/user".length, "the query string starts directly with _pageSize");
58+
});
59+
60+
QUnit.test("buildSearchUrl omits _sortKeys for system resources that may not support sorting", function (assert) {
61+
var url = SearchDelegate.buildSearchUrl("system/hr/account", ["email", "lastName"], "Sanchez", null, null, 10);
62+
63+
assert.equal(url.indexOf("_sortKeys"), -1, "no _sortKeys parameter is sent to a system connector");
64+
assert.equal(
65+
url,
66+
'/system/hr/account?_pageSize=10&_queryFilter=(email sw "Sanchez" or (lastName sw "Sanchez"))',
67+
"system resource query has no _sortKeys but keeps the query filter"
68+
);
69+
});
1570
});

0 commit comments

Comments
 (0)