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

Maven plugin : Cross-schema foreign keys support #621

Closed
nithril opened this Issue Jan 10, 2014 · 16 comments

Comments

Projects
None yet
2 participants
@nithril
Contributor

nithril commented Jan 10, 2014

Hello,

QueryDSL maven plugin does not support cross schemas, cross projects foreign key in different package.

I have two Oracle schema in two dedicated maven project (not module) with two dedicated package. A common schema (shared between projects) and an Oracle schema for my project (in fact per project). My project schema references the common schema (with synonym).

QueryDSL generates the classes for the common schema. But for the project schema, it's failing at compilation because the generated classes references the common classes and the java import is not set accordingly. With exportForeignKeys set to false the compilation succeed.

@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 10, 2014

Contributor

If this improvment involves only an imports tag I could give it a try if you agree

<imports>
   <import>com.....</import>
</imports>
Contributor

nithril commented Jan 10, 2014

If this improvment involves only an imports tag I could give it a try if you agree

<imports>
   <import>com.....</import>
</imports>
@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jan 10, 2014

Member

Yes, the imports addition would be good.

Member

timowest commented Jan 10, 2014

Yes, the imports addition would be good.

@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 10, 2014

Contributor

As querydsl can export entity and bean class with two different package I think I mustl add two entries:

<imports>
   <import>com.....</import>
</imports>

<beanImports>
   <beanImport>com.....</import>
</beanImports>

Is it correct?

Contributor

nithril commented Jan 10, 2014

As querydsl can export entity and bean class with two different package I think I mustl add two entries:

<imports>
   <import>com.....</import>
</imports>

<beanImports>
   <beanImport>com.....</import>
</beanImports>

Is it correct?

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jan 10, 2014

Member

You can start with imports. I think beanImports is not needed, since the foreign keys are not in the beans, only in the Q-types.

Member

timowest commented Jan 10, 2014

You can start with imports. I think beanImports is not needed, since the foreign keys are not in the beans, only in the Q-types.

@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 11, 2014

Contributor

Prior to pull request could you make a quick review of the diff?

Contributor

nithril commented Jan 11, 2014

Prior to pull request could you make a quick review of the diff?

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jan 11, 2014

Member

Could you make the changes SQL specific?

The changes to CodegenModule and AbstractMetaDataExportMojo are ok, but the custom imports could be injected into MetaDataSerializer, since they are general and not EntityType specific.

Member

timowest commented Jan 11, 2014

Could you make the changes SQL specific?

The changes to CodegenModule and AbstractMetaDataExportMojo are ok, but the custom imports could be injected into MetaDataSerializer, since they are general and not EntityType specific.

@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 12, 2014

Contributor

By "the custom imports could be injected" you mean "only the EntitySerializer.writeUserImports method" or "the EntitySerializer.writeUserImports method and the EntityType.imports property".

On my side, first proposition makes more sense. Serialization logic separates from EntityType specific imports

Contributor

nithril commented Jan 12, 2014

By "the custom imports could be injected" you mean "only the EntitySerializer.writeUserImports method" or "the EntitySerializer.writeUserImports method and the EntityType.imports property".

On my side, first proposition makes more sense. Serialization logic separates from EntityType specific imports

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jan 12, 2014

Member

By "the custom imports could be injected" you mean "only the EntitySerializer.writeUserImports method" or "the EntitySerializer.writeUserImports method and the EntityType.imports property".

I mean to inject the userImports into the constructor of MetaDataSerializer. No changes to EntityType are needed here.

Member

timowest commented Jan 12, 2014

By "the custom imports could be injected" you mean "only the EntitySerializer.writeUserImports method" or "the EntitySerializer.writeUserImports method and the EntityType.imports property".

I mean to inject the userImports into the constructor of MetaDataSerializer. No changes to EntityType are needed here.

@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 12, 2014

Contributor

Could you have a look? Thanks!

Contributor

nithril commented Jan 12, 2014

Could you have a look? Thanks!

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jan 12, 2014

Member

Looks good. Thank you very much.

Member

timowest commented Jan 12, 2014

Looks good. Thank you very much.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jan 12, 2014

Member

Could you also update the reference docs?

Member

timowest commented Jan 12, 2014

Could you also update the reference docs?

@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 12, 2014

Contributor

I have updated the docs, could you have a look?

Contributor

nithril commented Jan 12, 2014

I have updated the docs, could you have a look?

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jan 12, 2014

Member

Looks ok. Sorry, might have missed it.

Member

timowest commented Jan 12, 2014

Looks ok. Sorry, might have missed it.

@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 12, 2014

Contributor

Timestamps are wrong. I should have an issue with my git squash

Contributor

nithril commented Jan 12, 2014

Timestamps are wrong. I should have an issue with my git squash

timowest added a commit that referenced this issue Jan 12, 2014

Merge pull request #625 from nithril/master
#621 Maven plugin : Cross-schema foreign keys support
@nithril

This comment has been minimized.

Show comment
Hide comment
@nithril

nithril Jan 12, 2014

Contributor

Thanks for the merge.

Contributor

nithril commented Jan 12, 2014

Thanks for the merge.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Feb 8, 2014

Member

Released in 3.3.1

Member

timowest commented Feb 8, 2014

Released in 3.3.1

@timowest timowest closed this Feb 8, 2014

@timowest timowest added this to the 3.3.1 milestone Apr 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment