Skip to content

HHH-13604 Check APIs used after the SessionFactory has been created #3040

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

Closed
wants to merge 5 commits into from

Conversation

fax4ever
Copy link
Contributor

@fax4ever fax4ever commented Sep 17, 2019

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

I had to comment some code, since regex pattern compile is still used at runtime. not anymore!

At the very moment the rules are active only for CheckForbiddenAPIAtRuntimeTest run.
In order to apply the rules to all tests, please remove or comment:

// TODO HHH-13604 Temporary loading the rules only for CheckForbiddenAPIAtRuntimeTest.
if ( !"CheckForbiddenAPIAtRuntimeTest".equals( clazz.getSimpleName() ) ) {
  return;
}

of org.hibernate.testing.junit4.runtimecheck.BMRuntimeCheckCustomRunner class.

@fax4ever fax4ever requested a review from Sanne September 17, 2019 00:17
@fax4ever fax4ever force-pushed the HHH-13604 branch 2 times, most recently from 3e4d6ae to 4fac35b Compare October 9, 2019 07:01
@fax4ever
Copy link
Contributor Author

fax4ever commented Oct 9, 2019

It seems that skipping all the cases when the Pattern#regex is called from some class initialization method at any level, the test works.

@Sanne thanks for help you've provided so far,
now I'm going to try to apply the same rules to the whole test phase.At the moment I wouldn't know how to do it, but I can explore the case...

@Sanne
Copy link
Member

Sanne commented Oct 9, 2019

I'm going to try to apply the same rules to the whole test phase.At the moment I wouldn't know how to do it, but I can explore the case...

yes please! That's what we need.

@fax4ever fax4ever force-pushed the HHH-13604 branch 4 times, most recently from 735fe57 to 688f250 Compare October 15, 2019 14:53
@fax4ever
Copy link
Contributor Author

Hi @Sanne, it seems that our investment has borne some fruits.

In fact, activating the Byteman rules we can catch both some readings of annotations and some compations of new regular expressions after that the session factory has been created.

As a follow up, I created: HHH-13669

The rules are not active, in order to keep the tests from failing.
To active those, you can comment some code in BMRuntimeCheckCustomRunner class.

Meanwhile is it the case to merge this request and close the related issue? Thanks

@fax4ever fax4ever changed the title HHH-13604 Check regex pattern parse is not used at runtime HHH-13604 Check APIs used after the SessionFactory has been created Oct 18, 2019
if ( this == o ) {
return true;
}
Book book = (Book) o;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should check parameter's class before down casting

@Override
public int hashCode() {
return Objects.hash( id, isbn, title, author, copies );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that id field is not generated, it seems the above two methods should be based on it exclusively.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe using getter method is more appropriate for two reasons:

  • be consistent with equals() method above
  • proxy friendly

/**
* @author Fabio Massimo Ercoli
*/
@TestForIssue(jiraKey = "HHH-13604")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big deal, but using space chars right after '(' and before ')' would be consistent with existing coding format

readClass = Class.forName( "org.hibernate.session.runtime.check.Book" );
}
catch (ClassNotFoundException e) {
throw new RuntimeException( e );
Copy link
Contributor

Choose a reason for hiding this comment

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

You might well use throw new ExceptionInInitializerError( e );


doInHibernate( this::sessionFactory, session -> {
Book loaded = session.load( Book.class, 1 );
assertEquals( book, loaded );
Copy link
Contributor

Choose a reason for hiding this comment

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

Now I understand your implementation of equals for sake of testing purpose. We might well use Assert.assertTrue(EqualsBuilder.reflectionEquals(expected,actual)); to avoid providing a non-intuitive equals overridden method.

@fax4ever
Copy link
Contributor Author

I'm closing this pull request, I could reopen it if we need it.

@fax4ever fax4ever closed this Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants