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

SQL #158

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

SQL #158

wants to merge 9 commits into from

Conversation

lbajolet
Copy link
Collaborator

@lbajolet lbajolet commented Oct 31, 2016

1dan1m

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)


This change is Reviewable

@privat
Copy link
Collaborator

privat commented Oct 31, 2016

plase update tests/test_*.nit

.gitignore Outdated
@@ -3,7 +3,6 @@ bin/
*.orig
db_loader_serial.nit
out/
db/
Copy link
Collaborator

Choose a reason for hiding this comment

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

we still need to ignore the directory I think

.gitignore Outdated
@@ -3,7 +3,6 @@ bin/
*.orig
db_loader_serial.nit
out/
db/
Copy link
Collaborator

Choose a reason for hiding this comment

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

we still need to ignore the db generated files

@privat
Copy link
Collaborator

privat commented Oct 31, 2016

the PR is huge. If you do not plan to split it, could you explain what you did exactly?

@Morriar
Copy link
Owner

Morriar commented Oct 31, 2016

45 files changed, 3 commits... seriously?

privat pushed a commit that referenced this pull request Nov 1, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
@privat
Copy link
Collaborator

privat commented Nov 1, 2016

TODO:

  • move db/ to something else.
  • add libsqlite3-dev into the Dockerfile

@lbajolet
Copy link
Collaborator Author

lbajolet commented Nov 1, 2016

Done

privat pushed a commit that referenced this pull request Nov 1, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Dockerfile Outdated
@@ -1,7 +1,7 @@
FROM nitlang/nit

# Needed for nitcorn and to build mongo-c-driver
RUN apt-get update && apt-get install -y libevent-dev libssl-dev libsasl2-dev libcurl4-openssl-dev file
RUN apt-get update && apt-get install -y libevent-dev libssl-dev libsasl2-dev libcurl4-openssl-dev file libsqlite3-dev

# Install mongo-c-driver manually since it is not available in Debian/jessie
RUN curl -L https://github.com/mongodb/mongo-c-driver/releases/download/1.4.0/mongo-c-driver-1.4.0.tar.gz -o mongo-c-driver-1.4.0.tar.gz \
Copy link

@ventilooo ventilooo Nov 1, 2016

Choose a reason for hiding this comment

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

Mongo-c-driver is no longer required if sqlite is used .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should not be, but there seems to be some import somewhere at the moment which prevents us from compiling when mongoc is unavailable, so we left it for now

privat pushed a commit that referenced this pull request Nov 1, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
@lbajolet
Copy link
Collaborator Author

lbajolet commented Nov 1, 2016

OK, for the explanation of what is happening here, the new model is articulated around a few things:

  • DBContext: Basically the equivalent of a Context in Hibernate or Entity terminology, a glorified-ish Connection to a Database with eventual caches and such. Our DBContext has no caches at the moment, but might be added in the future to help with performance.
  • Two kinds of Entity are defined, UniqueEntity which are basically entities with a single Primary Key: Mission, Player, Track, etc., and BridgeEntity, which are tables with more than one primary key: MissionStatus, TrackStatus, StarStatus, etc.
  • All entities have a commit method which persists the current state of an object to the Database, which based on the current state of the object (are its primary keys set or not), will be either inserted or updated to the database. For now, entities do not have a delete method, but this could be added later if needs be. Therefore, each Entity needs to have an insert and an update redefinition for it to work properly.
  • Objects are a bit more lightweight with the actual model as the linked entities are not eagerly fetched in most cases (the exception being the TrackStatus, which triggers the collection/creation of MissionStatus objects, which themselves trigger the collection/creation of StarStatus objects). But the usage from the API POV should remain more or less the same.
  • The deserialization of objects from the database is done within the Statement and StatementRow objects, provided they contain the right data for the creation of the corresponding objects in the model. If this is not the case, they will print (log when available) an error message and return null or an empty Array of the proper type.
  • The DBContext provides the general request methods for each object type in the model with a slightly different naming convention as before. Ex: To get a mission by its id, using the old API this would have looked like config.missions.by_id(id), now it will look like ctx.mission_by_id(id). Fundamentally, this does not change much the use of the API.
  • The API side of the application should be more or less equal to the old one, at the exception that most of the uses of id have been replaced by slug as slug is now the user-friendly version of an id for use in URLs. We made the decision to have the ids as Int in the database to put less burden when creating Foreign Keys, as Ints are less costly than Strings. This is also speedier when joining or selecting data on the ID. Note that all the Entities do not necessarily have a slug field, only those who are used in URLs: Player, Mission, Track, Achievement.
  • Concerning Achivements, the model changed a bit too, as the old model used Achievement as both the definition and the unlock of an Achievement, the operation is now split in two, Achievements are their own table, and the unlocks are too.
  • The frontend part changed a bit due to the re-organization of the server-side data, but all in all, is pretty much the same as before. This is actually the part I'm less comfortable about since I might have overlooked some things, and this has changed a lot since I've started working on the PR. And I suck at JS.

This should sum-up the differences, if there are any questions left, feel free to ask.

privat pushed a commit that referenced this pull request Nov 1, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
privat pushed a commit that referenced this pull request Nov 1, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
@@ -127,7 +126,7 @@ class ShUser
serialize

# The *code permanent* (or the uid for non student)
var id: String
var slug: String
Copy link
Collaborator

Choose a reason for hiding this comment

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

bad

@@ -115,10 +115,15 @@ abstract class AuthLogin
# Register a new player and add a first-login achievement
#
# Helper method to use when a new account is created.
fun register_new_player(player: Player)
fun register_new_player(ctx: DBContext, player: Player)
Copy link
Collaborator

Choose a reason for hiding this comment

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

use the ctx of the player

@@ -30,6 +30,10 @@ class APIRouter
var config: AppConfig
end

redef class HttpRequest
var ctx: DBContext is lazy do return new DBContext
Copy link
Collaborator

Choose a reason for hiding this comment

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

doc


var old_score = self.old_score
if old_score != null then
res += " (was {old_score})"
end
return res
end

redef fun insert do
Copy link
Collaborator

Choose a reason for hiding this comment

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

factorize the insert/update/whatever with the "template method" design pattern™ by extracting the query part

# Create a dummy user
var player = new Player("John", "Doe")
config.players.save player
with ctx = new DBContext do
Copy link
Collaborator

Choose a reason for hiding this comment

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

a with? Ohhhhhh

@@ -38,6 +38,6 @@ <h1 class='pull-right'>
</div>
</div>
<mission-submit mission='missionCtrl.mission' mission-status='missionCtrl.missionStatus'
ng-if='missionCtrl.playerId' />
ng-if='missionCtrl.playerId && missionCtrl.missionStatus.status != "locked"' />
Copy link
Collaborator

Choose a reason for hiding this comment

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

:( see #29

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We'll add some debug/god mode someday for this.

@@ -4,9 +4,6 @@
</div>
<div class='card-body'>
<h5>{{notification.object}}</h5>
<p class='text-muted'>
{{notification.timestamp * 1000 | date: 'dd, MMM, yyyy HH:MM:ss a'}}
</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

var solve_reward: Int = 0 is writable, serialize_as("reward")
var path: String = "" is writable
# Template for the source code
var template: nullable String = null is writable
Copy link
Collaborator

Choose a reason for hiding this comment

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

add template in the DB

@privat
Copy link
Collaborator

privat commented Nov 1, 2016

mission tree is broken

privat pushed a commit that referenced this pull request Nov 2, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
privat pushed a commit that referenced this pull request Nov 3, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
@privat
Copy link
Collaborator

privat commented Nov 3, 2016

+1

@Morriar
Copy link
Owner

Morriar commented Nov 7, 2016

@R4PaSs: I would definitely expect some benchmarks explaining why we have to endure this 1500 lines addon :)

@@ -13,52 +13,56 @@ var opts = new AppOptions.from_args(args)
var config = new AppConfig.from_options(opts)

# clean bd
config.db.drop
#config.db.drop
Copy link
Owner

Choose a reason for hiding this comment

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

Just delete it(c)


# Load nit
config.load_track("tracks/nit")
#config.load_track("tracks/nit")
Copy link
Owner

Choose a reason for hiding this comment

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

Just delete it(c)

privat pushed a commit that referenced this pull request Nov 30, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->

---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
privat pushed a commit that referenced this pull request Nov 30, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->

---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
privat pushed a commit that referenced this pull request Dec 6, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->

---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
privat pushed a commit that referenced this pull request Dec 7, 2016
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->

---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
privat pushed a commit that referenced this pull request Jan 20, 2017
![1dan1m](https://cloud.githubusercontent.com/assets/1444825/19865386/650afce4-9f72-11e6-8e49-5152e7f5f2ed.jpg)

Big PR here, this should make the transition from a MongoDB model to the famed SQL model.
The code base being large, there might be some parts of the application that might break, but from the last checks I did, most of the app should work.
Some parts, like achievements may need a bit of an overhaul to make it production-ready, but the base is here.

Note: I know the login redirect is semi-broken, I'll check to fix it, in the meantime, you are more than encouraged to take a look at the code :)

<!-- Reviewable:start -->

---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/morriar/missions/158)
<!-- Reviewable:end -->

Pull-Request: #158
Reviewed-by: Jean Privat <[email protected]>
Reviewed-by: Alexandre Terrasa <[email protected]>
Reviewed-by: Istvan SZALAÏ <[email protected]>
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.

4 participants