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

Refactor Job API #44

Open
Lihis opened this issue Mar 7, 2020 · 12 comments
Open

Refactor Job API #44

Lihis opened this issue Mar 7, 2020 · 12 comments
Labels
enhancement New feature or request
Milestone

Comments

@Lihis
Copy link
Owner

Lihis commented Mar 7, 2020

When job is sent to API it contains plenty of fields which are relevant only
with specific status and it also forces server side to use just one route for
different statuses.

See this comment instead.

Job API should be split into:

New (a.k.a OnJob)

URL: /job

Method: POST

{
    "game": "ets2",
    "type": 1,
    "isSpecial": true,
    "income": 6878,
    "trailer: {
        ...
    },
    "truck": {
        ...
    },
    "cargo": {
        ...
    },
    "source": {
        ...
    },
    "destination": {
        ...
    }
}

On success

Respond with HTTP code 201 and return JSON:

{ "id": 123456 }

On failure

If user already has an active job respond with HTTP code 409.

Cancelled

URL: /job/<id>

Method: DELETE

{
    "penalty": 1200
}

On success

Respond with any of following HTTP codes: 200, 202, 204.

On failure

When job not found with <id> then HTTP code 404.

Delivered

URL: /job/<id>

Method: PUT

{
    "revenue": 6878,
    "xp": 120,
    "time": 234,
    "maxSpeed": 25.0,
    "fuelConsumed": 40.0,
    "autoPark": false,
    "autoLoad": false,
    "damage": 0.0,
    "distance": {
        ...
    }
}

On success

Respond with any of following HTTP codes: 200, 204.

On failure

When job not found with <id> then 404.

@Lihis Lihis added this to the v0.1.0 milestone Mar 7, 2020
@Lihis Lihis added the enhancement New feature or request label Mar 7, 2020
@doouz
Copy link
Contributor

doouz commented Mar 7, 2020

If you close the app, you will loose the jobid, and remember that one person can have two profiles, can start a job to prepare a convoy that will happen in some hours, and then change to other profile to do a job in the meantime.

How it works now, In the web side I search for the job first based on the status, 1, 2 and 3, and then filter by variables sent to web, in that way I can identify a job.

Maybe something like
/job/start (just once when job start)
/job/update (every 30 secs)
/job/end (just once when job is delivered)
/job/cancel (just once when job is cancelled)

The thing with the jobid is great, but I think that will not work with multiple jobs and if the client is closed, goodbye job. Instead of job id, maybe need to work as work today, sending the job variables like source and destination city and company, planed distance and cargo_id because with that we can identify a job without a job id.

@doouz
Copy link
Contributor

doouz commented Mar 7, 2020

Was thinking, if I had started a job in one profile and I change to that profile, how you will know if that is a new job and need to send to /job/start or is a job started before so need to send /job/update? I dont think in that before 🤔

@Lihis
Copy link
Owner Author

Lihis commented Mar 7, 2020

ID should go to disk with some information to identify the job, like those which you said. The information in disk would not be sent to the server but just be used to identify the job.

When plugin sends the job to GUI check is there matching job, if yes then just call /job/update, otherwise /job/start.

If we have an ID for the job, then server could be also queried for Distance driven and Fuel consumed for display purposes.

@doouz
Copy link
Contributor

doouz commented Mar 7, 2020

Nice! That sound even better, really good Tomi. And If someone change that jobid for another one and cancel the actual job? That can cancel the job of another one, maybe is good to always send some jobs variable to confirm that de jobid correspond to that job. Or maybe on database I need to store the jobs with random bigint ids so are not correlative numbers. 🤔

@Lihis
Copy link
Owner Author

Lihis commented Mar 7, 2020

Then they cancel if it is not cancelled, if it is cancelled then there is nothing to do. And obviously an user should not be able to cancel another user's job because you have an access control, right?

Or maybe on database I need to store the jobs with random bigint ids so are not correlative numbers.

I have heard some big boys talking about not to reveal database ID's but I ain't a web developer 😊

@doouz
Copy link
Contributor

doouz commented Mar 7, 2020

Your are right! I totally forget about the token! Yeah, this will work really great!

@Lihis
Copy link
Owner Author

Lihis commented Mar 7, 2020

So basically something like this would make sense?

Start

URL: /job
Method: POST

{
    "game": "ets2",
    "type": 1,
    "isSpecial": true,
    "income": 6878,
    "trailer: {
        ...
    },
    "truck": {
        ...
    },
    "cargo": {
        ...
    },
    "source": {
        ...
    },
    "destination": {
        ...
    }
}

Return ID:

{ "id": 123 }

Get info

Get Fuel consumed and Distance driven

URL: /job/<id>
Method: GET

{ "driven": 123, "fuelConsumed": 400 }

Update

URL: /job/<id>/update
Method: POST

{
    "fuel": 300,
    "driven": 123
    "maxSpeed": 20
}

Max speed in m/s (just like now).

Cancel

URL: /job/<id>/cancel
Method: POST

{
    "penalty": 1200
}

End

URL: /job/<id>/end
Method: POST

{
    "revenue": 6878,
    "xp": 120,
    "time": 234,
    "maxSpeed": 25.0,
    "fuelConsumed": 40.0,
    "autoPark": false,
    "autoLoad": false,
    "damage": 0.0,
    "distance": {
        ...
    }
}

Also add <id> to the Truck -endpoint.


ID and information to identify the job should be saved to disk.

@doouz
Copy link
Contributor

doouz commented Mar 7, 2020

Looks great, that will improve a lot the web side to manage all the info.

Also a “nice to have” is possible that when you send /truck (position, speed, etc) can add a variable there like jobid, and if the player is in job the /truck will post the jobid and when not, just post null or 0 or false. Im thinking about this because this will help to do a webmap and also for other VTC convoy control features.

Im just asking, really dont know of this can have a lot of complexity to do it.

@Lihis
Copy link
Owner Author

Lihis commented Mar 7, 2020

That would make sense.

@doouz
Copy link
Contributor

doouz commented Mar 7, 2020

Cool, sounds really great!

@Lihis
Copy link
Owner Author

Lihis commented Mar 22, 2020

Or maybe on database I need to store the jobs with random bigint ids so are not correlative numbers. 🤔

I have heard some big boys talking about not to reveal database ID's but I ain't a web developer 😊

I talked about this with friend of mine who is doing web development professionally.

Exposing database primary key or using UUID instead is not related to security but rather about hiding business logic. Consider that you have an online shop were you can view order details by the ID for example with an URL of http://example.com/orders/1234, if the ID does not exist you return 404, and 403 if it is not an order of yours. So what is the issue? Now your competitor can determine how many orders you process for example on daily basis; just find out the biggest ID which returns 403, note it down, tomorrow at same time find out what is the biggest ID returning 403.

This would have been easily prevented by returning the same HTTP code no matter does the ID exist or does the user have an permission to view it. But one can still see the ID by placing an order.

When using UUID then there is no way to determine how many orders per day you process as it's not an continuously growing number.

To anyone who is implementing an API this basically boils down to the question: do you want to expose your business logic? How valuable the exposed information is?

He also threw in an idea of the client generating the UUID's. Honestly it sounds good idea because then when there is no internet connection when an user starts a job, I don't need to assign a bogus ID for it until server responds with an real ID -> no need to then loop through of unsent messages and replace the bogus ID with the real one. But I'll discuss with him a little bit more.

@doouz
Copy link
Contributor

doouz commented Sep 27, 2020

Hi @Lihis I'd like to know if you have some time to review this? I would like to go back to the project I mentioned last year and much of it depends on the ETS2-Job-Logger working correctly.

As always, thank you very much for the time you spend on this!

@Lihis Lihis modified the milestones: v0.1.0, v0.2.0 Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants