Skip to content
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

Generate additional setters for foreign keys, accepting records as arguments #1510

Closed
lukaseder opened this Issue Jul 22, 2012 · 9 comments

Comments

Projects
None yet
1 participant
@lukaseder
Copy link
Member

lukaseder commented Jul 22, 2012

@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

Author: @digulla
Some sensible defaults for the getter/setter name:

  1. If it's a single-column FK and the FK ends with "_ID", the setter/getter name should be the FK name without "_ID"
  2. It it's a multi-column FK, the setter/getter name should be the remote table name
  3. If there are several multi-column FKs to the same table, use the name of the first FK column (the user will have to give the columns different names).
  4. Expose these strategies via the API and allow developers to override them.
@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

Author: @digulla
For reference, here is my code generator extension for this:

    @Override
    protected void printFetchMethod( GenerationWriter out, ColumnDefinition column, ForeignKeyDefinition foreignKey, TableDefinition referenced ) {

        printFKSetter( out, column, foreignKey, referenced );

        super.printFetchMethod( out, column, foreignKey, referenced );
    }

    private void printFKSetter( GenerationWriter out, ColumnDefinition column, ForeignKeyDefinition foreignKey, TableDefinition referenced ) {

        if( foreignKey.getReferencedColumns().size() != 1 ) {
            out.print( "\t// Unable to generate FK-setter because this is a multi-column FK" );
        }

        out.println("");
        out.println("\t/**");
        out.print("\t *  Create a connection between this instance and a {@link ");
        out.print( getStrategy().getFullJavaClassName(referenced, Mode.RECORD) );
        out.print( " " );
        out.print( getStrategy().getJavaClassName(referenced, Mode.RECORD) );
        out.println( "}" );
        out.println( "\t */" );

        out.print( "\tpublic void " );
        out.print( getStrategy().getJavaSetterName(column, Mode.DEFAULT) );
        out.print( "(" );
        out.print( getStrategy().getFullJavaClassName(referenced, Mode.RECORD) );
        out.print(" value)");
        out.println("\t{");
        out.print("\t\t");
        out.print( getStrategy().getJavaSetterName(column, Mode.DEFAULT) );
        out.print("(value.");
        out.print( getStrategy().getJavaGetterName( foreignKey.getReferencedColumns().get( 0 ) ) );
        out.println("());");
        out.println("\t}");

    }
@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

Thanks for the contribution. Nice thinking, relating this to the generation of fetch methods. In a way, this can be seen as "foreign key navigation", indeed.

I will fix some issues and commit this for the upcoming release. The issues are:

  • Fixed usage of Mode.DEFAULT where it should be Mode.RECORD
  • Made code safe for multi-column foreign keys (avoiding generation, though, as ambiguity is not yet fully resolved)
  • Handled potential type mismatch between PK / FK (e.g. PK is Integer whereas FK is Short)
  • Handled null arguments
@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

Implemented this for single-column foreign keys.

  • The _ID suffix is not omitted by default, as that would require an extension to the strategy (such an extension could be added later on popular request)
  • Multi-column foreign keys are not yet supported
@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

Remaining features (multi-column FK's): #1539

@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

Author: @digulla
I'm not happy with keeping the _ID suffix; reading

setUserId(user);

might make sense from a jOOQ point of view but it exposes details about the implementation (that you use the ID columns).

setUser(user);

says "connect those two instances" but not how. Would it really be that hard to add to the existing strategy?

@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

I like this solution better for two reasons:

  • It communicates what it really does (setting the ID, not setting the whole object, like any Hibernate-experienced user might expect)
  • It avoids naming clashes, if a table X has two single-column foreign keys to another table Y

The latter is important, specifically if X has only one reference to Y at first, but then another reference gets added later. If the method name would be changed, then client code would break.

@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Jul 22, 2012

... short of reliable heuristics, I suggest that "more clever" naming patterns are left to client code, once this is properly supported in naming strategies.

@lukaseder

This comment has been minimized.

Copy link
Member Author

lukaseder commented Dec 22, 2012

This feature will be removed again in jOOQ 3.0. Name mangling schemes are pretty tough to handle...

See also #2137, where this feature here introduces regressions in core jOOQ behaviour, where record setters are already reserved for UDT records

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.