Allow to use @Transactional() annotation to entire service (apply to all methods)

[herbertpimentel]

give us a possibility to anotate the entire service class as transactional instead of only methods. and then apply it to each method of the service class;

[odavid]

Hi @herbertpimentel, PR would be happily accepted...

[cassinaooo]

I extended the Transactional decorator for entire classes using this custom decorator:

export interface TransactionalOptions {
    propagation: Propagation
    isolationLevel: IsolationLevel
}


const TransactionalService = (options: TransactionalOptions): ClassDecorator => {
    return (target: Function) => {
        for (const key of Object.getOwnPropertyNames(target.prototype)) {
            const methodDescriptor = Object.getOwnPropertyDescriptor(target.prototype, key)

            // assumes that only async methods need a transaction
            const isAsyncMethod =
                methodDescriptor &&
                methodDescriptor.value instanceof Object.getPrototypeOf(async function() {}).constructor

            if (!methodDescriptor || !isAsyncMethod) {
                continue
            }

            Transactional(options)(target, key, methodDescriptor)

            Object.defineProperty(target.prototype, key, methodDescriptor)
        }
    }
}

It only adds transactions to async methods, since that is what made sense in my application.

@odavid Should this be a feature of the lib itself? How could I go about implementing it?

[odavid]

Hey @cassinaooo, Can you please open a PR for that decorator?

For this code to be added, I believe we need:

• Tests
• README section
• Since this decorator is an implicit one, and treats all the methods the same, I think we should also introduce an IgnoreTransactionalServiceMethod (Name is too long 😄 ) decorator that we can mark a method as not being decorated implicitly

Let me know what you think...
Cheers!

[cassinaooo]

@odavid Is there a way for the @Transactional decorator to be aware of this new class decorator, or vice-versa? I'm not familiar with meta programming on typescript, maybe Reflect Metadata?

Then we just need to decide on the override semantics instead of implementing a new decorator.

I don't really have anything against @IgnoreTransactionalServiceMethod or similar, just generally inclined to keep APIs as minimal as possible.

[odavid]

@cassinaooo - I believe reflect metadata should help listing methods that are decorated with Ignore

I believe you can start without the ignore feature.

It was just a suggestion, since setting Transactional on all async methods seems to me a bit "brutal", but a user can always remove the class decorator and put the Transactional explicitly.

Hope it helps...

[cassinaooo]

@odavid I see and agree with your point.

What should be expected of @Transactional when we have @TransactionalService active, specially in nested calls? This will eventually happen in large enough applications when services have methods that are called in multiple contexts.

I'm trying to understand how Transactional should behave in regards to propagation in the presence of TransactionalService, and if that helps avoid using @Ignore explicitly.

Suppose we have a class annotated with @TransactionalService({propagation: REQUIRED}) and decide to use @Transactional({propagation: NOT_SUPORTED}) in a given method. Would the ignore/override semantics be implicit, but clear? What are combinations of TransactionalService / Transactional that could be problematic?

[mscottnelson]

Just weighing in here since I would like to see this. It seems to me that @transactional would always override @TransactionalService for defined options, and use the options declared in @TransactionalService as defaults. At least, that would be my personal expectation.

As an additional alternative to a decorator like @NotTransactional(), perhaps @TransactionalService could also take an array of method names to exclude, eg:

@TransactionalService({ excludeMethods: ["myMethod"] })

[odavid]

Agree with @mscottnelson

[cassinaooo]

I'll try and submit a PR in the next few days. Thanks for your inputs @mscottnelson! I'll will follow your suggestion of @Transactional always overriding the class decorator. We can use @Transactional({propagation: NOT_SUPORTED}) as a substitute for @NotTransactional().