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

[Go]Paging tests and general paging fixes #1489

Merged
merged 16 commits into from
Oct 18, 2016
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,6 @@ Gemfile.lock
src/generator/AutoRest.Go.Tests/pkg/*
src/generator/AutoRest.Go.Tests/bin/*
src/generator/AutoRest.Go.Tests/src/github.com/*
src/generator/AutoRest.Go.Tests/src/test/vendor/*
src/generator/AutoRest.Go.Tests/src/test/generated/*
src/generator/AutoRest.Go.Tests/src/tests/generated/*
src/generator/AutoRest.Go.Tests/src/tests/vendor/*
src/generator/AutoRest.Go.Tests/src/tests/glide.lock
16 changes: 16 additions & 0 deletions CodeGenerator.sln
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AutoRest.Ruby", "src\genera
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AutoRest.Ruby.Azure", "src\generator\AutoRest.Ruby.Azure\AutoRest.Ruby.Azure.csproj", "{31931998-7543-41DA-9E58-D9670D810352}"
EndProject
Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "AutoRest.Go", "src\generator\AutoRest.Go\AutoRest.Go.xproj", "{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes we should. Install Golang and glide.

EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Go", "Go", "{49002B6B-C3F6-490E-874A-3113905DF17B}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".build", ".build", "{04E8E124-852C-4B5D-83EB-0B8ADDE825CB}"
ProjectSection(SolutionItems) = preProject
build.proj = build.proj
Expand Down Expand Up @@ -247,6 +251,16 @@ Global
{31931998-7543-41DA-9E58-D9670D810352}.Portable-Release|Any CPU.ActiveCfg = Portable-Release|Any CPU
{31931998-7543-41DA-9E58-D9670D810352}.Portable-Release|Any CPU.Build.0 = Portable-Release|Any CPU
{31931998-7543-41DA-9E58-D9670D810352}.Release|Any CPU.ActiveCfg = Portable-Release|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Debug|Any CPU.ActiveCfg = Portable-Debug|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Net45-Debug|Any CPU.ActiveCfg = Net45-Debug|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Net45-Debug|Any CPU.Build.0 = Net45-Debug|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Net45-Release|Any CPU.ActiveCfg = Net45-Release|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Net45-Release|Any CPU.Build.0 = Net45-Release|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Portable-Debug|Any CPU.ActiveCfg = Portable-Debug|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Portable-Debug|Any CPU.Build.0 = Portable-Debug|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Portable-Release|Any CPU.ActiveCfg = Portable-Release|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Portable-Release|Any CPU.Build.0 = Portable-Release|Any CPU
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5}.Release|Any CPU.ActiveCfg = Portable-Release|Any CPU
{34B82690-0083-4F4C-8ABF-2D2A09304915}.Debug|Any CPU.ActiveCfg = Portable-Debug|Any CPU
{34B82690-0083-4F4C-8ABF-2D2A09304915}.Net45-Debug|Any CPU.ActiveCfg = Net45-Debug|Any CPU
{34B82690-0083-4F4C-8ABF-2D2A09304915}.Net45-Debug|Any CPU.Build.0 = Net45-Debug|Any CPU
Expand Down Expand Up @@ -440,6 +454,8 @@ Global
{DE95444A-F6FD-46DC-BBDC-A1A6886A6F2D} = {EF9E346D-70C6-45F5-8FF9-9B734F4A1298}
{A7780698-3072-486E-A105-81EDDF552598} = {DE95444A-F6FD-46DC-BBDC-A1A6886A6F2D}
{31931998-7543-41DA-9E58-D9670D810352} = {DE95444A-F6FD-46DC-BBDC-A1A6886A6F2D}
{BEDCF556-6CEA-4EAE-ACE3-8B9EFE0818E5} = {49002B6B-C3F6-490E-874A-3113905DF17B}
{49002B6B-C3F6-490E-874A-3113905DF17B} = {EF9E346D-70C6-45F5-8FF9-9B734F4A1298}
{5989E210-AE15-4DF4-8CEE-DEE609740FD2} = {04E8E124-852C-4B5D-83EB-0B8ADDE825CB}
{9750C692-C2D0-4D0F-9F73-D45DB9E906CE} = {5989E210-AE15-4DF4-8CEE-DEE609740FD2}
{A9C01442-1E93-4C2D-9182-B61C9F53C3FF} = {EF9E346D-70C6-45F5-8FF9-9B734F4A1298}
Expand Down
11 changes: 11 additions & 0 deletions Documentation/building-code.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ To set up a machine with the necessary tools for building AutoRest, run `.\Tools
- Gradle
- Python
- tox
- Go
- glide
- Visual Studio 2015
- .NET CoreCLR

Expand Down Expand Up @@ -89,6 +91,15 @@ gem install bundler
Install [Python 2.7 and Python 3.5](https://www.python.org/downloads/), and add one of them to your PATH (we recommend 3.5).
>set PATH=PATH;C:\Python35

### Go
Install [Go 1.7](https://golang.org/dl/). Ensure Go is in your `PATH`.
>set PATH=PATH;C:\Go\bin
Add your [GOPATH](https://golang.org/doc/code.html#GOPATH) to your environment variables.

#### Glide

Install [glide](https://github.com/Masterminds/glide). Add glide to your `PATH`.

### Testing Your Environment
To make sure you've set up all the prerequisites correctly, run `.\Tools\Verify-Settings.ps1` before you attempt to build.

Expand Down
2 changes: 2 additions & 0 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ var goMappings = {
'required-optional':['../../dev/TestServer/swagger/required-optional.json','optionalgroup'],
'url':['../../dev/TestServer/swagger/url.json','urlgroup'],
'validation':['../../dev/TestServer/swagger/validation.json', 'validationgroup'],
'paging':['../../dev/TestServer/swagger/paging.json', 'paginggroup'],
'azurereport':['../../dev/TestServer/swagger/azure-report.json', 'azurereport']
};

var defaultAzureMappings = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package paginggrouptest

import (
"net/http"
"testing"

chk "gopkg.in/check.v1"

"tests/acceptancetests/utils"
. "tests/generated/paging"
)

func Test(t *testing.T) { chk.TestingT(t) }

type PagingGroupSuite struct{}

var _ = chk.Suite(&PagingGroupSuite{})

var pagingClient = getPagingClient()
var clientID = "client-id"

func getPagingClient() PagingClient {
c := NewPagingClient()
c.BaseURI = utils.GetBaseURI()
return c
}

func (s *PagingGroupSuite) TestGetMultiplePages(c *chk.C) {
res, err := pagingClient.GetMultiplePages(clientID, nil, nil)
c.Assert(err, chk.IsNil)
c.Assert(res.NextLink, chk.NotNil)
count := 1
for res.NextLink != nil {
count++
resNext, err := pagingClient.GetMultiplePagesNextResults(res)
c.Assert(err, chk.IsNil)
res = resNext
}
c.Assert(count, chk.Equals, 10)
}

func (s *PagingGroupSuite) TestGetSinglePages(c *chk.C) {
res, err := pagingClient.GetSinglePages()
c.Assert(err, chk.IsNil)
c.Assert(res.NextLink, chk.IsNil)
}

func (s *PagingGroupSuite) TestGetOdataMultiplePages(c *chk.C) {
res, err := pagingClient.GetOdataMultiplePages(clientID, nil, nil)
c.Assert(err, chk.IsNil)
c.Assert(res.OdataNextLink, chk.NotNil)
count := 1
for res.OdataNextLink != nil {
count++
resNext, err := pagingClient.GetOdataMultiplePagesNextResults(res)
c.Assert(err, chk.IsNil)
res = resNext
}
c.Assert(count, chk.Equals, 10)
}

func (s *PagingGroupSuite) TestGetMultiplePagesWithOffset(c *chk.C) {
res, err := pagingClient.GetMultiplePagesWithOffset(100, clientID, nil, nil)
c.Assert(err, chk.IsNil)
c.Assert(res.NextLink, chk.NotNil)
count := 1
for res.NextLink != nil {
count++
resNext, err := pagingClient.GetMultiplePagesWithOffsetNextResults(res)
c.Assert(err, chk.IsNil)
res = resNext
}
c.Assert(count, chk.Equals, 10)
c.Assert(*(*res.Values)[0].Properties.ID, chk.Equals, int32(110))
}

func (s *PagingGroupSuite) TestGetMultiplePagesRetryFirst(c *chk.C) {
res, err := pagingClient.GetMultiplePagesRetryFirst()
c.Assert(err, chk.IsNil)
count := 1
for res.NextLink != nil {
count++
resNext, err := pagingClient.GetMultiplePagesRetryFirstNextResults(res)
c.Assert(err, chk.IsNil)
res = resNext
}
c.Assert(count, chk.Equals, 10)
}

func (s *PagingGroupSuite) TestGetMultiplePagesRetrySecond(c *chk.C) {
res, err := pagingClient.GetMultiplePagesRetrySecond()
c.Assert(err, chk.IsNil)
count := 1
for res.NextLink != nil {
count++
resNext, err := pagingClient.GetMultiplePagesRetrySecondNextResults(res)
c.Assert(err, chk.IsNil)
res = resNext
}
c.Assert(count, chk.Equals, 10)
}

func (s *PagingGroupSuite) TestGetSinglePagesFailure(c *chk.C) {
res, err := pagingClient.GetSinglePagesFailure()
c.Assert(err, chk.NotNil)
c.Assert(res.StatusCode, chk.Equals, http.StatusBadRequest)
}

func (s *PagingGroupSuite) TestGetMultiplePagesFailure(c *chk.C) {
res, err := pagingClient.GetMultiplePagesFailure()
c.Assert(err, chk.IsNil)
c.Assert(res.NextLink, chk.NotNil)
res, err = pagingClient.GetMultiplePagesFailureNextResults(res)
c.Assert(err, chk.NotNil)
c.Assert(res.StatusCode, chk.Equals, http.StatusBadRequest)
}

func (s *PagingGroupSuite) TestGetMultiplePagesFailureURI(c *chk.C) {
res, err := pagingClient.GetMultiplePagesFailureURI()
c.Assert(err, chk.IsNil)
c.Assert(*res.NextLink, chk.Equals, "*&*#&$")
_, err = pagingClient.GetMultiplePagesFailureURINextResults(res)
c.Assert(err, chk.NotNil)
c.Assert(err, chk.ErrorMatches, ".*No scheme detected in URL.*")
}

func (s *PagingGroupSuite) TestGetMultiplePagesFragmentNextLink(c *chk.C) {
res, err := pagingClient.GetMultiplePagesFragmentNextLink("1.6", "test_user")
c.Assert(err, chk.IsNil)
count := 1
for res.OdataNextLink != nil {
count++
resNext, err := pagingClient.NextFragment("1.6", "test_user", *res.OdataNextLink)
c.Assert(err, chk.IsNil)
res = resNext
}
c.Assert(count, chk.Equals, 10)
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,5 @@ func ToDateTime(s string) date.Time {
}

func GetBaseURI() string {
/*
if strings.HasPrefix(baseURI, "https") {
baseURI = "http://localhost:3000"
} else {
baseURI += ":3000"
}
return baseURI
*/
return "http://localhost:3000"
}
25 changes: 20 additions & 5 deletions src/generator/AutoRest.Go.Tests/src/tests/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"tests/acceptancetests/utils"
"tests/generated/azurereport"
"tests/generated/report"
)

Expand All @@ -17,6 +18,7 @@ func main() {
allPass := true
runTests(&allPass)
getReport()
getAzureReport()
server.Kill()
if !allPass {
fmt.Println("Not all tests passed")
Expand Down Expand Up @@ -64,7 +66,7 @@ func runTests(allPass *bool) {
"custombaseurlgroup",
"filegroup",
// "formdatagroup",
}
"paginggroup"}

for _, suite := range testSuites {
fmt.Printf("Run test (go test ./acceptancetests/%vtest -v) ...\n", suite)
Expand All @@ -78,7 +80,7 @@ func runTests(allPass *bool) {
fmt.Printf("Error! %v\n", err)
*allPass = false
}
if stderr.String()[:2] != "OK" {
if len(stderr.String()) >= 2 && stderr.String()[:2] != "OK" {
*allPass = false
}
}
Expand All @@ -90,15 +92,28 @@ func getReport() {
if err != nil {
fmt.Println("Error:", err)
}
printReport(res.Value, "")
}

func getAzureReport() {
var reportClient = azurereport.NewWithBaseURI(utils.GetBaseURI())
res, err := reportClient.GetReport()
if err != nil {
fmt.Println("Error:", err)
}
printReport(res.Value, "Azure")
}

func printReport(res *map[string]*int32, report string) {
count := 0
for key, val := range *res.Value {
for key, val := range *res {
if *val <= 0 {
fmt.Println(key, *val)
count++
}
}
total := len(*res.Value)
total := len(*res)
fmt.Printf("\nReport: Passed(%v) Failed(%v)\n", total-count, count)
fmt.Println("Go Done.......\n")
fmt.Printf("Go %s Done.......\n\n", report)

}
34 changes: 30 additions & 4 deletions src/generator/AutoRest.Go/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using Newtonsoft.Json;

using AutoRest.Core.ClientModel;
using AutoRest.Core.Utilities;
using AutoRest.Go.TemplateModels;
using AutoRest.Extensions.Azure;
using AutoRest.Extensions.Azure.Model;

namespace AutoRest.Go
{
Expand Down Expand Up @@ -380,7 +382,7 @@ private static string GetSeparator(this CollectionFormat format)

public static bool IsClientProperty(this Parameter parameter)
{
return parameter.ClientProperty != null;
return parameter.ClientProperty != null || parameter.SerializedName.IsApiVersion();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please help me understand which bug you are resolving with this updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this line in particular, it is not so much about solving, but classifing APIversion as a Client property. Currently, I don't like that much how APIversion is handled by the generator, but I didn't want to poke that a lot in this PR.

}

public static string GetParameterName(this Parameter parameter)
Expand All @@ -397,7 +399,7 @@ public static bool IsMethodArgument(this Parameter parameter)

public static bool IsApiVersion(this string name)
{
string rgx = @"^api[^a-zA-Z0-9]?version";
string rgx = @"^api[^a-zA-Z0-9_]?version";
return Regex.IsMatch(name, rgx, RegexOptions.IgnoreCase);
}

Expand Down Expand Up @@ -560,7 +562,13 @@ public static string Fields(this CompositeType compositeType)
&& !String.IsNullOrEmpty((compositeType as ModelTemplateModel).NextLink))
{
var nextLinkField = (compositeType as ModelTemplateModel).NextLink;
if (!properties.Any(p => p.Name == nextLinkField))
foreach (Property p in properties) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment explaining why this is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This foreach removes all dots in properties (for code to work), and if it finds the already existing next link, changes its name to how NextLink was added originally (NextLink is a ModelTemplateModel property and it is used in more places, including the razor tamplates)

p.Name = GoCodeNamer.PascalCaseWithoutChar(p.Name, '.');
if (p.Name.Equals(nextLinkField, StringComparison.OrdinalIgnoreCase)) {
p.Name = nextLinkField;
}
}
if (!properties.Any(p => p.Name.Equals(nextLinkField, StringComparison.OrdinalIgnoreCase)))
{
var property = new Property();
property.Name = nextLinkField;
Expand Down Expand Up @@ -983,6 +991,24 @@ public static bool IsPageable(this Method method)
return !string.IsNullOrEmpty(method.NextLink());
}

/// <summary>
/// Checks if method for next page of results on paged methods is already present in the method list.
/// </summary>
/// <param name="method"></param>
/// <param name="methods"></param>
/// <returns></returns>
public static bool NextMethodExists(this Method method, List<Method> methods) {
if (method.Extensions.ContainsKey(AzureExtensions.PageableExtension))
{
var pageableExtension = JsonConvert.DeserializeObject<PageableExtension>(method.Extensions[AzureExtensions.PageableExtension].ToString());
if (pageableExtension != null && !string.IsNullOrWhiteSpace(pageableExtension.OperationName))
{
return methods.Any(m => m.SerializedName.Equals(pageableExtension.OperationName, StringComparison.OrdinalIgnoreCase));
}
}
return false;
}

/// <summary>
/// Check if method has long running extension (x-ms-long-running-operation) enabled.
/// </summary>
Expand Down Expand Up @@ -1022,7 +1048,7 @@ public static string NextLink(this Method method)
var nextLinkName = (string)pageableExtension["nextLinkName"];
if (!string.IsNullOrEmpty(nextLinkName))
{
nextLink = GoCodeNamer.PascalCase(nextLinkName);
nextLink = GoCodeNamer.PascalCaseWithoutChar(nextLinkName, '.');
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, PascalCase is not removing dots from names. I added this function so it removes dots too.
Without it, generating graph client is not very good (there are problems with a property named odata.nextLink)

}
}
Expand Down
Loading