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

Added generator for Table class for StorIOSQLiteProcessor. (Issue #694) #734

Closed
wants to merge 5 commits into from
Closed

Added generator for Table class for StorIOSQLiteProcessor. (Issue #694) #734

wants to merge 5 commits into from

Conversation

pbochenski
Copy link

Since all data for creating such class is already in annotation processors META classes it might save some boilerplate to create such class automatically.
It generates SQL code for creating table, for additive update (only adds columns) and stores table name and column names in easy to use way.

Since all data for creating such class is already in annotation processors META classes it might  save some boilerplate to create such class automatically.
It generates SQL code for creating table, for additive update (only adds columns) and stores table name and column names in easy to use way.
@codecov-io
Copy link

codecov-io commented Dec 12, 2016

Codecov Report

Merging #734 into master will not change coverage.

@@           Coverage Diff           @@
##           master     #734   +/-   ##
=======================================
  Coverage   96.05%   96.05%           
=======================================
  Files          87       87           
  Lines        2459     2459           
  Branches      329      329           
=======================================
  Hits         2362     2362           
  Misses         68       68           
  Partials       29       29

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5d3fdb...740a589. Read the comment docs.

Copy link
Collaborator

@nikitin-da nikitin-da left a comment

Choose a reason for hiding this comment

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

Great work!
In few cases (updating, deleting fields, adding constrains) we have to (and we can) handle it manually and in all other it will be really helpful!

*
* @return version of database on which column was added.
*/
int version() default 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we start from 1? Because it is the base value by docs

String content;

// This column is not used in app, but shows how update code is generated
@StorIOSQLiteColumn(name = "NewColumn", ignoreNull = true, version = 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe make name snake case like another fields?

String content;

// This column is not used in app, but shows how update code is generated
@StorIOSQLiteColumn(name = "NewColumn", ignoreNull = true, version = 1)
String newColumn;
Copy link
Collaborator

Choose a reason for hiding this comment

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

And add nullability please ;)


import com.pushtorefresh.storio.sqlite.queries.Query;

// We suggest to store table meta such as table name, columns names, queries, etc in separate class
Copy link
Collaborator

@nikitin-da nikitin-da Dec 14, 2016

Choose a reason for hiding this comment

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

to store table meta such as table name,

DOUBLE_OBJECT,
STRING,
BYTE_ARRAY;
JavaType(String sqlType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

NotNull

import static com.pushtorefresh.storio.common.annotations.processor.generate.Common.INDENT;

/**
* Created by Pawel Bochenski on 11.12.2016.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We try to remove file headers

.build();
}

private Iterable<FieldSpec> generateFields(String table, Collection<StorIOSQLiteColumnMeta> columns) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add please nullability for methods in this file)

.initializer("$S", table)
.addModifiers(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)
.build());
for (StorIOSQLiteColumnMeta m : columns) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

m -> meta or column?!

int i = 0;
for (StorIOSQLiteColumnMeta entry : columns) {
code += entry.storIOColumn.name() + " " + entry.javaType.getSqlType();
if (!entry.storIOColumn.ignoreNull()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

=( ignoreNull absence doesn't guarantee database value existence.
It's just prevent overriding db field of there is no value in java entity.
Can we check NotNull, NonNull annotations?

if (!entry.storIOColumn.ignoreNull()) {
code += " NOT NULL";
}
if (entry.storIOColumn.key()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@nikitin-da
Copy link
Collaborator

Closes #694

Creating NOT NULL column now depends on field annotation (NonNull or NotNull)
@nikitin-da
Copy link
Collaborator

Thanks, @pbochenski !
@artem-zinnatullin PTAL, please)

Copy link
Member

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

Tests!!


import static com.pushtorefresh.storio.common.annotations.processor.generate.Common.INDENT;

public class TableGenerator implements Generator<StorIOSQLiteTypeMeta> {
Copy link
Member

Choose a reason for hiding this comment

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

This needs lots of tests :)

// Just a class for demonstration, real Tweet structure is more complex.

// This annotation will trigger annotation processor
// Which will generate type mapping code in compile time,
// You just need to link it in your code.
@StorIOSQLiteType(table = TweetsTable.TABLE)
@StorIOSQLiteType(table = "tweets")
Copy link
Member

Choose a reason for hiding this comment

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

Nice! But I not sure that everyone will need generated class, maybe add flag to this annotation? Something like @StorIOSQLiteType(table = "tweets", generateTableClass = false) which will be true by default?

@artem-zinnatullin
Copy link
Member

@geralt-encore can you please rebase this and merge? You're expert in StorIO annotation processing :)

@geralt-encore
Copy link
Collaborator

That will be tough since annotation processor was rewritten quite a lot since this PR...

@geralt-encore
Copy link
Collaborator

I'll try to convert it in the near future

@geralt-encore geralt-encore mentioned this pull request Oct 24, 2017
@nikitin-da
Copy link
Collaborator

Closed via #840

@nikitin-da nikitin-da closed this Oct 28, 2017
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.

5 participants