Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

Roadmap suggestion #3

Open
peteraba opened this issue Apr 8, 2018 · 11 comments
Open

Roadmap suggestion #3

peteraba opened this issue Apr 8, 2018 · 11 comments

Comments

@peteraba
Copy link
Contributor

peteraba commented Apr 8, 2018

Hi @gernest,

First of all thanks for this project, I find it pretty handy and I find the code structure nice as well. I have a made a few modifications to it to better fit my need, but before I contribute back I'd like to discuss my ideas with you.

Features I already added:

  1. Enabled various HTTP Methods.
  2. Enabled imitating errors by optionally providing response code probabilities.
  3. Make 201 the default response code for POST calls to make it more REST-compliant (http://www.restapitutorial.com/lessons/httpmethods.html)

Features I'm considering adding:

  1. Reading an initial set of endpoints from a directory based on a CLI flag / Environment variable. (Currently I call a bash file on startup that will create the endpoints via curl, not the nicest workaround)
  2. Reading a swagger file and creating endpoints automatically. (This might be out of scope, but I'd find it useful.)
  3. Adding optional request validation based on the swagger file read previously. (I'm least sure about this one, but this would potentially make the error imitation
  4. Support for more API frameworks.

What are your thoughts?

In case you wondered, this is how my API structure currently looks:

// API is the struct for the json object that is passed to apidemic for registration.
type API struct {
	Endpoint                  string                 `json:"endpoint"`
	HTTPMethod                string                 `json:"http_method"`
	ResponseCodeProbabilities map[int]int            `json:"response_code_probabilities"`
	Payload                   map[string]interface{} `json:"payload"`
}

(Changes so far are 100% backwards compatible)

@gernest
Copy link
Owner

gernest commented Apr 8, 2018

Hey, thanks for feedback. I think the changes are good. So, we will need to breakdown, and implement the features incrementally in small patches to allow me to do review.

More discussion will be held on the patches (PR) since this is rather a long list and it is easy to miss small details.

One thing, can you expand more about ResponseCodeProbabilities and why do we need it?

How about we start with number 3(Make 201 the default response code for POST ) ?
Looking forward to your PR.

@peteraba
Copy link
Contributor Author

peteraba commented Apr 8, 2018

I'm very pleased with you being so available.Thank you!

Well, depending on which #3 your suggesting it might not make sense without #1 or #2. :)

I need ResponseCodeProbabilities to be able to test error handling on my UI.

I"ll split my changes to 1+3 and 2 for the ones I already have to make it easier for you to review. Then I'll add PR for #1 from the proposed

@peteraba
Copy link
Contributor Author

peteraba commented Apr 8, 2018

Btw, I have one more feature request which is not super mandatory but could be handy: being able to change the endpoints on the fly. Right now we just reject repeated registrations for the same endpoint.

@gernest
Copy link
Owner

gernest commented Apr 8, 2018

Uh! I see. I get it now. I agree, we can change the registration object to reflect on which method the response should match.

So, I think ResponseCodeProbabilities will be an overkill for now.

type API struct {
	Endpoint                  string                 `json:"endpoint"`
	HTTPMethod                string                 `json:"http_method"`
	Payload                   interface{}            `json:"payload"`
}

Looks much convenient , we don't want to limit the payload to map[sting]interface{} there are cases you want to serve array of objects instead of maps.

@gernest
Copy link
Owner

gernest commented Apr 8, 2018

Btw, I have one more feature request which is not super mandatory but could be handy: being able to change the endpoints on the fly. Right now we just reject repeated registrations for the same endpoint

Lets go slowly, one feature at a time. Maybe you can open a separate Issue for each feature so it can be easily discussed/tracked

@peteraba
Copy link
Contributor Author

peteraba commented Apr 8, 2018

Looks much convenient , we don't want to limit the payload to map[sting]interface{} there are cases you want to serve array of objects instead of maps.

Well this sounds fine to me in general but not something I need right now.

@peteraba
Copy link
Contributor Author

peteraba commented Apr 8, 2018

Hi @gernest, thanks for the quick code reviews so far. I just wanted to let you know that I'll stop working on the project for now until further feedback. What exists as a PR already covers all my current needs and I think if you'd prefer to keep this project simple, swagger support can even go into a different project which would use apidemic under the hood.

@gernest
Copy link
Owner

gernest commented Apr 9, 2018

@peteraba thanks for the contribution. Glad you find this project useful.

@peteraba
Copy link
Contributor Author

peteraba commented Apr 9, 2018

So I kind of had to create two new PRs to fulfill my needs:

  1. I needed UUID generation, but added some number and boolean generation on the way
  2. I needed to be able to return lists (as you mentioned above).

With these PRs in place I think I'll have everything I need for now.

@gernest
Copy link
Owner

gernest commented Apr 10, 2018

Thanks a lot for helping me out. I'm happy that someone finds this useful

@peteraba
Copy link
Contributor Author

@gernest and I thank you for the fast responses. It's been a very smooth experience to contribute so far!

makasim referenced this issue in makasim/apidemic Oct 3, 2019
makasim referenced this issue in makasim/apidemic Oct 3, 2019
string(3127) "MongoDB\Driver\Exception\BulkWriteException: Updating the
path 'message.reply_markup.inline_keyboard.0' would create a conflict at
'message.reply_markup.inline_keyboard' in
/app/vendor/mongodb/mongodb/src/Operation/Update.php:179
app_1      | Stack trace:
app_1      | #0
/app/vendor/mongodb/mongodb/src/Operation/Update.php(179):
MongoDB\Driver\Server->executeBulkWrite('avita.pvm_token',
Object(MongoDB\Driver\BulkWrite), Array)
app_1      | #1
/app/vendor/mongodb/mongodb/src/Operation/UpdateOne.php(105):
MongoDB\Operation\Update->execute(Object(MongoDB\Driver\Server))
app_1      | #2 /app/vendor/mongodb/mongodb/src/Collection.php(1033):
MongoDB\Operation\UpdateOne->execute(Object(MongoDB\Driver\Server))
app_1      | #3 /app/vendor/formapro/yadm/src/Storage.php(161):
MongoDB\Collection->updateOne(Array, Array, Array)
app_1      | #4
/app/vendor/formapro/pvm/src/Yadm/StandaloneTokenDAL.php(98):
Formapro\Yadm\Storage->update(Object(App\Model\PvmToken))
app_1      | #5 /app/vendor/formapro/pvm/src/ProcessEngine.php(289):
Formapro\Pvm\Yadm\StandaloneTokenDAL->persistToken(Object(App\Model\PvmToken))
app_1      | #6 /app/vendor/formapro/pvm/src/ProcessEngine.php(217):
Formapro\Pvm\ProcessEngine->persistToken(Object(App\Model\PvmToken))
app_1      | gernest#7 /app/vendor/formapro/pvm/src/ProcessEngine.php(251):
Formapro\Pvm\ProcessEngine->doProceed(Object(App\Model\PvmToken))
app_1      | gernest#8 /app/vendor/formapro/pvm/src/ProcessEngine.php(188):
Formapro\Pvm\ProcessEngine->transition(Object(App\Model\PvmToken))
app_1      | gernest#9 /app/vendor/formapro/pvm/src/ProcessEngine.php(78):
Formapro\Pvm\ProcessEngine->doProceed(Object(App\Model\PvmToken))
app_1      | gernest#10 /app/src/Service/PvmHandler.php(132):
Formapro\Pvm\ProcessEngine->proceed(Object(App\Model\PvmToken))
app_1      | gernest#11 [internal function]:
App\Service\PvmHandler->App\Service\{closure}(Object(App\Model\PvmToken),
Object(App\Storage\PvmTokenStorage))
app_1      | gernest#12 /app/vendor/formapro/yadm/src/Storage.php(300):
call_user_func(Object(Closure), Object(App\Model\PvmToken),
Object(App\Storage\PvmTokenStorage))
app_1      | gernest#13 /app/src/Storage/PvmStorage.php(70):
Formapro\Yadm\Storage->lock(Object(MongoDB\BSON\ObjectId),
Object(Closure))
app_1      | #14 /app/src/Service/PvmHandler.php(137):
App\Storage\PvmStorage->lockToken(Object(App\Model\PvmToken),
Object(Closure))
app_1      | #15 /app/src/Service/PvmHandler.php(114):
App\Service\PvmHandler->handleToken(Object(App\Model\PvmToken))
app_1      | #16
/app/src/Controller/TelegramUpdatesHookController.php(31):
App\Service\PvmHandler->handleUpdate(Object(Formapro\TelegramBot\Update))
app_1      | #17 /app/vendor/symfony/http-kernel/HttpKernel.php(150):
App\Controller\TelegramUpdatesHookController->__invoke(Object(Symfony\Component\HttpFoundation\Request),
'725382688:AAFZQ...', Object(Formapro\TelegramBot\Bot),
Object(App\Service\PvmHandler), Object(Symfony\Bridge\Monolog\Logger))
app_1      | #18 /app/vendor/symfony/http-kernel/HttpKernel.php(67):
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request),
1)
app_1      | #19 /app/vendor/symfony/http-kernel/Kernel.php(198):
Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request),
1, true)
app_1      | #20 /app/public/index.php(25):
Symfony\Component\HttpKernel\Kernel->handle(Object(Symfony\Component\HttpFoundation\Request))
app_1      | #21 {main}"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants