From 39a520a9077f55f877f2b1ae4aaebb7496749588 Mon Sep 17 00:00:00 2001 From: Oren Rosen Date: Sat, 3 Dec 2022 18:05:07 +0200 Subject: [PATCH 1/7] add todos --- coolfacts/cmd/coolfacts_client/client.go | 10 +++-- coolfacts/cmd/coolfacts_client/main.go | 25 +++++++++-- coolfacts/cmd/coolfacts_server/server.go | 12 ++++- coolfacts/coolfact/fact.go | 4 ++ coolfacts/coolfact/service.go | 2 + coolfacts/coolfact/service_test.go | 56 +++++++++++++++++------- coolfacts/inmem/factsrepo.go | 12 +++-- 7 files changed, 95 insertions(+), 26 deletions(-) diff --git a/coolfacts/cmd/coolfacts_client/client.go b/coolfacts/cmd/coolfacts_client/client.go index 4262a44..08b4b46 100644 --- a/coolfacts/cmd/coolfacts_client/client.go +++ b/coolfacts/cmd/coolfacts_client/client.go @@ -47,7 +47,11 @@ func NewClient(endpoint string) *client { } func (c *client) GetLastCreatedFact() (coolfact.Fact, error) { - allFacts, err := c.GetAllFacts() + filters := coolfact.Filters{ + Limit: 1, + } + + allFacts, err := c.GetFacts(filters) if err != nil { return coolfact.Fact{}, fmt.Errorf("GetLastCreatedFact: %w", err) } @@ -59,8 +63,8 @@ func (c *client) GetLastCreatedFact() (coolfact.Fact, error) { return allFacts[0], nil } -func (c *client) GetAllFacts() ([]coolfact.Fact, error) { - ul := c.endpoint + pathGetFacts +func (c *client) GetFacts(filters coolfact.Filters) ([]coolfact.Fact, error) { + ul := fmt.Sprintf("%s%s?limit=%d&topic=%s", c.endpoint, pathGetFacts, filters.Limit, filters.Topic) res, err := c.httpClient.Get(ul) if err != nil { return nil, fmt.Errorf("client.GetLastCreatedFact to do request: %v", err) diff --git a/coolfacts/cmd/coolfacts_client/main.go b/coolfacts/cmd/coolfacts_client/main.go index 8c4b2d3..37dbd4a 100644 --- a/coolfacts/cmd/coolfacts_client/main.go +++ b/coolfacts/cmd/coolfacts_client/main.go @@ -7,6 +7,7 @@ import ( "log" "os" "regexp" + "strconv" "strings" "github.com/FTBpro/go-workshop/coolfacts/coolfact" @@ -15,7 +16,7 @@ import ( const ( serverEndpoint = "http://127.0.0.1:9002" - commandGetAllFacts = "getAllFact" + commandGetFacts = "getFacts" createFactCommand = "createFact" commandGetLastFact = "getLastFact" ) @@ -59,8 +60,26 @@ func processCmd(cl *client, cmd string, args []string) (string, error) { switch cmd { case "": return "", nil - case commandGetAllFacts: - facts, err := cl.GetAllFacts() + case commandGetFacts: + if len(args) < 1 { + return "", fmt.Errorf("must add argument for limit") + } + limit, err := strconv.Atoi(args[0]) + if err != nil { + return "", fmt.Errorf("limit must be a number") + } + + var topic string + if len(args) > 1 { + topic = args[1] + } + + filters := coolfact.Filters{ + Topic: topic, + Limit: limit, + } + + facts, err := cl.GetFacts(filters) if err != nil { return "", err } diff --git a/coolfacts/cmd/coolfacts_server/server.go b/coolfacts/cmd/coolfacts_server/server.go index 9d436af..136865b 100644 --- a/coolfacts/cmd/coolfacts_server/server.go +++ b/coolfacts/cmd/coolfacts_server/server.go @@ -12,6 +12,7 @@ import ( ) type FactsService interface { + // TODO: fix signature for GetFacts GetFacts() ([]coolfact.Fact, error) CreateFact(fact coolfact.Fact) error } @@ -75,9 +76,12 @@ func (s *server) HandlePing(w http.ResponseWriter, _ *http.Request) { } } -func (s *server) HandleGetFacts(w http.ResponseWriter, _ *http.Request) { +func (s *server) HandleGetFacts(w http.ResponseWriter, r *http.Request) { log.Println("Handling getFact ...") + // TODO: add filters. Read from query params using r.URL.Query() + // If the user didn't set limit, or the limit isn't an int. return bad request - User method HandleBadRequest + facts, err := s.factsService.GetFacts() if err != nil { s.HandleError(w, fmt.Errorf("server.GetFactsHandler: %w", err)) @@ -156,3 +160,9 @@ func (s *server) HandleError(w http.ResponseWriter, err error) { fmt.Printf("HandleGetFacts ERROR writing response: %s", err) } } + +func (s *server) HandleBadRequest(w http.ResponseWriter, err error) { + log.Println("Handling Bad Request ...") + + // TODO: implement +} diff --git a/coolfacts/coolfact/fact.go b/coolfacts/coolfact/fact.go index a764521..49420f1 100644 --- a/coolfacts/coolfact/fact.go +++ b/coolfacts/coolfact/fact.go @@ -7,3 +7,7 @@ type Fact struct { Description string CreatedAt time.Time } + +// TODO: add struct Filters with: +// - Topic +// - Limit diff --git a/coolfacts/coolfact/service.go b/coolfacts/coolfact/service.go index 9037ea0..641c588 100644 --- a/coolfacts/coolfact/service.go +++ b/coolfacts/coolfact/service.go @@ -3,6 +3,7 @@ package coolfact import "fmt" type Repository interface { + //TODO: fix signature for GetFacts GetFacts() ([]Fact, error) CreateFact(fct Fact) error } @@ -17,6 +18,7 @@ func NewService(factsRepo Repository) *service { } } +//TODO: fix signatur and call to repo func (s *service) GetFacts() ([]Fact, error) { facts, err := s.factsRepo.GetFacts() if err != nil { diff --git a/coolfacts/coolfact/service_test.go b/coolfacts/coolfact/service_test.go index eb9c2cb..f8fec20 100644 --- a/coolfacts/coolfact/service_test.go +++ b/coolfacts/coolfact/service_test.go @@ -17,40 +17,62 @@ func Test_service_GetFacts(t *testing.T) { facts := generateRandomFactsDesc(10) tests := []struct { - name string - repoField coolfact.Repository - want []coolfact.Fact - wantErr bool + name string + repoField coolfact.Repository + filtersInput coolfact.Filters + want []coolfact.Fact + wantErr bool }{ { name: "add in a sorted way", repoField: inmem.NewFactsRepository(facts...), - want: facts, - wantErr: false, + filtersInput: coolfact.Filters{ + Limit: 10, + }, + want: facts, + wantErr: false, }, { name: "add in a UNsorted way", repoField: inmem.NewFactsRepository(facts[5], facts[4], facts[2]), - want: []coolfact.Fact{facts[2], facts[4], facts[5]}, - wantErr: false, + filtersInput: coolfact.Filters{ + Limit: 10, + }, + want: []coolfact.Fact{facts[2], facts[4], facts[5]}, + wantErr: false, }, { name: "no facts - should get nil", repoField: inmem.NewFactsRepository(), - want: nil, - wantErr: false, + filtersInput: coolfact.Filters{ + Limit: 10, + }, + want: nil, + wantErr: false, }, { name: "repo returns error", repoField: mockRepoError{}, - want: nil, - wantErr: true, + filtersInput: coolfact.Filters{ + Limit: 10, + }, + want: nil, + wantErr: true, + }, + { + name: "limit", + repoField: inmem.NewFactsRepository(facts...), + filtersInput: coolfact.Filters{ + Limit: 5, + }, + want: facts[:5], + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := coolfact.NewService(tt.repoField) - got, err := s.GetFacts() + got, err := s.GetFacts(tt.filtersInput) if (err != nil) != tt.wantErr { t.Errorf("GetFacts() error = %v, wantErr %v", err, tt.wantErr) return @@ -111,7 +133,11 @@ func Test_service_CreateFact(t *testing.T) { require.NoError(t, err) } - gotFacts, err := s.GetFacts() + filters := coolfact.Filters{ + Limit: 10, + } + + gotFacts, err := s.GetFacts(filters) if tt.wantErr { require.Error(t, err) return @@ -146,7 +172,7 @@ func randomFact() coolfact.Fact { type mockRepoError struct { } -func (m mockRepoError) GetFacts() ([]coolfact.Fact, error) { +func (m mockRepoError) GetFacts(_ coolfact.Filters) ([]coolfact.Fact, error) { return nil, fmt.Errorf("mock repo returns error") } diff --git a/coolfacts/inmem/factsrepo.go b/coolfacts/inmem/factsrepo.go index 6495e00..f943508 100644 --- a/coolfacts/inmem/factsrepo.go +++ b/coolfacts/inmem/factsrepo.go @@ -2,27 +2,31 @@ package inmem import ( "sort" - + "github.com/FTBpro/go-workshop/coolfacts/coolfact" ) type factsRepo struct { - facts []coolfact.Fact + factsByTopic map[string][]coolfact.Fact } func NewFactsRepository(facts ...coolfact.Fact) *factsRepo { + // TODO: fix initialization according to the new field type return &factsRepo{ facts: facts, } } -func (r *factsRepo) GetFacts() ([]coolfact.Fact, error) { +func (r *factsRepo) GetFacts(filters coolfact.Filters) ([]coolfact.Fact, error) { + // TODO: fix method. Return according to the filters. + // note - topic is optional. sort.Sort(byCreatedAt(r.facts)) - + return r.facts, nil } func (r *factsRepo) CreateFact(fact coolfact.Fact) error { + // TODO: fix according to the new field type r.facts = append(r.facts, fact) return nil } From 35cef8ecec32abb43e76a6b4977f8b98305cdc11 Mon Sep 17 00:00:00 2001 From: Oren Rosen Date: Sun, 4 Dec 2022 15:42:07 +0200 Subject: [PATCH 2/7] Add doc --- coolfacts/docs/ex6-search-facts.md | 172 +++++++++++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 coolfacts/docs/ex6-search-facts.md diff --git a/coolfacts/docs/ex6-search-facts.md b/coolfacts/docs/ex6-search-facts.md new file mode 100644 index 0000000..45ab4d1 --- /dev/null +++ b/coolfacts/docs/ex6-search-facts.md @@ -0,0 +1,172 @@ +# Part 6 + +In this exercise you will add some more interesting use case for the get facts API. The option to search facts by topic, and add a limit. + +### The starting point +To get started, run this command to clone the necessary exercise materials in a convenient folder: +```commandline +$ git clone --branch v5-create-fact https://github.com/FTBpro/go-workshop.git +``` + +# **Getting Started** +You will add two query params to the get-facts API. `limit` and `topic`. Usually, we don't want that the client will get all resources, so we require they to send a limit. We also add the option to filter facts by a specific topic. + +In the client, we've added args to the command `getFacts` +```commandline +$ > getFacts [limit] [topic (optional)] +``` +You will implement this new ability at the server side. + +The tests were updated for testing the new functionality. After all is implemented they should pass. + +## Step 1 - coolfact/fact.go + +For supporting the new use case, you will add new type in the entity package coolfact. The function `GetFacts` will be changed to `SearchFacts` since it will get the filters and the repo will need to search the appropriate facts instead of just returns all of them. + +- Add the type `Filters` that will hold the fields that can be used for filtering the facts. + +## Step 2 - fix signatures +As said, the method `GetFacts` should be called `SearchFacts` and should receive arg `coolfact.Filters` +- Go over the project and fix all the signatures of the service and repo methods and in all the interfaces that requires them. There is a TODO next to any method/interface that needs to be fixed. + +## Step 3 - inmem/factsrepo.go +- In here you can notice that the type `factsRepo` now holds `factsByTopic map[string][]coolfact.Fact` instead of just `facts []coolfact.Fact`. Use this field and fix all the implementation inside the repo. + - `NewFactsRepository` - fix implementation. + - `GetFacts` - fix method name/signature and implementation. + - Note that `topic` is optional. + - `CreateFact` - fix implementation. + +## Step 3 - coolfact_service/server.go +- In `HandleGetFacts`, reaad the query params from the request and use them for inialize `coolfact.Filters` struct to be sent to the service method. + - Use `r.URL.Query()` for accessing the query params. + - Note that the limit is mandatory, meaning that if there isn't limit (or the limit isn't string), you should return bad request. In this case call the method `HandleBadRequest` which you will implement. +- Implement `HandleBadRequest`. Just like any other error response, but the status should be 400 (`http.StatusBadRequest`) + +# Build And Run +If all is implemented, you should be able to run tests and see them pass. +And you should see this: + +TODO: add gif + +# full Walkthrough + +## Step 1 - coolfact/fact.go +We'll add the new type for the available filters: +```go +type Filters struct { + Topic string + Limit int +} +``` + +## Step 2 -fix signature + +The method `GetFacts` is changed to `SearchFacts(filters coolfact.Filters) ([]coolfact.Fact, error)` + +We'll change in the service: +```go +type Repository interface { + SearchFacts(filters Filters) ([]Fact, error) // Was changed from `GetFacts` + CreateFact(fct Fact) error +``` +```go +func (s *service) SearchFacts(filters Filters) ([]Fact, error) {...} // Was changed from `GetFacts` +``` + +In the repo: +```go +func (r *factsRepo) SearchFacts(filters coolfact.Filters) ([]coolfact.Fact, error) {...} +``` + +And in the server: +```go +type FactsService interface { + SearchFacts(filters coolfact.Filters) ([]coolfact.Fact, error) // Was changed from `GetFacts` + CreateFact(fact coolfact.Fact) error +} +``` + +## Step 3 - the repo +We'll fix the initializer. We'll go over the facts, and add them under the right key +```go +func NewFactsRepository(facts ...coolfact.Fact) *factsRepo { + factsByTopic := map[string][]coolfact.Fact{} + for _, fact := range facts { + factsByTopic[fact.Topic] = append(factsByTopic[fact.Topic], fact) + } + + return &factsRepo{ + factsByTopic: factsByTopic, + } +} +``` +We'll fix the `GetFacts` to support the new filters: (Note that it is only one example of implementation, not focusing on performance at all) +```go +func (r *factsRepo) SearchFacts(filters coolfact.Filters) ([]coolfact.Fact, error) { + var facts []coolfact.Fact + if filters.Topic != "" { + facts = r.factsByTopic[filters.Topic] + } else { + facts = r.allFacts() + } + + sort.Sort(byCreatedAt(facts)) + + if filters.Limit < len(facts) { + facts = facts[:filters.Limit] + } + + return facts, nil +} + +func (s *factsRepo) allFacts() []coolfact.Fact { + var allFacts []coolfact.Fact + for _, facts := range s.factsByTopic { + allFacts = append(allFacts, facts...) + } + + return allFacts +} +``` + +Finally, we'll fix the `CreateFact` method, just add the fact to the facts of its topic: +```go +func (r *factsRepo) CreateFact(fact coolfact.Fact) error { + r.factsByTopic[fact.Topic] = append(r.factsByTopic[fact.Topic], fact) + + return nil +} +``` + +## Step 4 - Server +In `HandleGetFacts`, the server needs to call the service's method `SearchFact(filters)`. The filters will be built from the query params of the request. For example, if the client wish to get 10 facts of topic `TV`, it will call: +```commandline +GET /facts?limit=10&topic=TV +``` +Both the arguments will be received as strings, but we will need to convert the limit top int. If we can't, we'll return bad request +```go +func (s *server) HandleGetFacts(w http.ResponseWriter, r *http.Request) { + log.Println("Handling getFact ...") + + limitString := r.URL.Query().Get("limit") + if limitString == "" || limitString == "0" { + err := fmt.Errorf("limit is mandatory") + s.HandleBadRequest(w, err) + } + + limit, err := strconv.Atoi(limitString) + if err != nil { + err = fmt.Errorf("limit isn't int") + s.HandleBadRequest(w, err) + } + + filters := coolfact.Filters{ + Topic: r.URL.Query().Get("topic"), + Limit: limit, + } + + facts, err := s.factsService.GetFacts() + // code omitted +``` + +# Finish :boom: \ No newline at end of file From e35bc0c6b298f6396ff9b9dcc1ce6138fe5c6991 Mon Sep 17 00:00:00 2001 From: Oren Rosen Date: Sun, 4 Dec 2022 15:54:02 +0200 Subject: [PATCH 3/7] rename --- coolfacts/coolfact/service_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/coolfacts/coolfact/service_test.go b/coolfacts/coolfact/service_test.go index f8fec20..bb6b36f 100644 --- a/coolfacts/coolfact/service_test.go +++ b/coolfacts/coolfact/service_test.go @@ -13,7 +13,7 @@ import ( "github.com/FTBpro/go-workshop/coolfacts/inmem" ) -func Test_service_GetFacts(t *testing.T) { +func Test_service_SearchFacts(t *testing.T) { facts := generateRandomFactsDesc(10) tests := []struct { @@ -72,13 +72,13 @@ func Test_service_GetFacts(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := coolfact.NewService(tt.repoField) - got, err := s.GetFacts(tt.filtersInput) + got, err := s.SearchFacts(tt.filtersInput) if (err != nil) != tt.wantErr { - t.Errorf("GetFacts() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("SearchFacts() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("GetFacts() got = %v, want %v", got, tt.want) + t.Errorf("SearchFacts() got = %v, want %v", got, tt.want) } }) } @@ -137,7 +137,7 @@ func Test_service_CreateFact(t *testing.T) { Limit: 10, } - gotFacts, err := s.GetFacts(filters) + gotFacts, err := s.SearchFacts(filters) if tt.wantErr { require.Error(t, err) return @@ -172,7 +172,7 @@ func randomFact() coolfact.Fact { type mockRepoError struct { } -func (m mockRepoError) GetFacts(_ coolfact.Filters) ([]coolfact.Fact, error) { +func (m mockRepoError) SearchFacts(_ coolfact.Filters) ([]coolfact.Fact, error) { return nil, fmt.Errorf("mock repo returns error") } From da4bb3e27b77c31d8eba92b348cf5a6759a849a0 Mon Sep 17 00:00:00 2001 From: Oren Rosen Date: Mon, 5 Dec 2022 16:25:15 +0200 Subject: [PATCH 4/7] s --- coolfacts/cmd/coolfacts_server/server.go | 2 +- coolfacts/cmd/coolfacts_server/server_test.go | 208 ++++++++++++++++++ 2 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 coolfacts/cmd/coolfacts_server/server_test.go diff --git a/coolfacts/cmd/coolfacts_server/server.go b/coolfacts/cmd/coolfacts_server/server.go index 136865b..1d3292c 100644 --- a/coolfacts/cmd/coolfacts_server/server.go +++ b/coolfacts/cmd/coolfacts_server/server.go @@ -80,7 +80,7 @@ func (s *server) HandleGetFacts(w http.ResponseWriter, r *http.Request) { log.Println("Handling getFact ...") // TODO: add filters. Read from query params using r.URL.Query() - // If the user didn't set limit, or the limit isn't an int. return bad request - User method HandleBadRequest + // If the user didn't set limit, or the limit isn't an int. return bad request - (use method HandleBadRequest) facts, err := s.factsService.GetFacts() if err != nil { diff --git a/coolfacts/cmd/coolfacts_server/server_test.go b/coolfacts/cmd/coolfacts_server/server_test.go new file mode 100644 index 0000000..dd0ca0c --- /dev/null +++ b/coolfacts/cmd/coolfacts_server/server_test.go @@ -0,0 +1,208 @@ +package main_test + +import ( + "bytes" + "encoding/json" + "fmt" + "math/rand" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/require" + + server "github.com/FTBpro/go-workshop/coolfacts/cmd/coolfacts_server" + "github.com/FTBpro/go-workshop/coolfacts/coolfact" +) + +func Test_Server_GetFacts(t *testing.T) { + facts := generateRandomFactsDesc(10) + tests := []struct { + name string + queryParamsToSend string + expectedFilters coolfact.Filters + want []coolfact.Fact + wantErr bool + expectedHTTPStatus int + }{ + { + name: "10 facts with filters", + queryParamsToSend: "?limit=10&topic=TV", + expectedFilters: coolfact.Filters{ + Topic: "TV", + Limit: 10, + }, + want: facts, + expectedHTTPStatus: http.StatusOK, + }, + { + name: "no topic", + queryParamsToSend: "?limit=10", + expectedFilters: coolfact.Filters{ + Topic: "", + Limit: 10, + }, + want: facts, + expectedHTTPStatus: http.StatusOK, + }, + { + name: "no limit - expect bad request", + queryParamsToSend: "", + want: nil, + wantErr: true, + expectedHTTPStatus: http.StatusBadRequest, + }, + { + name: "limit is not an int - expect bad request", + queryParamsToSend: "?limit=one", + want: nil, + wantErr: true, + expectedHTTPStatus: http.StatusBadRequest, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockService := mockFactsService{ + factsToReturn: tt.want, + } + + srv := server.NewServer(&mockService) + ts := httptest.NewServer(srv) + + res, err := http.Get(ts.URL + "/facts" + tt.queryParamsToSend) + require.NoError(t, err) + require.Equal(t, tt.expectedHTTPStatus, res.StatusCode) + + if tt.wantErr { + return + } + + gotFacts, err := factsFromResponse(t, res) + require.NoError(t, err) + + require.Equal(t, tt.expectedFilters, mockService.filtersGot) + + require.Equal(t, tt.want, gotFacts) + }) + } +} + +func Test_Server_CreateFacts(t *testing.T) { + facts := generateRandomFactsDesc(10) + tests := []struct { + name string + queryParamsToSend string + factToCreate coolfact.Fact + wantErr bool + expectedHTTPStatus int + }{ + { + name: "10 facts with filters", + factToCreate: facts[0], + expectedHTTPStatus: http.StatusOK, + }, + { + name: "no topic", + factToCreate: facts[0], + expectedHTTPStatus: http.StatusOK, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockService := mockFactsService{} + srv := server.NewServer(&mockService) + ts := httptest.NewServer(srv) + + payload := map[string]interface{}{ + "topic": tt.factToCreate.Topic, + "description": tt.factToCreate.Description, + } + + postBody, err := json.Marshal(payload) + require.NoError(t, err) + + responseBody := bytes.NewBuffer(postBody) + + res, err := http.Post(ts.URL+"/facts", "application/json", responseBody) + require.NoError(t, err) + require.Equal(t, tt.expectedHTTPStatus, res.StatusCode) + + if tt.wantErr { + return + } + require.Equal(t, tt.factToCreate.Topic, mockService.createFactGotFact.Topic) + require.Equal(t, tt.factToCreate.Description, mockService.createFactGotFact.Description) + }) + } +} + +func generateRandomFactsDesc(n int) []coolfact.Fact { + var facts []coolfact.Fact + for i := 0; i < n; i++ { + fact := randomFact() + fact.CreatedAt = time.Now().Add(-(time.Duration(i) * time.Hour)).UTC() + facts = append(facts, fact) + } + + return facts +} + +func randomFact() coolfact.Fact { + rand.Seed(time.Now().UnixNano()) + return coolfact.Fact{ + Topic: fmt.Sprintf("Topic %d", rand.Intn(10000)), + Description: fmt.Sprintf("Some Description %d", rand.Intn(10000)), + } +} + +type getFactsResponse struct { + Facts []struct { + Topic string `json:"topic"` + Description string `json:"description"` + CreatedAt time.Time `json:"createdAt"` + } `json:"facts"` +} + +func factsFromResponse(t *testing.T, res *http.Response) ([]coolfact.Fact, error) { + var factsResponse getFactsResponse + err := json.NewDecoder(res.Body).Decode(&factsResponse) + require.NoErrorf(t, err, "factsFromResponse failed decode get facts response") + + facts := make([]coolfact.Fact, len(factsResponse.Facts)) + for i, fact := range factsResponse.Facts { + facts[i] = coolfact.Fact(fact) + } + + return facts, nil +} + +type mockFactsService struct { + filtersGot coolfact.Filters + factsToReturn []coolfact.Fact + shouldReturnError bool + + createFactGotFact coolfact.Fact +} + +func (m *mockFactsService) SearchFacts(filters coolfact.Filters) ([]coolfact.Fact, error) { + m.filtersGot = filters + + if m.shouldReturnError { + return nil, fmt.Errorf("mockFactsService asked to return an error") + } + + return m.factsToReturn, nil +} + +func (m *mockFactsService) CreateFact(fact coolfact.Fact) error { + m.createFactGotFact = fact + + if m.shouldReturnError { + return fmt.Errorf("mockFactsService asked to return an error") + } + + return nil +} From 28447f3acfa4b88eaa359d242759440a05e967bf Mon Sep 17 00:00:00 2001 From: Oren Rosen Date: Fri, 9 Dec 2022 15:05:45 +0200 Subject: [PATCH 5/7] rename --- coolfacts/cmd/coolfacts_server/server_test.go | 2 +- coolfacts/coolfact/service_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/coolfacts/cmd/coolfacts_server/server_test.go b/coolfacts/cmd/coolfacts_server/server_test.go index dd0ca0c..7e3f6d7 100644 --- a/coolfacts/cmd/coolfacts_server/server_test.go +++ b/coolfacts/cmd/coolfacts_server/server_test.go @@ -187,7 +187,7 @@ type mockFactsService struct { createFactGotFact coolfact.Fact } -func (m *mockFactsService) SearchFacts(filters coolfact.Filters) ([]coolfact.Fact, error) { +func (m *mockFactsService) GetFacts(filters coolfact.Filters) ([]coolfact.Fact, error) { m.filtersGot = filters if m.shouldReturnError { diff --git a/coolfacts/coolfact/service_test.go b/coolfacts/coolfact/service_test.go index bb6b36f..f8fec20 100644 --- a/coolfacts/coolfact/service_test.go +++ b/coolfacts/coolfact/service_test.go @@ -13,7 +13,7 @@ import ( "github.com/FTBpro/go-workshop/coolfacts/inmem" ) -func Test_service_SearchFacts(t *testing.T) { +func Test_service_GetFacts(t *testing.T) { facts := generateRandomFactsDesc(10) tests := []struct { @@ -72,13 +72,13 @@ func Test_service_SearchFacts(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := coolfact.NewService(tt.repoField) - got, err := s.SearchFacts(tt.filtersInput) + got, err := s.GetFacts(tt.filtersInput) if (err != nil) != tt.wantErr { - t.Errorf("SearchFacts() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GetFacts() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("SearchFacts() got = %v, want %v", got, tt.want) + t.Errorf("GetFacts() got = %v, want %v", got, tt.want) } }) } @@ -137,7 +137,7 @@ func Test_service_CreateFact(t *testing.T) { Limit: 10, } - gotFacts, err := s.SearchFacts(filters) + gotFacts, err := s.GetFacts(filters) if tt.wantErr { require.Error(t, err) return @@ -172,7 +172,7 @@ func randomFact() coolfact.Fact { type mockRepoError struct { } -func (m mockRepoError) SearchFacts(_ coolfact.Filters) ([]coolfact.Fact, error) { +func (m mockRepoError) GetFacts(_ coolfact.Filters) ([]coolfact.Fact, error) { return nil, fmt.Errorf("mock repo returns error") } From eb69e1c1d8b28128b1d9c5b236281b899a9ff4b6 Mon Sep 17 00:00:00 2001 From: Oren Rosen Date: Sat, 10 Dec 2022 15:52:10 +0200 Subject: [PATCH 6/7] fix test --- coolfacts/cmd/coolfacts_server/server.go | 11 +---------- coolfacts/cmd/coolfacts_server/server_test.go | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/coolfacts/cmd/coolfacts_server/server.go b/coolfacts/cmd/coolfacts_server/server.go index 8252467..a436360 100644 --- a/coolfacts/cmd/coolfacts_server/server.go +++ b/coolfacts/cmd/coolfacts_server/server.go @@ -88,16 +88,6 @@ func (s *server) HandleGetFacts(w http.ResponseWriter, r *http.Request) { return } - // we first format the facts to map[string]interface. - formattedFacts := make([]map[string]interface{}, len(facts)) - for i, coolFact := range facts { - formattedFacts[i] = map[string]interface{}{ - "topic": coolFact.Topic, - "description": coolFact.Description, - "createdAt": coolFact.CreatedAt, - } - } - response := s.formatGetFactsResponse(facts) // write status and content-type @@ -171,6 +161,7 @@ func (s *server) formatGetFactsResponse(facts []coolfact.Fact) map[string]interf formattedFacts[i] = map[string]interface{}{ "topic": coolFact.Topic, "description": coolFact.Description, + "createdAt": coolFact.CreatedAt, } } diff --git a/coolfacts/cmd/coolfacts_server/server_test.go b/coolfacts/cmd/coolfacts_server/server_test.go index 7e3f6d7..147aa27 100644 --- a/coolfacts/cmd/coolfacts_server/server_test.go +++ b/coolfacts/cmd/coolfacts_server/server_test.go @@ -83,8 +83,7 @@ func Test_Server_GetFacts(t *testing.T) { require.NoError(t, err) require.Equal(t, tt.expectedFilters, mockService.filtersGot) - - require.Equal(t, tt.want, gotFacts) + expectEqualFacts(t, tt.want, gotFacts) }) } } @@ -206,3 +205,15 @@ func (m *mockFactsService) CreateFact(fact coolfact.Fact) error { return nil } + +func expectEqualFacts(t *testing.T, expected, got []coolfact.Fact) { + require.Equalf(t, len(expected), len(got), "expectEqualFacts: different length") + + for i, gotFact := range got { + expectedFact := expected[i] + require.Equal(t, expectedFact.Topic, gotFact.Topic) + require.Equal(t, expectedFact.Description, gotFact.Description) + fmt.Println("------------------ ", gotFact.CreatedAt) + require.Equal(t, expectedFact.CreatedAt, gotFact.CreatedAt) + } +} From 8a77ffe1503fb2a600777faaf845c129d01fa520 Mon Sep 17 00:00:00 2001 From: Oren Rosen Date: Sat, 10 Dec 2022 16:25:23 +0200 Subject: [PATCH 7/7] doc --- coolfacts/cmd/coolfacts_server/server.go | 34 ++++++------ coolfacts/cmd/coolfacts_server/server_test.go | 38 ++++++------- coolfacts/docs/ex6-search-facts.md | 54 +++++++++++++++++-- 3 files changed, 87 insertions(+), 39 deletions(-) diff --git a/coolfacts/cmd/coolfacts_server/server.go b/coolfacts/cmd/coolfacts_server/server.go index a4c2d1e..bab58f7 100644 --- a/coolfacts/cmd/coolfacts_server/server.go +++ b/coolfacts/cmd/coolfacts_server/server.go @@ -7,7 +7,7 @@ import ( "net/http" "strings" "time" - + "github.com/FTBpro/go-workshop/coolfacts/coolfact" ) @@ -42,7 +42,7 @@ func NewServer(factsService FactsService) *server { func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) { log.Println("incoming request", r.Method, r.URL.Path) - + switch r.Method { case http.MethodGet: switch strings.ToLower(r.URL.Path) { @@ -67,9 +67,9 @@ func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) { func (s *server) HandlePing(w http.ResponseWriter, _ *http.Request) { log.Println("Handling Ping ...") - + w.WriteHeader(http.StatusOK) - + if _, err := fmt.Fprint(w, "PONG"); err != nil { fmt.Printf("ERROR writing to ResponseWriter: %s\n", err) return @@ -78,20 +78,20 @@ func (s *server) HandlePing(w http.ResponseWriter, _ *http.Request) { func (s *server) HandleGetFacts(w http.ResponseWriter, r *http.Request) { log.Println("Handling getFact ...") - + facts, err := s.factsService.GetFacts() if err != nil { s.HandleError(w, fmt.Errorf("server.GetFactsHandler: %w", err)) return } - + response := s.formatGetFactsResponse(facts) - + // write status and content-type // status must be written before the body w.WriteHeader(http.StatusOK) w.Header().Set("Content-Type", "application/json") - + // write the body. We use json encoding if err := json.NewEncoder(w).Encode(response); err != nil { fmt.Printf("HandleGetFacts ERROR writing response: %s", err) @@ -100,33 +100,33 @@ func (s *server) HandleGetFacts(w http.ResponseWriter, r *http.Request) { func (s *server) HandleCreateFact(w http.ResponseWriter, r *http.Request) { log.Println("Handling createFact ...") - + var request createFactRequest if err := json.NewDecoder(r.Body).Decode(&request); err != nil { err = fmt.Errorf("server.HandleCreateFact failed to decode request: %s", err) s.HandleError(w, err) return } - + if err := s.factsService.CreateFact(request.ToCoolFact()); err != nil { err = fmt.Errorf("server.HandleCreateFact: %s", err) s.HandleError(w, err) return } - + w.WriteHeader(http.StatusOK) } func (s *server) HandleNotFound(w http.ResponseWriter, r *http.Request) { log.Println("Handling notFound ...") - + w.WriteHeader(http.StatusNotFound) w.Header().Set("Content-Type", "application/json") - + response := map[string]string{ "error": fmt.Sprintf("path %s %s not found", r.Method, r.URL.Path), } - + if err := json.NewEncoder(w).Encode(response); err != nil { err = fmt.Errorf("HandleNotFound failed to decode: %s", err) s.HandleError(w, err) @@ -135,7 +135,7 @@ func (s *server) HandleNotFound(w http.ResponseWriter, r *http.Request) { func (s *server) HandleError(w http.ResponseWriter, err error) { log.Println("Handling error ...") - + w.WriteHeader(http.StatusInternalServerError) w.Header().Set("Content-Type", "application/json") response := map[string]string{ @@ -148,7 +148,7 @@ func (s *server) HandleError(w http.ResponseWriter, err error) { func (s *server) HandleBadRequest(w http.ResponseWriter, err error) { log.Println("Handling Bad Request ...") - + // TODO: implement } @@ -161,7 +161,7 @@ func (s *server) formatGetFactsResponse(facts []coolfact.Fact) map[string]interf "createdAt": coolFact.CreatedAt, } } - + return map[string]interface{}{ "facts": formattedFacts, } diff --git a/coolfacts/cmd/coolfacts_server/server_test.go b/coolfacts/cmd/coolfacts_server/server_test.go index 147aa27..813d2ba 100644 --- a/coolfacts/cmd/coolfacts_server/server_test.go +++ b/coolfacts/cmd/coolfacts_server/server_test.go @@ -18,7 +18,7 @@ import ( func Test_Server_GetFacts(t *testing.T) { facts := generateRandomFactsDesc(10) - tests := []struct { + testCases := []struct { name string queryParamsToSend string expectedFilters coolfact.Filters @@ -62,35 +62,35 @@ func Test_Server_GetFacts(t *testing.T) { }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { mockService := mockFactsService{ - factsToReturn: tt.want, + factsToReturn: tc.want, } srv := server.NewServer(&mockService) ts := httptest.NewServer(srv) - res, err := http.Get(ts.URL + "/facts" + tt.queryParamsToSend) + res, err := http.Get(ts.URL + "/facts" + tc.queryParamsToSend) require.NoError(t, err) - require.Equal(t, tt.expectedHTTPStatus, res.StatusCode) + require.Equal(t, tc.expectedHTTPStatus, res.StatusCode) - if tt.wantErr { + if tc.wantErr { return } gotFacts, err := factsFromResponse(t, res) require.NoError(t, err) - require.Equal(t, tt.expectedFilters, mockService.filtersGot) - expectEqualFacts(t, tt.want, gotFacts) + require.Equal(t, tc.expectedFilters, mockService.filtersGot) + expectEqualFacts(t, tc.want, gotFacts) }) } } func Test_Server_CreateFacts(t *testing.T) { facts := generateRandomFactsDesc(10) - tests := []struct { + testCases := []struct { name string queryParamsToSend string factToCreate coolfact.Fact @@ -109,15 +109,16 @@ func Test_Server_CreateFacts(t *testing.T) { }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { mockService := mockFactsService{} srv := server.NewServer(&mockService) ts := httptest.NewServer(srv) + defer ts.Close() payload := map[string]interface{}{ - "topic": tt.factToCreate.Topic, - "description": tt.factToCreate.Description, + "topic": tc.factToCreate.Topic, + "description": tc.factToCreate.Description, } postBody, err := json.Marshal(payload) @@ -127,13 +128,13 @@ func Test_Server_CreateFacts(t *testing.T) { res, err := http.Post(ts.URL+"/facts", "application/json", responseBody) require.NoError(t, err) - require.Equal(t, tt.expectedHTTPStatus, res.StatusCode) + require.Equal(t, tc.expectedHTTPStatus, res.StatusCode) - if tt.wantErr { + if tc.wantErr { return } - require.Equal(t, tt.factToCreate.Topic, mockService.createFactGotFact.Topic) - require.Equal(t, tt.factToCreate.Description, mockService.createFactGotFact.Description) + require.Equal(t, tc.factToCreate.Topic, mockService.createFactGotFact.Topic) + require.Equal(t, tc.factToCreate.Description, mockService.createFactGotFact.Description) }) } } @@ -213,7 +214,6 @@ func expectEqualFacts(t *testing.T, expected, got []coolfact.Fact) { expectedFact := expected[i] require.Equal(t, expectedFact.Topic, gotFact.Topic) require.Equal(t, expectedFact.Description, gotFact.Description) - fmt.Println("------------------ ", gotFact.CreatedAt) require.Equal(t, expectedFact.CreatedAt, gotFact.CreatedAt) } } diff --git a/coolfacts/docs/ex6-search-facts.md b/coolfacts/docs/ex6-search-facts.md index 78aebfd..988eb45 100644 --- a/coolfacts/docs/ex6-search-facts.md +++ b/coolfacts/docs/ex6-search-facts.md @@ -10,11 +10,59 @@ In the client, the command `getFacts` now receives two new argument: ```commandline $ > getFacts [limit] [topic (optional)] ``` -You will implement this new ability at the server side. Limit will be mandatory, and topic will be optional. +You will implement this new functionality on the server side. Limit will be mandatory, and topic will be optional. -The tests were updated for testing the new functionality. After all is implemented they should pass. +## Added Tests + +You can notice that we've added tests for the server under file `.../cmd/coolfacts_server/server_test.go`. The structure of the tests is the same as we saw earlier, but there is a couple of new concepts that we can go over. Let's look in the `t.run` scope in `Test_server_GetFacts`: +```go +t.Run(tt.name, func(t *testing.T) { + mockService := mockFactsService{ + factsToReturn: tt.want, + } + + srv := server.NewServer(&mockService) + ts := httptest.NewServer(srv) + + res, err := http.Get(ts.URL + "/facts" + tt.queryParamsToSend) + require.NoError(t, err) + require.Equal(t, tt.expectedHTTPStatus, res.StatusCode) + + if tt.wantErr { + return + } + + gotFacts, err := factsFromResponse(t, res) + require.NoError(t, err) + + require.Equal(t, tt.expectedFilters, mockService.filtersGot) + expectEqualFacts(t, tt.want, gotFacts) +}) +``` + +### `mockFactsService` +Our server depends on `FactsService` interface. Since we don't wish to use a real service, we use a mock. our type `mockFactsService` implements this interface, so we can use it to initialize the server +```go + mockService := mockFactsService{ + factsToReturn: tt.want, + } + + srv := server.NewServer(&mockService) +``` + +We initialize our mock with the facts we defined in our `testCase`. Go over the implementation of this mock which is a really simple Go code. + +Next you can notice this line of code: +```go +ts := httptest.NewServer(srv) +``` +We use a Go package `httptest` which provides utilities for HTTP testing. The method `httptest.NewServer` starts and returns a new `httptest.Server`. This server has a field `URL` which is a base URL of form "http://ipaddr:port". When we will issue a call to this URL, the request will be directed to our `srv` that we've passed. + +So we can use this URL to issue a "real" http request: +```go +res, err := http.Get(ts.URL + "/facts" + tc.queryParamsToSend) +``` -In this ## Step 1 - coolfact/fact.go For supporting the new use case, you will add a new type in the entity package `coolfact`. This type will represent the filters that the service supports.