General codebase fixes by hrstoyanov · Pull Request #293 · ArcadeData/arcadedb

hrstoyanov · 2022-01-25T09:04:26Z

What does this PR do?
I ran ArcadeDB through Intellij code inspection and got back a lot of issues.

Motivation
Wanted to improve/modernize the code base.

I did not accept all fixes IntelliJ offered. For example, some of them are related to generics and require more investigation.

Checklist
[X] I have run the build using mvn clean package command
[X] My unit tests cover both failure and success scenarios

This reverts commit e7ebd57.

lvca · 2022-01-25T16:31:58Z

Hi @hrstoyanov, thanks for your PR! I quickly looked at some of the commits contained in the PR and they are legit. I can't merge the entire PR because it contains some commits I prefer not to include, like the first commit that undoes some changes you did in your branch but are not relevant in the main codebase.

Also, I see you fixed some issues in the SQL and GraphQL classes, but that code is automatically generated by JavaCC parser every time we modify the SQL/GraphQL grammars, so they would be lost at the very next change to the grammar.

I'm going to check one by one commit and cherry-pick the relevant ones.

hrstoyanov · 2022-01-25T16:50:49Z

Thanks Luka. Please cherry pick the commits - this is the best approach. I did try to avoid fixing auto-genetated classes, but I missed - please ignore them.

I did not realize how big of PR it is, so it's best to cherry pick and do this in stages.

hrstoyanov · 2022-01-25T16:55:34Z

A few things I can try once you are done with absorbing this batch:

there were a few fixes I could not understand why IntelliJ suggested. I can try them again.
I see the code uses SecurityManager which is to be removed from Java soon (jep 411).
I see usage of Unsafe for comparisons?
Generics. This could be sizable work.

lvca

Thanks @hrstoyanov, your contribution is much appreciated! I've finished my one by one commi review and I've already merged most of them. Thanks! I see you reverted the last commit but there were nice changes. WDYT about getting a rebase from main and see if you can send more commits by the topic of change?

lvca · 2022-01-25T16:46:09Z

engine/src/main/java/com/arcadedb/graph/EdgeIteratorFilter.java


  @Override
  protected void handleCorruption(final Exception e, final RID edge, final RID vertex) {
-    if ((e instanceof RecordNotFoundException || e instanceof SchemaException) &&//


Why did you remove this exception catching?

@lvca I always start from the latest code in your repo. I will wait for you to finish picking changes for the last 2 PRs, and continue from the latest code base.

engine/src/main/java/com/arcadedb/query/sql/executor/MultiValue.java

engine/src/main/java/com/arcadedb/query/sql/parser/ContainsTextCondition.java

engine/src/main/java/com/arcadedb/database/async/DatabaseAsyncExecutorImpl.java

e2e/pom.xml

engine/src/main/java/com/arcadedb/index/lsm/LSMTreeIndexCursor.java

engine/src/main/java/com/arcadedb/query/sql/executor/CartesianProductStep.java

#293

lvca · 2022-02-01T17:00:48Z

Merged single commits.

#293

@hrstoyanov

* First working implementation and test cases. Only for the engine. Still missing docs, integration with studio, storing on file and HTTP support. Issue #288 * User function: managed timeout on script execution This prevent infinite loops by mistake and on purpose intended as an attack * js engine: Added another test for the sandbox * Supported Java query engine. This invokes a Java method. Nice to write custom function easy to debug when runs embedded * Custom function: added security registration of allowed classes This avoid to use the java query engine to execute any Java method, but only from the classes registrated beforehand * Hava query engine: improved using of underlying exception * Refactoring to let the user to register the functions with a specific API * Complete refactoring of the function API To have libraries with functions written in different languages. With fluent API it's easier to declare the functions. Also, now functions are registered on a central place, so we could call them from SQL too. * SQL: merged SQLEngine class into SQLQueryEngine. Now SQL can see functions defined in libraries * Added javadoc on Database interface + minor changes to API * Implemented suggestions by @hrstoyanov #293 * Fixed Java engine supported languages * Fixed typo with Javadoc * Fixed compilation problems after rebase * Fixed compilation errors after rebase * fix: resolved merge issues * Fix issue with caching functions * feat: functions can be managed from SQL Issue #288 task 5

hr.stoyanov added 30 commits January 24, 2022 22:10

This looks like an error.

3495dd3

Private method also final (redundant)

cdc584c

Static method also final (redundant)

dbb6383

C-style array declaration

34c6c9f

Fields that can be final.

7e724f0

Optional value compared to null

7bf7718

Replacing sort.

d35c747

Remove redundant extends Object.

c1bc782

Unnecessary call to toString()

c4aa42e

Unnecessary semicolon

3536e7b

common part extracted out from if statement

8c03600

if statement simplified

16c9d57

Static member accessed via instance reference.

a7216a7

Static member accessed via instance reference.

ee4a98a

Empty catch (most likely error)

4a4741c

Empty catch - param renamed to ignore

0517281

Moder for-loop.

816c8ac

Explicit arguments replaced with <>

a368afc

Identical catch statements can be combined.

fc0fd78

'try' can use automatic resource management.

a092912

Use lambda.

d59ca36

Use method reference.

23ef119

Loop replaced.

5f46fdb

Use computeIfAbsent

df76df7

Use computeIfAbsent

afc5d6c

Replace with expression lambda.

4ca6ea1

Inner class can be static

0d05838

Long literal designated with L instead of l (better clarity).

c0fd2f0

+ removed

dc83a3f

Iteration replaced with addAll

1553cf6

hr.stoyanov added 7 commits January 25, 2022 00:08

Manual array copy replaced.

92b36ae

Manual array copy replaced.

05265f5

Redundant call to String.format

5d93934

String concatenation as argument to StringBuilder.append() call.

362e3db

String concatenation in loop fix (better performance).

d4b1307

Various fixes.

e7ebd57

Revert "Various fixes."

dedb327

This reverts commit e7ebd57.

lvca reviewed Jan 25, 2022

View reviewed changes

hrstoyanov changed the title ~~Genetral codebase fixes~~ General codebase fixes Jan 27, 2022

hrstoyanov marked this pull request as draft January 27, 2022 08:24

lvca self-assigned this Jan 29, 2022

lvca added this to the 22.1.3 milestone Jan 29, 2022

lvca added the enhancement New feature or request label Jan 29, 2022

lvca added a commit that referenced this pull request Jan 31, 2022

Implemented suggestions by @hrstoyanov

a942a18

#293

lvca closed this Feb 1, 2022

lvca added a commit that referenced this pull request Dec 23, 2022

Implemented suggestions by @hrstoyanov

e150c4d

#293

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

General codebase fixes#293

General codebase fixes#293
hrstoyanov wants to merge 37 commits intoArcadeData:mainfrom
hrstoyanov:fixes

hrstoyanov commented Jan 25, 2022

Uh oh!

lvca commented Jan 25, 2022

Uh oh!

hrstoyanov commented Jan 25, 2022

Uh oh!

hrstoyanov commented Jan 25, 2022

Uh oh!

lvca left a comment

Uh oh!

lvca Jan 25, 2022

Uh oh!

hrstoyanov Jan 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lvca commented Feb 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hrstoyanov commented Jan 25, 2022

Uh oh!

lvca commented Jan 25, 2022

Uh oh!

hrstoyanov commented Jan 25, 2022

Uh oh!

hrstoyanov commented Jan 25, 2022

Uh oh!

lvca left a comment

Choose a reason for hiding this comment

Uh oh!

lvca Jan 25, 2022

Choose a reason for hiding this comment

Uh oh!

hrstoyanov Jan 29, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lvca commented Feb 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants