function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion

BUG: Id.valueOf does not identify Id values correctly.

See last paragraph for summary...


I was trying to create an adaptive loader routine in Apex Code, and I came across an interesting bug. First, let's take a look at what I was attempting to do:


sobject record = cache.load( recordId );
set< id > relatedIds = new set< Id >( );

for( SObjectField field: Cache.getFieldTokens( record.getSObjectType( ) ) ) {
   if( record.get( field ) != null && record.get( field ) instanceOf id ) {
       relatedIds.add( Id.valueOf( record.get( field ) );
map< id, sobject > related = new map< id, sobject >( Cache.load( relatedIds ) );


* Cache.load( Id ) returns a single record from a static map, querying the record, if necessary.

* Cache.getFieldTokens( SObjectType ) returns all tokens from a SObjectType.getDescribe().fields.getMap().values(), again loading from a static map if previously defined.

* Cache.load( Set< Id > ) is a batch version of above, returning a list of SObject records in arbitrary order, querying records that are missing from the cache, and capable of returning multiple types of SObjects.


When I tried this initially, I got an odd exception: "Invalid ID."; the exception was thrown from my Cache class, so I wondered how it could have gotten there.


I fiddled around with the straightforward means of assigning an ID via a string:


Id a = '0013000000f3adA'; // OKAY
Id b = 'john doe';        // EXCEPTION

So, it would appear the setter function works fine.


Next, I tried casting:


String a = '0013000000f3Adf', b = 'john doe';
Id c = ( Id )a; // OKAY
Id d = ( Id )b; // EXCEPTION

So far, so good; it's also using the setter function of ID.


Next, I tried using Id.valueOf:


Id a = Id.valueOf( '0013000000d3afA' ); // OKAY
Id b = Id.valueOf( 'john doe' );        // OKAY ?!?!

This means that Id.valueOf doesn't use the setter method, but instead internally constructs an ID.


The next tidbit came when I tested the Set<T> class against an ID.


First, a straight assignment:


Set< Id > a = new set< Id >( );
a.add( '00130000003faZs' ); // OKAY
a.add( 'john doe' ); // EXCEPTION

So, it seems that Id's setter is in play here.


Next, I tried using the defunct valueOf:


Set< Id > a = new Set< Id >( );
a.add( Id.valueOf( 'john doe' ) ); // OKAY ?!?!

So, it seems that a "corrupted" ID value will be accepted into a set of IDs.


This led to the next problem:


for( Id b: a) {
  // Do something

If a is a set of IDs, and a corrupted ID value (via valueOf) is in the set, you will receive an "invalid ID" error here.


Finally, this led me back to my source code:


if( Id.valueOf( 'john doe' ) instanceOf Id ) {
  System.debug('** Invalid ID was accepted by instanceOf **');

So, Set<T>.add(Object) apparently checks the class of the incoming object against the class of its template, and automatically accepts them without question, otherwise attempts a cast (calling the correctly-working setter function).


I submitted a case to support, and they told me to go away because I don't have premier support, and all I wanted to do was log a bug with the dev team.


Hopefully an admin will see this and it will get logged as a bug. In the interim, the community should note that Id.valueOf is broken, and instanceOf is also broken as a side-effect. Instead, you should always use casting, and try-catch the cast so you can detect incorrect ID values.


Best Answer chosen by sfdcfox
For what it's worth this no longer seems to be the case. 

In APIv29 this fails with a System.StringException:
Id wat = Id.valueOf('foxy');

All Answers

For what it's worth this no longer seems to be the case. 

In APIv29 this fails with a System.StringException:
Id wat = Id.valueOf('foxy');
This was selected as the best answer
Thanks. I hadn't revisited this since I posted this some months ago, and I just tested with 'john doe', which no longer works. I think this means I can revert my code back to the simpler, "more sane" version that I had before I implemented the workaround...

Curious, it still returns true for the following =):

System.debug(',.;:-00#$%^?&*()=+' instanceof Id); // true
System.debug('     00           ' instanceof Id); // true

Id a = Id.valueOf(',.;:-00#$%^?&*()=+'); // valid operation
Id b = Id.valueOf('     00           '); // valid operation