apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
914 stars 518 forks source link

Type casting SObjectField to 'String' throws errors for non-string field types #495

Open cnwork opened 2 months ago

cnwork commented 2 months ago

Describe the bug In the fflib_SObjects class, all these methods:

are type casting SObjectFields to String. This is fine for all the field types that can be casted to String. However, type casting the number types to a String as all the above methods are doing: (String) record.get(field) throws errors like Invalid conversion from runtime type Integer to String, or Invalid conversion from runtime type Decimal to String

To Reproduce Modify the generateDomain method from the fflib_SObjectsTest class to include in the Accounts the NumberOfEmployees field with a random Integer value and then run the entire test class. The tests for all the above methods will now fail.

Steps to reproduce the behavior:

  1. Create a regular scratch org
  2. Deploy ApexMocks and apex-common to it
  3. Modify the fflib_SObjectsTest.cls to include the NumberOfEmployees field with an interger value to one or more of the new Accounts and deploy the changes
  4. Run the entire test class
  5. Notice error: System.TypeException: Invalid conversion from runtime type Integer to String

Expected behavior When the above methods are called for any field no matter its type, they correctly return the promised values or records.

Screenshots and text of error observed Example of modified generateDomain method:

private static DomainAccounts generateDomain()
    {
        DomainAccounts domain = new DomainAccounts(
                new List<Account>
                {
                        new Account(Name = 'A', ShippingCountry = 'USA'),
                        new Account(Name = 'B', ShippingCountry = 'Ireland'),
                        new Account(Name = 'C', ShippingCountry = 'UK', NumberOfEmployees = 10),
                        new Account(Name = 'D', ShippingCountry = ''),
                        new Account(Name = 'E', NumberOfEmployees = 20),
                        new Account(),
                        new Account(Name = 'G', ShippingCountry = 'Canada', NumberOfEmployees = 30)
                }
        );
        return domain;
    }

and the error for one of the existing tests (more are needed to be added to show the extent of the failures)

fflib_SObjectsTest.itShouldReturnRecordsWithAllFieldValues                    fflib_Objects       Fail     14%      System.TypeException: Invalid conversion from runtime type Integer to String                            
                                                                                                                    Class.fflib_SObjects.getRecordsWithAllNotBlankFieldValues: line 340, column 1                           
                                                                                                                    Class.fflib_SObjectsTest.DomainAccounts.selectPopulatedRecords: line 343, column 1                      
                                                                                                                    Class.fflib_SObjectsTest.itShouldReturnRecordsWithAllFieldValues: line 141, column 1                    8

Version Did you try to reproduce the problem against the latest fflib-apex-common code? Yes,

cnwork commented 2 months ago

Would love to have some feedback on this and the PR.

Thanks, Cristian

@daveespo @wimvelzeboer @ImJohnMDaniel

ImJohnMDaniel commented 2 months ago

Apologies for the delay. I am DF24 and I am planning to get into this when I get back.

cnwork commented 2 months ago

Apologies for the delay. I am DF24 and I am planning to get into this when I get back.

😄 I had a feeling that everyone was busy with DF24. Wish I was there too! Enjoy, no worries we can talk more next week.

Have a great DF24 and a nice weekend! Cristian

cnwork commented 1 month ago

Any chance we can look into this together? There are also some other discussions that would be nice to have feedback on.

Thanks! Cris

@daveespo @wimvelzeboer @ImJohnMDaniel

cnwork commented 4 days ago

I'm sorry to keep pushing but aside from this Issue I would like to have other discussions and proposals/questions but it looks like nobody is actually following this repo anymore. I know everyone is involved in something and busy with work and life and all but this library is something so valuable for (I guess) so many people and companies that it would be really disappointing if it wouldn't be maintained anymore. I want to be positive and patient but it's been more than 2 months since I opened this issue, a PR and a discussion and I would like to open more but with no feedback I feel like I'm just talking to myself 😄 Can anyone of you please show some signs of life 😉 ?

Thank you very much and sorry for tagging as many of you as I could find: @afawcett @afawcettffdc @ImJohnMDaniel @daveespo @wimvelzeboer @daveerickson @stohn777

ImJohnMDaniel commented 3 days ago

G'day @cnwork -- Apologies for the delay. You are correct in saying that "...everyone is involved something and busy with work and life" but I have time coming up soon to focus on this and other questions with AEP/FFLIB that have been stacking up. I will look into this one this weekend and get you some feedback.