HHH-18462 Provide alternatives to deprecated org.hibernate.engine.spi.Mapping #8959
Conversation
| Type getIdentifierType(String className) throws MappingException; | ||
| String getIdentifierPropertyName(String className) throws MappingException; | ||
| Type getReferencedPropertyType(String className, String propertyName) throws MappingException; |
There was a problem hiding this comment.
I think we can just delete this methods and leave the interface empty, right?
There was a problem hiding this comment.
This would unfortunately be a binary incompatible change, though maybe that is ok since this is an SPI?
There was a problem hiding this comment.
I don't believe we guarantee binary compatibility between major versions.
| * License: GNU Lesser General Public License (LGPL), version 2.1 or later. | ||
| * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>. | ||
| */ | ||
| package org.hibernate.type.spi; |
There was a problem hiding this comment.
Should it maybe just go into org.hibernate.type, since that package uses it?
There was a problem hiding this comment.
What I mean is: with this design, we're leaking an SPI type onto what is in principle still an API.
Now, yeah, I realize that the stuff in org.hibernate.type has been gradually reinterpreted as SPI over the years, but even if it is now SPI, it's still OK for MappingContext to live right there alongside its main usages.
dreab8
force-pushed
the
HHH-18462_mappingcontext
branch
from
b64a7a1 to
2f72f51
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hibernate-core/src/main/java/org/hibernate/type/SpecialOneToOneType.java
Fixed
Show fixed
Hide fixed
dreab8
force-pushed
the
HHH-18462_mappingcontext
branch
from
2f72f51 to
504e059
Compare
|
What is the reason for keeping |
Because there has to be some place those operations are declared, and it has to be available at both boot time and at runtime. And AFAICT there's no other existing interface that makes sense. @dreab8 suggested putting them on |
|
I think you are thinking of @Deprecated(since = "6.0")
public interface Mapping extends MappingContext {
}If this really was deprecated since 6.0 we could just remove it. What's the argument for keeping it? |
|
Oh, sorry, my bad, yes, I misread. |
|
@sebersole i kept it based on your comments on my first PR #8769 |
Sure. Just questioning my suggestion :) I guess it is nicer to leave it for the time being for people upgrading. |
4 participants
https://hibernate.atlassian.net/browse/HHH-18462
this is an alternative to #8843
[Please describe here what your change is about]
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.