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

[Feature] Allow Custom Serializer for jobs #234

Closed
maxwellclarke-wf opened this issue Oct 14, 2021 · 10 comments
Closed

[Feature] Allow Custom Serializer for jobs #234

maxwellclarke-wf opened this issue Oct 14, 2021 · 10 comments
Labels
enhancement New feature or request
Milestone

Comments

@maxwellclarke-wf
Copy link

Are you running the latest version of JobRunr?
Yes

Describe the bug
There is no way to configure and pass in a custom json mapper instance from JobRunr.configure().

JobRunr.configure() always creates a JobMapper with its own JsonMapper. When supplying a custom-built StorageProvider, JobRunr always overwrites the JobMapper in the StorageProvider with its own default JobMapper instance with no way to override. This pattern continues down to the ObjectMapper supplied by JacksonJsonMapper, resulting in a mapper with no way to inject custom serialization/deserialization logic as a custom jackson ObjectMapper or register a Module.

Environment
I'm using JobRunr version: v4.0.0
I'm running on JRE / JDK (e.g. OpenJDK 1.8.0_292): OpenJDK 1.11
I'm using the following StorageProvider (e.g. Oracle / MongoDB / ...): MySQL/MariaDB

How to Reproduce
Provide a simple Github project or code fragment which shows how to reproduce the issue (ideally a Github repo).

Apologies it's in Kotlin:

        val customJobMapper = JacksonJsonMapper(customObjectMapper)
        val config = JobRunr.configure()
            // register Spring as the source of IoC lookups
            .useJobActivator(object : JobActivator {
                override fun <T> activateJob(type: Class<T>): T? = applicationContext.getBean(type)
            })
            .useStorageProvider(
                SqlStorageProviderFactory.using(dataSource).apply {
                    // This is what I'd like to do
                    setJobMapper(customJobMapper)
                }
            )
            .useBackgroundJobServer()
            .useJmxExtensions()
            .useDashboard()

        return config.initialize()

Expected behavior
An option to override or augment default mapping behavior

Additional context
There could be a way to do this in Spring (I haven't crawled through the config yet), but I'm using this option right now because the repository I'm working in didn't register any beans.

@rdehuyss rdehuyss changed the title [BUG] JobRunr Overrides Custom Serializer [Feature] Allow Custom Serializer for jobs Oct 14, 2021
@rdehuyss
Copy link
Contributor

This was done on purpose as there are specific requirements for the client. Providing a custom serializer increases the risk to break the dashboard a lot.

@maxwellclarke-wf
Copy link
Author

Would it be worth exposing an addModule delegate for jackson? That way the ObjectMapper remains configured in its appropriate state, but custom rules can be added per-type. Looking at the code, there's some fairly common types used by JobRunr, so that wouldn't have any guarantees.

Another option could be separating custom serialization rules provided by the application from serialization used by the the framework. I have no idea how much work that is though, I'm guessing a lot.

I suppose for now the workaround is to manually serialize your arguments with the application's preferred method, and pass in a string payload to be deserialized on the other side when the job runs.

@rdehuyss rdehuyss added the enhancement New feature or request label Oct 16, 2021
@rdehuyss rdehuyss added this to the 4.0.1 milestone Oct 22, 2021
@rdehuyss
Copy link
Contributor

Will be resolved in JobRunr 4.0.1 where you have full control over the JsonMapper.

@maxwellclarke-wf
Copy link
Author

Yay! Thank you!

@cebbens
Copy link

cebbens commented Apr 18, 2023

Will be resolved in JobRunr 4.0.1 where you have full control over the JsonMapper.

Hey @rdehuyss. I wonder if it was in fact resolved in v4.0.1. I'm using v6.1.3 (the latest) and I still experience the issue as @maxwellclarke-wf described.

If it was in fact resolved, how do we set a JobMapper with our own custom ObjectMapper? In our case, we have an ObjectMapper with specific needs for de/serializing currency types which will error if doing it with JobRunr's own ObjectMapper.

@rdehuyss
Copy link
Contributor

Hi @cebbens - how are you using JobRunr? Standalone, with Spring, ... ?

@cebbens
Copy link

cebbens commented Apr 21, 2023

Hi @cebbens - how are you using JobRunr? Standalone, with Spring, ... ?

Hey @rdehuyss We're using it within a Dropwizard/Guice/Kotlin/Gradle app.

We have a Guice module that creates the JobScheduler:

image

@rdehuyss
Copy link
Contributor

What about the useJsonMapper() function? Have you tried it?

@rdehuyss
Copy link
Contributor

Hi @cebbens - did you try the useJsonMapper(...) functionality?

ObjectMapper objectMapper = new ObjectMapper(); // your configuration can happen here
JobRunr.configure()
                .useJsonMapper(new JacksonJsonMapper(objectMapper))
                .useJobActivator(jobActivator)
                .useStorageProvider(storageProvider)
                .useBackgroundJobServer(usingStandardBackgroundJobServerConfiguration().andPollIntervalInSeconds(5))
                .initialize();

@cebbens
Copy link

cebbens commented Apr 26, 2023

Hi @cebbens - did you try the useJsonMapper(...) functionality?

ObjectMapper objectMapper = new ObjectMapper(); // your configuration can happen here
JobRunr.configure()
                .useJsonMapper(new JacksonJsonMapper(objectMapper))
                .useJobActivator(jobActivator)
                .useStorageProvider(storageProvider)
                .useBackgroundJobServer(usingStandardBackgroundJobServerConfiguration().andPollIntervalInSeconds(5))
                .initialize();

Hi @rdehuyss Yes, we initially had that config and thought that it was there for precisely re-using our own ObjectMapper, but we kept getting serialization errors. So, useJsonMapper is precisely for that use case?

We had some other problems at the same time, so maybe the useJsonMapper wasn't the root cause, but not sure. @MikeyBower93, do you recall if this was the root cause when we debugged it?

I'll try it again whenever I find the time and let you know how it went, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants