zjeh / force-dot-com-esapi

Automatically exported from code.google.com/p/force-dot-com-esapi
0 stars 0 forks source link

Id not available for child object inserts when AccessControllerInterface.insertAsUser is used on a parent #1

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I have wizards that create multiple objects that have master/detail 
(parent/child) relationships in one 
go. So the child needs hooking up to the parent meaning the parent id must be 
available. After an insert 
an SObject's Id field is populated and so this sequence works:

Parent__c p = new Parent__c();
insert p;
Child__c c = new Child__c();
c.Parent__c = p.Id;
insert c;

But AccessControllerInterface.insertAsUser does not expose the resulting id of 
the inserted object...

Given that the id value cannot be assigned, returning the created SObject would 
allow the access control 
to be introduced with the least code changes like this:

Parent__c p = new Parent__c();
p = (Parent__c) ESAPI.accessController().insertAsUser(p, ...);
Child__c c = new Child__c();
c.Parent__c = p.Id;
c = (Child__c) ESAPI.accessController().insertAsUser(c, ...);

But related list-based calls already return Database.SaveResult[]. I am unclear 
why this approach is used 
rather than throwing an exception. If an exception-based approach can be 
accomplished then the natural 
equivalent for the list-based case is obviously:

Parent__c[] ps = ...;
ps = (Parent__c[]) ESAPI.accessController().insertAsUser(ps, ...);

Original issue reported on code.google.com by keith.clarke.claimvantage@gmail.com on 27 May 2010 at 11:42

GoogleCodeExporter commented 9 years ago
Thanks for reporting this issue, we will be looking in to this shortly.

Original comment by apex.es...@gmail.com on 1 Jun 2010 at 4:59

GoogleCodeExporter commented 9 years ago
Fixed in r40 (http://code.google.com/p/force-dot-com-esapi/source/detail?r=40)
insertAsUser and updateAsUser now return the objects they create internally.

This fix should be available form the downloads page in the next release.

For lists we chose to return SaveResult[] and not just a single exception, 
because 
this way you can get feedback on the specific object that caused the failure. 
Also, 
you may choose to allow some of them to be inserted/updated even if some are 
failing.

With lists you can still use the SaveResult[i].getId() to get the id of the new 
object after insertion.

Original comment by apex.es...@gmail.com on 1 Jun 2010 at 10:04

GoogleCodeExporter commented 9 years ago
In the list case, you are forcing the user of your API to have to deal with a 
pair of lists (pass both, 
iterate over both): the object list they passed into your API and an Id list 
they have to build for 
themselves from the returned SaveResult list. The poor code this produces 
doesn't seem consistent with 
your objective "to make it easier for programmers to retrofit security into 
existing applications or 
build a solid foundation for new development".

It seems that code to support the error case (that would perhaps be better 
handled using exceptions) 
may be getting in the way of a clean interface for the normal case.

Original comment by keith.clarke.claimvantage@gmail.com on 4 Jun 2010 at 7:42

GoogleCodeExporter commented 9 years ago
In the list case we chose an approach that tries to mimic the "update arr;" 
and "Database.insert(arr);" calls as much as possible.

For example, if you call "update arr;" it assumes a "all or none" mode. The 
same 
thing happens if you call "Database.update(arr, true);" it assumes all or none. 
As 
long as you are in all or none mode, if one of the objects in the array fails 
to 
update, none of the updates will be committed and you will get an exception. 
Only if 
you call "Database.update(arr, false);" then you choose a best effort mode. In 
this 
case, even if some objects fail, it will not throw an exception. Instead it 
will 
return an array of SaveResult objects with the results of each update.

The ESAPI library has a similar approach. As long as you use the all or none 
mode, 
if any of the objects fail to insert/update you should get an exception. Only 
if you 
are in best effort mode, it will not throw an exception, in which case you can 
use 
the SaveResult array. 

In addition, the ESAPI library also uses the same default as the 
Database.insert/update which is all or none mode.

However, the one difference is, when calling "insert arr;" or "Database.insert" 
the 
array gets populated with the ids that were just created. In our case, because 
we 
create new objects inside the library, the caller's original objects don't get 
populated with those ids. (And we can't just set them in the library because 
the ID 
field is read only.) We could have returned the new array of objects we use 
internally as a return value, but we chose to stick to the 
Database.insert/update 
approach which was to return the SaveResult array. Especially that the IDs are 
also 
available from the SaveResult objects.

Having said that, I think this is not the easiest approach for users to work 
with 
when they need to continue and manipulate the objects they just inserted. We 
are 
thinking of the best way to implement it in such a way that it will also return 
the 
new objects with the IDs set, and also the SaveResult array.

Original comment by apex.es...@gmail.com on 4 Jun 2010 at 10:00

GoogleCodeExporter commented 9 years ago
Thanks for the additional consideration of this. 

Original comment by keith.clarke.claimvantage@gmail.com on 5 Jun 2010 at 2:04

GoogleCodeExporter commented 9 years ago
Changed the return value of the insertAsUser/updateAsUser/deleteAsUser 
functions in r45 (in array mode) to return a special object which can be used 
to get info such as the Database.SaveResult[] and the actual objects that were 
used to insert/update the db.

This way, we can have the SaveResult info the same way the Database.insert 
works with arrays. And at the same time we can return the actual objects that 
were used to insert/update the db.

Original comment by apex.es...@gmail.com on 9 Jun 2010 at 11:18