Fluent builder api #1004 #1014

gitmotte · 2020-07-09T20:48:52Z

except for the create methods all changes are committed.

chaining - methods returning "this" white_check_mark But leave the original ones intact. In fact setters should not return anything, so I would prefer the with way.
overwrite chaining - methods of abstract parents/interfaces for returning concrete type white_check_mark
add methods on collection-fields with varargs-parameter white_check_mark
add public T get(Class) - casting and returning an inner interface-type

Further the chaining api is used

within the parser (where reasonable)
within some testcases by manually creating the model, comparing the parsed model against the created one, comparing the deparsed created model with the statement which is parsed
within ReflectionModelTest to check consistency of model (added some empty constructors to the model if fields are not final)

* create(...) methods * chaining - methods returning "this" * overwrite chaining - methods of abstract parents/interfaces for returning concrete type * add<Name> methods on collection-fields with varargs-parameter * add public T get<Name>(Class<T>) - casting and returning an inner interface-type

…r.git into master.fluent

* add<Name> methods on collection-fields with collection-parameter JSQLParser#1004

…r.git into master.fluent

* add<Name> methods on collection-fields with varargs-parameter * add<Name> methods on collection-fields with collection-parameter JSQLParser#1004

type JSQLParser#1004

type (swap Class<? extends E> for Class<E>) JSQLParser#1004

returning concrete type JSQLParser#1004

…r.git into master.fluent

JSQLParser#1004

not break current api) JSQLParser#1004

https://github.com/gitmotte/JSqlParser.git into master.fluent

* add some constructors * fix and add "with" / "add" methods

* add some constructors

AnEmortalKid · 2020-07-15T03:06:41Z

src/main/java/net/sf/jsqlparser/statement/create/function/CreateFunction.java

+    }
+
+    @Override()
+    public CreateFunction addFunctionDeclarationParts(String... functionDeclarationParts) {


if we're just using super does it make sense to override? Ah I guess this might be because we want it to return the specific type and not AnythingThat extends CreateFunctionStatement

Yes, because it will be casted to CreateFunction, the parent returns CreateFunctionalStatement - the effect is, that the order of chaining does not matter, otherwise it would matter and the methods returning the parent or the super-class of the parent needs to be called at last in a chaining-context.

AnEmortalKid · 2020-07-15T03:09:04Z

src/test/java/net/sf/jsqlparser/util/deparser/ExecuteDeParserTest.java

+        expressions.add(new JdbcParameter());
+
+        execute.withName(name)
+        .withExecType(EXEC_TYPE.EXEC).withParenthesis(true)


super nit we probably should have kept EXEC_TYPE.EXECUTE just so the assertion didn't change?

wumpz

That is a huge pull request. +1
You should not change existing tests. For instance you replaced the "old" variant of getting some parameter with the new, but both should be tested.

wumpz · 2020-07-15T05:27:27Z

src/main/java/net/sf/jsqlparser/schema/Sequence.java

-        }
-    }
-}
+/*


Why is this file completely replaced? Are there some kind of linefeed differences?

324ccf1
tried to replace formatchanges, that compare finds matching lines

0d434b8
seems that the original commit had different linefeeds than the other files in this project

I',m sorry, did't work for this diff-view

I think that was me. I work on a Mac and on a windows device from time to time. I’ll double check that we have a gitatttrbutes setup correctly to avoid these issues!

Unfortunately only the latest versions of jgit, if you are using this, are taking gitattributes into account. But that explains this. If the file was wrong, now it would be corrected. So all fine.

AH, I use intellij, so i bet my Windows gitbash auto-core config crap wasn't setup right.

wumpz · 2020-07-15T05:29:56Z

src/main/java/net/sf/jsqlparser/statement/create/function/CreateFunction.java

+        super("FUNCTION", functionDeclarationParts);
+    }
+
+    @Override()


Could you remove this brackets on a simple override?

ok, 9865783

wumpz · 2020-07-15T05:34:49Z

src/test/java/net/sf/jsqlparser/statement/alter/AlterTest.java

    @Test
    public void testAlterOnlyIssue928() throws JSQLParserException {
-        assertSqlCanBeParsedAndDeparsed("ALTER TABLE ONLY categories ADD CONSTRAINT pk_categories PRIMARY KEY (category_id)");
+        String statement = "ALTER TABLE ONLY categories ADD CONSTRAINT pk_categories PRIMARY KEY (category_id)";


Don't replace existing tests. Create new tests.

normally I would agree to the principle of not changing tests, but didn't change the assert, but extending it with additional asserts.
doesn't it make sense to check parsing/deparsing, creation of the model and comparing it against the parsed model all together?

But for instance you changed on various places the getExpression() to getExpression(Classtype).

For the rest you wrote I agree.

getExpression(Classtype) calls getExpression() inside. I changed it if explicit casting was done within testcase.

wumpz · 2020-07-15T05:36:12Z

src/test/java/net/sf/jsqlparser/statement/select/SelectTest.java

@@ -52,6 +89,13 @@ public void testMultiPartTableNameWithServerNameAndDatabaseNameAndSchemaName() t
        final String statement = "SELECT columnName FROM [server-name\\server-instance].databaseName.schemaName.tableName";
        assertSqlCanBeParsedAndDeparsed(statement, false,
                parser -> parser.withSquareBracketQuotation(true));
+        assertDeparse(new Select().withSelectBody(new PlainSelect()


Don't change existing tests in this way. Create new tests.

Please have a look to the comment above at AlterTest.java

wumpz · 2020-07-15T05:41:14Z

src/test/java/net/sf/jsqlparser/util/deparser/ExecuteDeParserTest.java


        executeDeParser.deParse(execute);

        String actual = buffer.toString();
-        assertTrue(actual.matches("EXECUTE " + name + " .*?, .*"));
+        assertEquals("EXEC " + name + " (?, ?)", actual);


Here the outcome is suddenly EXEC not EXECUTE. Why?

reverted it to be EXEC - see #1014 (comment) too

…r.git into master.fluent

* fix RandomUtils to support enums * fix RandomUtils to map objects by its interfaces and super-classes * filter method "setASTNode" - do not test setters (cannot randomly create a SimpleNode)

JSQLParser#1014 (comment)

wumpz

So far it looks good.

wumpz · 2020-07-15T21:14:27Z

As always I merge using squash and merge for multi commit pull requests. If you want to squash it yourself, you are free to do so. One commit pull requests are merged normally.

gitmotte · 2020-07-15T21:39:44Z

No objections if you want to sqash into one commit. The summary in the first comment of this merge request would be an appropriate comment for this commit.
Thanks in advance!

gitmotte · 2020-07-17T12:18:46Z

No objections if you want to sqash into one commit. The summary in the first comment of this merge request would be an appropriate comment for this commit.
Thanks in advance!

@wumpz it's ready to merge!

wumpz · 2020-07-20T15:23:09Z

Sorry. I am out of business for some days. Will merge if I am back

wumpz · 2020-08-09T21:39:26Z

So back on business. After I merged some other PRs you have some conflicts here. Please resolve those first. After this, I will merge. This PR was so far mergeable, right? Or do you want to add anything more?

gitmotte · 2020-08-10T19:26:09Z

ok, i'v resolved the conflicts, it's ready to merge again. thanks in advance.

gitmotte · 2020-08-17T18:11:37Z

@wumpz

wumpz · 2020-08-23T20:08:45Z

I am sorry, it took a while. I will change the version number to reflect this large change.

@OverRide

* JSQLParser#1004 * create(...) methods * chaining - methods returning "this" * overwrite chaining - methods of abstract parents/interfaces for returning concrete type * add<Name> methods on collection-fields with varargs-parameter * add public T get<Name>(Class<T>) - casting and returning an inner interface-type * 1004 add chaining - methods returning "this" * JSQLParser#1004 add chaining - methods returning "this" * * add<Name> methods on collection-fields with varargs-parameter * add<Name> methods on collection-fields with collection-parameter JSQLParser#1004 * * add chaining - methods returning "this" * add<Name> methods on collection-fields with varargs-parameter * add<Name> methods on collection-fields with collection-parameter JSQLParser#1004 * * add public T get<Name>(Class<T>) - casting and returning the concrete type JSQLParser#1004 * * add public T get<Name>(Class<T>) - casting and returning the concrete type (swap Class<? extends E> for Class<E>) JSQLParser#1004 * * overwrite chaining - methods of abstract parents/interfaces for returning concrete type JSQLParser#1004 * * add with prefix for fluent setters. JSQLParser#1004 * add getters * * add with prefix for fluent setters. (revert to chaining setters, do not break current api) JSQLParser#1004 * * add with prefix for fluent setters. (revert to chaining setters, do not break current api) JSQLParser#1004 * use new methods within testcases * use new methods within testcases * use new methods within testcases * use new methods within testcases * use new methods within testcases * use new methods within testcases * use new methods within testcases * use new methods within testcases * remove create() methods - they do not add enough value to be justified * * use new methods within testcases * add some constructors * fix and add "with" / "add" methods * * use new methods within testcases * * use new methods within testcases * add some constructors * * renamed constant * use new methods within testcases * use new methods within testcases * use new methods within testcases * use new methods within testcases * * use new methods within testcases * add some with-methods * add getter/setter named after the field without abbrivation * * use new methods within testcases * remove empty implicit constructor * return the deparsed Statement - object * compare object tree * compare object tree * * fix ObjectTreeToStringStyle * compare object tree * remove casts not needed * * use new methods within testcases * add some "set" "with" "add" methods missing * * use new methods within testcases * add empty constructors and override with-/add-methods returning concrete type * * add ReflectionModelTest * * use new methods within testcases * fix checkstyle errors * license header * remove test-classes from ReflectionModelTest * remove visitoradapter-classes from ReflectionModelTest * remove duplicate import declaration (checkstyle error) * * fix RandomUtils to support used java.sql.* types * fix RandomUtils to support enums * fix RandomUtils to map objects by its interfaces and super-classes * filter method "setASTNode" - do not test setters (cannot randomly create a SimpleNode) * add javadoc, stating that this is a marker interface JSQLParser#1014 (comment) * revert formatting change JSQLParser#1014 (comment) * change to EXEC_TYPE.EXECUTE just so the assertion didn't change JSQLParser#1014 (comment) * try to revert format changes JSQLParser#1014 (comment) * try to revert format changes JSQLParser#1014 (comment) * remove brackets on @OverRide() -> @OverRide * add with-methods to new fields

gitmotte added 30 commits June 29, 2020 14:24

1004 add chaining - methods returning "this"

8e71f9b

JSQLParser#1004 add chaining - methods returning "this"

aeb1502

Merge branch 'master.fluent' of https://github.com/gitmotte/JSqlParse…

274ef21

…r.git into master.fluent

f38084c

* add<Name> methods on collection-fields with varargs-parameter

a6ae42f

* add<Name> methods on collection-fields with collection-parameter JSQLParser#1004

Merge branch 'master.fluent' of https://github.com/gitmotte/JSqlParse…

88a5e4f

…r.git into master.fluent

* add chaining - methods returning "this"

f0b06e8

* add<Name> methods on collection-fields with varargs-parameter * add<Name> methods on collection-fields with collection-parameter JSQLParser#1004

* add public T get<Name>(Class<T>) - casting and returning the concrete

74a0eb6

type JSQLParser#1004

* add public T get<Name>(Class<T>) - casting and returning the concrete

8a53418

type (swap Class<? extends E> for Class<E>) JSQLParser#1004

* overwrite chaining - methods of abstract parents/interfaces for

812b6ab

returning concrete type JSQLParser#1004

Merge branch 'master.fluent' of https://github.com/gitmotte/JSqlParse…

4b3a1a4

…r.git into master.fluent

* add with prefix for fluent setters.

34db4f5

JSQLParser#1004

add getters

a2600c7

* add with prefix for fluent setters. (revert to chaining setters, do

bb2b15f

not break current api) JSQLParser#1004

* add with prefix for fluent setters. (revert to chaining setters, do

8b6fd6b

not break current api) JSQLParser#1004

use new methods within testcases

0880fde

use new methods within testcases

63679e0

use new methods within testcases

1d0a24d

use new methods within testcases

fda2f67

use new methods within testcases

8d2cde4

use new methods within testcases

1027829

use new methods within testcases

8de6c09

use new methods within testcases

a0cc175

Merge branch 'master.fluent' of

dd60c2e

https://github.com/gitmotte/JSqlParser.git into master.fluent

remove create() methods - they do not add enough value to be justified

ea4356a

* use new methods within testcases

8ece698

* add some constructors * fix and add "with" / "add" methods

* use new methods within testcases

77d92c0

* use new methods within testcases

0c99e8c

* add some constructors

* renamed constant

89fe5f4

AnEmortalKid reviewed Jul 15, 2020

View reviewed changes

wumpz reviewed Jul 15, 2020

View reviewed changes

gitmotte added 9 commits July 15, 2020 10:54

Merge branch 'master.fluent' of https://github.com/gitmotte/JSqlParse…

73b338b

…r.git into master.fluent

remove duplicate import declaration (checkstyle error)

d8afd17

* fix RandomUtils to support used java.sql.* types

4f2443c

* fix RandomUtils to support enums * fix RandomUtils to map objects by its interfaces and super-classes * filter method "setASTNode" - do not test setters (cannot randomly create a SimpleNode)

add javadoc, stating that this is a marker interface

9a9422f

JSQLParser#1014 (comment)

revert formatting change

db3b4ae

JSQLParser#1014 (comment)

change to EXEC_TYPE.EXECUTE just so the assertion didn't change

d77f3af

JSQLParser#1014 (comment)

try to revert format changes

324ccf1

JSQLParser#1014 (comment)

try to revert format changes

0d434b8

JSQLParser#1014 (comment)

remove brackets on @OverRide() -> @OverRide

9865783

gitmotte requested a review from wumpz July 15, 2020 15:02

wumpz approved these changes Jul 15, 2020

View reviewed changes

AnEmortalKid approved these changes Jul 15, 2020

View reviewed changes

Merge branch 'master' into master.fluent

f7ec34e

add with-methods to new fields

f3d113d

wumpz merged commit 6cff161 into JSQLParser:master Aug 23, 2020

gitmotte deleted the master.fluent branch August 26, 2020 08:24

gitmotte mentioned this pull request Sep 23, 2020

Addons/fixes for Fluent API #1049

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fluent builder api #1004 #1014

Fluent builder api #1004 #1014

gitmotte commented Jul 9, 2020

AnEmortalKid Jul 15, 2020 •

edited

Loading

gitmotte Jul 15, 2020 •

edited

Loading

AnEmortalKid Jul 15, 2020

gitmotte Jul 15, 2020

wumpz left a comment

wumpz Jul 15, 2020

gitmotte Jul 15, 2020

gitmotte Jul 15, 2020

gitmotte Jul 15, 2020

AnEmortalKid Jul 15, 2020

wumpz Jul 15, 2020

AnEmortalKid Jul 15, 2020

wumpz Jul 15, 2020

gitmotte Jul 15, 2020

wumpz Jul 15, 2020

gitmotte Jul 15, 2020 •

edited

Loading

wumpz Jul 15, 2020

wumpz Jul 15, 2020

gitmotte Jul 15, 2020

wumpz Jul 15, 2020

gitmotte Jul 15, 2020 •

edited

Loading

wumpz Jul 15, 2020

gitmotte Jul 15, 2020

wumpz left a comment

wumpz commented Jul 15, 2020 •

edited

Loading

gitmotte commented Jul 15, 2020 •

edited

Loading

gitmotte commented Jul 17, 2020

wumpz commented Jul 20, 2020

wumpz commented Aug 9, 2020

gitmotte commented Aug 10, 2020

gitmotte commented Aug 17, 2020

wumpz commented Aug 23, 2020

Fluent builder api #1004 #1014

Fluent builder api #1004 #1014

Conversation

gitmotte commented Jul 9, 2020

AnEmortalKid Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

gitmotte Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wumpz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitmotte Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitmotte Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wumpz left a comment

Choose a reason for hiding this comment

wumpz commented Jul 15, 2020 • edited Loading

gitmotte commented Jul 15, 2020 • edited Loading

gitmotte commented Jul 17, 2020

wumpz commented Jul 20, 2020

wumpz commented Aug 9, 2020

gitmotte commented Aug 10, 2020

gitmotte commented Aug 17, 2020

wumpz commented Aug 23, 2020

AnEmortalKid Jul 15, 2020 •

edited

Loading

gitmotte Jul 15, 2020 •

edited

Loading

gitmotte Jul 15, 2020 •

edited

Loading

gitmotte Jul 15, 2020 •

edited

Loading

wumpz commented Jul 15, 2020 •

edited

Loading

gitmotte commented Jul 15, 2020 •

edited

Loading