TechnionYP5777 / Bugquery

Bug query
9 stars 1 forks source link

dbparsing Redesign Discussion #102

Closed ZivIzhar closed 7 years ago

ZivIzhar commented 7 years ago

I think there should be a redesign in dbparsing. To be clear, I'm not talking about the orm (dealt with in #76) - I'm reffering to our "database representation" in the code.

The problems in the current design as far as I can see (if I'm wrong, please correct me):

Let's discuss :) (no redesign is also an option)

Amit-Oha commented 7 years ago

@tonylekhtman convinced me that ORM is not right for us, mostly because it lacks optimizations which could be essential for us. We are rethinking the design of the entire DB department. Currently reading about DAO pattern.

Amit-Oha commented 7 years ago

after reading this I have changed my mind again. full documentation is here, we should read this before coming to conclusion

Amit-Oha commented 7 years ago

Spring Test DBUnit could help too

Amit-Oha commented 7 years ago

last for today: we can configure an in-memory db profile for testing and use this to use it on tests. more about embedded dbs here

Amit-Oha commented 7 years ago

testing db specifically in spring boot tutorial here - saves configuration, probably suits us best

Amit-Oha commented 7 years ago

the most explicit example i found - here, but when trying to implement on our project it throws an exception. will try again tomorrow

Amit-Oha commented 7 years ago

regarding custom configuration for tests in a manner that runs tests on in-memory db vs. production on mysql server - it says here that:

@DataJpaTest can be used if you want to test JPA applications. By default it will configure an in-memory embedded database, scan for @Entity classes and configure Spring Data JPA repositories. Regular @Component beans will not be loaded into the ApplicationContext.

so we can use this annotation to save context configuration time while running the tests and we don't need to define our own context.

Amit-Oha commented 7 years ago

about embedded objects (in our case stacktrace embedded in post)

Amit-Oha commented 7 years ago

Good news - we got over many obstacles regarding usage and testing of JPA and Spring Data, and got to the point where we created the table for posts like we wanted. Apparently, creating the table for base class "post" also creates columns for each inheriting class (such as MinSOPost, StackOvrflowPost etc). In order to complete the redesign issue, I need to know if there is any need to keep these inheriting classes - especially considering that we dropped many features regarding the site that could have used that extra information. A compromise for now can be creating an extra column specifying the data's source, but we don't really need it at the moment because we know all of our posts come from stack overflow. Please express your opinion ASAP so we can be done with it by sunday's meeting @ZivIzhar @yonzarecki @rodedzats @tonylekhtman

yonzarecki commented 7 years ago

Nah, I don't think we really need these classes. They were made with the future in mind, but if they are in the way there is no real reason to keep them IMO.

Amit-Oha commented 7 years ago

screenshots of the tables schemas (not all column types are correct, will be fixed soon) screenshot from 2017-05-12 13-01-20 screenshot from 2017-05-12 13-02-20

Amit-Oha commented 7 years ago

Apparently to access an embedded object's property you need to name the method differently as explained here, so I'm hoping no one else will have to spend an hour on this issue 😢

tonylekhtman commented 7 years ago

@AmitOhayon and I finished. With additional demands new issues would open in the future. Closed.