Declare getNewInstance as final in favor of alterNewInstance

[VincentLanglet]

Subject

I am targeting this branch, because BC.

There is an alterNewInstance for extensions, but not for the AbstractAdmin.
I think we should provide one instead of allowing to override getNewInstance.

Changelog

### Added
- Added `AbstractAdmin::alterNewInstance()`

### Deprecated
- Overriding `AbstractAdmin::getNewInstance()`

[franmomu]

IMHO getNewInstance should not be final, if someone uses a factory, a service or just has some mandatory parameters, should be able to override it.

[VincentLanglet]

IMHO getNewInstance should not be final, if someone uses a factory, a service or just has some mandatory parameters, should be able to override it.

He should override ModelManager::getModelInstance() then.

When the AbstractAdmin is calling an external service the best practice is to override the service and not the AbstractAdmin method. We promote this best practice by declaring the AbstractAdmin method as final.

[franmomu]

Well, in this specific case, we are not promoting best practices with ModelManager::getModelInstance() since we "force" the user to create their objects empty (not usable with mandatory parameters in the constructor), but that's another story.

I was thinking that maybe there is no need for ModelManager::getModelInstance() since creating the object it's not really related to the persistence layer and the code of both persistence bundles is the same.

[VincentLanglet]

Since our getNewInstance has some logic, I don't like the fact we're allowing to override it.

If you're doing

public function getNewInstance() {
   $object = $this->factory->create();
   
   return $object;
}

You'll losing the extension feature, the appendParentObject call and can have some bug.
I don't like the fact you have to know all the logic inside the method and copy it

public function getNewInstance() {
   $object = $this->factory->create();

   $this->appendParentObject($object);
   $this->alterNewInstance($object);

   foreach ($this->getExtensions() as $extension) {
       $extension->alterNewInstance($this, $object);
   }
   
   return $object;
}

Let's say one day we modify the getNewInstance to add/remove some code, in order to add a feature or remove a bug. You won't benefit of the feature/bugfix. It could even lead to a buggy behavior in your project.

What do you think about providing a protected createNewInstance method ?
We could add the logic of ModelManager::getModelInstance and deprecate the method in the persistence bundle.

[franmomu]

Yeah, I meant if there was no need of ModelManager::getModelInstance, something else should replace it, I'll see if I can do it at the weekend.