geoffrey42 / fizzbuzz Goto Github PK

Describe the bug

If str1 or str2 contain '-' character in /api/fizzbuzz query, they are not returned by /api/stats.

To Reproduce
Steps to reproduce the behavior:

1. Run the server
2. hit /api/fizzbuzz enough time to be the top request by doing:

curl http://127.0.0.1:5000/api/fizzbuzz?int1=2&int2=8&limit=16&str1=Perceval&str2=---

3. hit /api/stats and check the result.

{
    "hit": 4,
    "int1": 2,
    "int2": 8,
    "limit": 16,
    "str1": "Perceval"
}

Expected behavior

Given previous step it should return str2 like so:

{
    "hit": 4,
    "int1": 2,
    "int2": 8,
    "limit": 16,
    "str1": "Perceval",
    "str2": "---"
}

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: MacOS BigSur 11.1
• Browser: Firefox
• Version: 84.0.1

Additional context
Add any other context about the problem here.

Is your feature request related to a problem? Please describe.

This is the current api/stats return model:

{
    "hit": 6,
    "int1": 3,
    "int2": 5,
    "limit": 16,
    "str1": "fizz",
    "str2": "buzz"
}

Even if it satisfies the assignment's required, it's not really something a client would use in situ.

Describe the solution you'd like

{
    "hit": 6,
    "params": 
    {
        "int1": 3,
        "int2": 5,
        "limit": 16,
        "str1": "fizz",
        "str2": "buzz"
    }
}

Describe alternatives you've considered

Additional context or screenshots

Is your feature request related to a problem? Please describe.

utils.BuildMemberFromParams function has no Example in its test file.

Describe the solution you'd like

add doc example for utils.BuildMemberFromParams function.

Describe alternatives you've considered

Additional context or screenshots

Is your feature request related to a problem? Please describe.

This test main requirement rely on a REST API server performing a fizzbuzz based on specific input.

Describe the solution you'd like

Add fizz-buzz core logic.

Based on assignment, it should take the form a the following function's prototype:

func doFizzBuzz(int1 int, int2 int, limit int, str1 string, str2 string) string

Describe alternatives you've considered

None

Additional context or screenshots

None

Describe the bug

/api/stats does not return error when there are multiple top request

To Reproduce
Steps to reproduce the behavior:

1. Flush redis by running make redis-cli then FLUSHALL.
2. hit /api/fizzbuzz once:

curl http://127.0.0.1:5000/api/fizzbuzz?int1=2&int2=8&limit=16&str1=Perceval&str2=Karadoc

3. hit again /api/fizzbuzz once with different query parameters:

curl http://127.0.0.1:5000/api/fizzbuzz?int1=3&int2=5&limit=16&str1=fizz&str2=buzz

4. hit /api/stats to check the top request:

{
    "hit": 4,
    "int1": 2,
    "int2": 8,
    "limit": 16,
    "str1": "Perceval",
    "str2": "Karadoc"
}

Expected behavior

Return some error about existing multiple top requests.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: MacOS BigSur 11.1
• Browser: Firefox
• Version: 84.0.1

Additional context
Add any other context about the problem here.

Is your feature request related to a problem? Please describe.

For now the server was mainly built and ran locally on my laptop. In order make it more maintainable or to easily add some CI/CD, having the app running inside containers would be a nice.

Describe the solution you'd like

Package the whole server application in docker composition.

Describe alternatives you've considered

Additional context or screenshots

Is your feature request related to a problem? Please describe.

The core fizzbuzz logic is built. Now we have to serve its results whenever a client request it.

Describe the solution you'd like

Given original assignment I have to:

Implement a web server that will expose a REST API endpoint that:

• Accepts five parameters : three integers int1, int2 and limit, and two strings str1 and str2.
• Returns a list of strings with numbers from 1 to limit, where: all multiples of int1 are replaced by str1, all multiples of int2 are replaced by str2, all multiples of int1 and int2 are replaced by str1str2.

The server needs to be:

• Ready for production
• Easy to maintain by other developers

One way to do that would be to use go-swagger OpenAPI specification.

Describe alternatives you've considered

Implementing the web server endpoint myself.

Additional context or screenshots

None

Is your feature request related to a problem? Please describe.

Redis is a key element in our stack. Without it, it's impossible to achieve the 2nd assignment's requirement.

Describe the solution you'd like

Add redis monitoring. Maybe using Prometheus/redis-exporter/Grafana.

Describe alternatives you've considered

Additional context or screenshots

Describe the bug

fizzbuzz is incorrect for multiples of 1

To Reproduce
Steps to reproduce the behavior:

1. hit /api/fizzbuzz?int1=1&int2=8&limit=3&str1=Perceval&str2=Karadoc
2. See error:

[
    "1",
    "Perceval",
    "Perceval"
]

Expected behavior

[
    "Perceval",
    "Perceval",
    "Perceval"
]

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: MacOS BigSur 11.1
• Browser: Firefox
• Version: 84.0.1

Additional context
Add any other context about the problem here.

Describe the bug

ExampleDoFizzBuzz fails when tested

To Reproduce
Steps to reproduce the behavior:

1. At the root the project, run go test github.com/Geoffrey42/fizzbuzz/fb
2. See error:

go test github.com/Geoffrey42/fizzbuzz/fb   
--- FAIL: ExampleDoFizzBuzz (0.00s)
got:
[]
want:
["1", "2", "fizz", "4", "buzz", "fizz", "7", "8", "fizz", "buzz", "11", "fizz", "13", "14", "fizzbuzz", "16"]
FAIL
FAIL    github.com/Geoffrey42/fizzbuzz/fb       0.344s
FAIL

Expected behavior

Test should pass.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: MacOS BigSur 11.1
• Browser: Firefox
• Version: 84.0.1

Additional context

The problem originated from DoFizzBuzz's limit parameter being set to 0 in ExampleDoFizzBuzz preventing any number enumeration. See:

func ExampleDoFizzBuzz() {
	res, _ := DoFizzBuzz(3, 5, 0, "fizz", "buzz")

	fmt.Println(res)
	// Output: ["1", "2", "fizz", "4", "buzz", "fizz", "7", "8", "fizz", "buzz", "11", "fizz", "13", "14", "fizzbuzz", "16"]
}

Is your feature request related to a problem? Please describe.

The server is up and running. It's possible to get a fizzbuzz collection of numbers. But the other main assignment's requirement is still missing: a stats endpoint to get the most frequent request.

Describe the solution you'd like

Here is the assignment's requirement:

Add a statistics endpoint allowing users to know what the most frequent request has been.

This endpoint should:

• Accept no parameter
• Return the parameters corresponding to the most used request, as well as the number of hits for this request

Describe alternatives you've considered

None

Additional context or screenshots

None

Is your feature request related to a problem? Please describe.

For now the whole travis build pipeline run in parallel. But it makes no sens to build the app if unit tests fails first.

Some jobs need to run sequentially.

Describe the solution you'd like

split build in multiple stages

Describe alternatives you've considered

Additional context or screenshots

Is your feature request related to a problem? Please describe.

There are several little issues with current README that prevent it to be crystal clear.

Describe the solution you'd like

make documentation clearer

Describe alternatives you've considered

Additional context or screenshots

Is your feature request related to a problem? Please describe.

Currently there's an explanation of the fizzbuzz logic in the README. But nothing about how api/stats endpoint works.

Describe the solution you'd like

document api/stats endpoint source code

Describe alternatives you've considered

Additional context or screenshots

Describe the bug

doFizzBuzz function is returning a string.

To Reproduce

Not relevant

Expected behavior

doFizzBuzz function should return a list of strings.

Screenshots

Not relevant

Smartphone (please complete the following information):

Not applicable

Additional context
Add any other context about the problem here.

Is your feature request related to a problem? Please describe.

This is it. Despite few remaining bugs, the project build successfully on Travis. But it only ran on my laptop so far.

Describe the solution you'd like

deploy once the build succeed on Travis. It wil be the occasion to merge on main.

Describe alternatives you've considered

Additional context or screenshots

Is your feature request related to a problem? Please describe.

Project lies in docker containers for now, but the whole development process is pretty much local and not suitable for shared workflow and team work.

Describe the solution you'd like

Add some CI/CD. At least checking:

At Test stage

• unit tests
• code coverage
• swagger.yml validation

At Build state

• build the project using docker-compose.

Describe alternatives you've considered

Additional context or screenshots