Combining Repository Validating Interceptor and IG installments #210
Conversation
|
@tadgh If @jamesagnew is busy doing other stuff I would value your approval / comments on this piece |
|
Heya, I can read over this by EOD hopefully |
jvitrifork
changed the title
| @@ -97,6 +97,9 @@ | |||
| @Autowired | |||
| ApplicationContext myApplicationContext; | |||
|
|
|||
| @Autowired(required = false) | |||
| RepositoryValidationInterceptorFactoryR4 factory; | |||
|
|
|||
There was a problem hiding this comment.
Does it make sense for the R4 factory to be in BaseJpaREstfulServer? Seems like those repositoryValidationInterceptorFactorys could inherit some interface, and you can autowire the interface.
There was a problem hiding this comment.
Agree - all the leg work had been done so I've introduced the interface
| .setName(guide.getValue().getName()) | ||
| .setVersion(guide.getValue().getVersion()) | ||
| .setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL)); | ||
| PackageInstallOutcomeJson outcome = packageInstallerSvc.install(new PackageInstallationSpec() |
There was a problem hiding this comment.
Doesn't seem like this variable assignment is necessary, though I may just be missing its usage.
There was a problem hiding this comment.
Was used in a debug session. It shouldn't have survived
| } | ||
| } | ||
|
|
||
| if(factory != null) | ||
| interceptorService.registerInterceptor(factory.buildUsingStoredStructureDefinitions()); |
There was a problem hiding this comment.
Its just a style thing, but would appreciate braces around the if, even though its only one line.
|
@tadgh any chance you could get @jamesagnew to read my last message I left him at zulip? |
@jamesagnew I would like your comments on this way of wiring the interceptors. Is different from the current way of doing it but it is logically the same. I would say it is a bit more type safe and stream lined, but I will only continue doing so if you're ok with it