Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve #124 - Improve testmethod finalizerThatInvokesDmlShouldCauseException #125

Conversation

benjaminloerincz
Copy link
Contributor

This pull request addresses issue #124 by suggesting an adjustment to the existing DML approach.
Instead of utilizing insert new Account(Name = 'Acme'); to increase the value of Limits.getDmlStatements(), I propose using EventBus.publish(new ProcessExceptionEvent()).

Here's the rationale behind this change:

  • The apex trigger actions framework works for triggers on platform events as well.
  • ProcessExceptionEvent is a standard platform event.
  • Publishing this event increments Limits.getDmlStatements() by one, mirroring the behavior of DML statements on standard/custom objects.
  • The act of publishing such events should consistently be feasible without encountering any issues.
  • Unlike the current method of inserting an account, publishing the event is not susceptible to failure due to validation rules or any other customizing in customer orgs.

Furthermore, I removed the unused variables ACCOUNT, SOBJECT_TYPE, OPERATION.

  • Tests pass

@benjaminloerincz benjaminloerincz changed the title Subject: Resolve #124 - Improve testmethod finalizerThatInvokesDmlShouldCauseException Resolve #124 - Improve testmethod finalizerThatInvokesDmlShouldCauseException Nov 14, 2023
@mitchspano
Copy link
Owner

mitchspano commented Nov 20, 2023

Thanks for the contribution @benjaminloerincz,

This actually highlights a deeper issue with the current implementation; if platform event publication consumes a DML operation, then how are we to distinguish between normal sObject record DML operations performed in the finalizer context vs platform event publication 🤔

The original intent was to allow EventBus.publish but throw error on Database.insert/update/delete/undelete.

I might need to go back to the drawing board on this one.

@benjaminloerincz
Copy link
Contributor Author

The original intent was to allow EventBus.publish but throw error on Database.insert/update/delete/undelete.

Ah, I see. I thought finalizer would be more about things like batch jobs, queueables, future methods.

Regarding the issue: There is Limits.getPublishImmediateDML() for platform events of type "Publish Immediately". But unfortunately there is no similar method for platform events of type "Publish After Commit", something like Limits.getPublishAfterCommitDML().

System.debug(Limits.getPublishImmediateDML()); // 0
Immediate__e i = new Immediate__e();
EventBus.publish(i);
System.debug(Limits.getPublishImmediateDML()); // 1
AfterCommit__e a = new AfterCommit__e();
EventBus.publish(a);
System.debug(Limits.getPublishImmediateDML()); // 1

I think this would be a good "Salesforce idea".

@mitchspano mitchspano closed this Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants