Skip to content

HHH-18462 Provide alternatives to deprecated org.hibernate.engine.spiMapping #8843

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

HHH-18462 Provide alternatives to deprecated org.hibernate.engine.spiMapping #8843

wants to merge 1 commit into from

Conversation

@dreab8
Copy link
Member

@dreab8 dreab8 commented Aug 29, 2024

https://hibernate.atlassian.net/browse/HHH-18462

see comments

[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.

github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 29, 2024
View reviewed changes
hibernate-core/src/main/java/org/hibernate/engine/spi/SessionFactoryImplementor.java
}

default Type getReferencedPropertyType(String className, String propertyName) throws MappingException{
return getMappingMetamodel().getEntityDescriptor( className ).getPropertyType( propertyName );

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [EntityPersister.getPropertyType](1) should be avoided because it has been deprecated.
hibernate-core/src/main/java/org/hibernate/mapping/Value.java
if ( subtype instanceof EntityType ) {
final EntityType entityType = (EntityType) subtype;
final Type idType = getIdType( entityType );
columnSpan = idType.getColumnSpan( mapping );

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [Type.getColumnSpan](1) should be avoided because it has been deprecated.
hibernate-core/src/main/java/org/hibernate/mapping/Value.java
columnSpan = idType.getColumnSpan( mapping );
}
else {
columnSpan = subtype.getColumnSpan( mapping );

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [Type.getColumnSpan](1) should be avoided because it has been deprecated.
hibernate-core/src/main/java/org/hibernate/metamodel/RuntimeMetamodels.java
}

default Type getReferencedPropertyType(String className, String propertyName) throws MappingException {
return getMappingMetamodel().getEntityDescriptor( className ).getPropertyType( propertyName );

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [EntityPersister.getPropertyType](1) should be avoided because it has been deprecated.
hibernate-core/src/main/java/org/hibernate/type/AnyType.java
public int[] getSqlTypeCodes(Mapping mapping) throws MappingException {
return join( discriminatorType.getSqlTypeCodes( mapping ), identifierType.getSqlTypeCodes( mapping ) );
return join(
discriminatorType.getSqlTypeCodes( mapping ),

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [Type.getSqlTypeCodes](1) should be avoided because it has been deprecated.
hibernate-core/src/main/java/org/hibernate/type/AnyType.java
return join( discriminatorType.getSqlTypeCodes( mapping ), identifierType.getSqlTypeCodes( mapping ) );
return join(
discriminatorType.getSqlTypeCodes( mapping ),
identifierType.getSqlTypeCodes( mapping )

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation

Invoking [Type.getSqlTypeCodes](1) should be avoided because it has been deprecated.
beikov
beikov approved these changes Sep 12, 2024
View reviewed changes
Copy link
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the approach. This cleans up a lot which is IMO a good thing to do for 7.0.

@gavinking
Copy link
Member

gavinking commented Sep 12, 2024

This is fine, but @dreab8 I wonder if it might be better to pass the Metadata instance to methods that currently accept Mapping, instead of the TypeConfiguration. That would be sort of a "smaller" change, in that Metadata is currently a subtype of Mapping, and it avoids adding these new operations to TypeConfiguration, where I don't think they really belong.

TypeConfiguration could have a getMetadata() method if necessary, but I guess it's probably not needed.

WDYT?

@gavinking
Copy link
Member

gavinking commented Sep 12, 2024

(Note that Metadata is implemented by both InflightMetadataCollectorImpl and MetadataImpl, so it's available at both boot and at runtime.)

@dreab8
Copy link
Member Author

dreab8 commented Sep 12, 2024

@gavinking I may be wrong but i don't think it is possible, most of the time we need these information at runtime where we have no more access to the Metadata info

@gavinking
Copy link
Member

gavinking commented Sep 12, 2024

I may be wrong

Nono, you're right and I'm wrong.

Hrrm....

@gavinking
Copy link
Member

gavinking commented Sep 12, 2024

@dreab8 Alright, so how about this then:

  1. we simply move the Mapping interface (perhaps renaming it MappingContext or whatever) into org.hibernate.type and undeprecate it,
  2. if you guys think it's ugly that Metadata extends Mapping, no problem, it doesn't need to—any client can instantiate a Mapping that delegates to the Metadata instance (it's only three methods).

The reason I suggest this design is:

  • the only reason Mapping really exists is to let Types determine information about foreign keys, but
  • those operations don't really seem to fit in with the other operations of TypeConfiguration, and the operations of Type which need a Mapping don't seem to have any use for them.

@beikov
Copy link
Member

beikov commented Sep 13, 2024

I'd be curious if we still need access to the mapping information once we get rid of using Type (except BasicType) in the runtime model. Maybe all of these methods can go away then?

@gavinking
Copy link
Member

gavinking commented Sep 13, 2024

@beikov most likely, yes

@dreab8
Copy link
Member Author

dreab8 commented Sep 14, 2024

@gavinking your suggestion is to move and rename Mapping to org.hibernate.type.MappingContext and having Metadata and SessionFactoryImplementor extending org.hibernate.type.MappingContext, am I right?

@gavinking
Copy link
Member

gavinking commented Sep 14, 2024

@gavinking your suggestion is to move and rename Mapping to org.hibernate.type.MappingContext

Yeah.

and having Metadata and SessionFactoryImplementor extending org.hibernate.type.MappingContext, am I right?

That's the easiest thing, but if you don't like that option, I guess you should be able to easily change it so that it's MetadataImplementor which implements MappingContext, or some other little object e.g. an anonymous class of whatever.

@dreab8 dreab8 mentioned this pull request Sep 16, 2024
Merged
@sebersole
Copy link
Member

sebersole commented Sep 16, 2024

Closed in favor of #8959

@sebersole sebersole closed this Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beikov beikov beikov approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dreab8 @gavinking @beikov @sebersole