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

[sfdcfox]

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 ) );

Where:

* 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.