Skip to content

Commit 8333e21

Browse files
mjheilmannmjheilmann
committed
fix bug in grouping, and related tests
1 parent b2302af commit 8333e21

File tree

4 files changed

+46
-160
lines changed

4 files changed

+46
-160
lines changed

‎lib/grpc_reflection/service/state.ex‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ defmodule GrpcReflection.Service.State do
126126
# reference this new file
127127

128128
# Step 1: Collect descriptors to be combined
129-
symbol_files = Enum.map(symbols, &state.symbols[&1])
130-
files_to_combine = state.files |> Map.take(symbol_files) |> Map.values()
129+
symbol_files = Enum.map(symbols, &state_acc.symbols[&1])
130+
files_to_combine = state_acc.files |> Map.take(symbol_files) |> Map.values()
131131

132132
# Step 2: Combine the descriptors
133133
combined_file =
@@ -162,11 +162,11 @@ defmodule GrpcReflection.Service.State do
162162
%{
163163
state_acc
164164
| symbols:
165-
state.symbols
165+
state_acc.symbols
166166
|> Map.drop(symbols)
167167
|> Map.merge(Map.new(symbols, &{&1, cleaned_file.name})),
168168
files:
169-
state.files
169+
state_acc.files
170170
|> Map.drop(symbol_files)
171171
|> Map.new(fn {filename, descriptor} ->
172172
if Enum.any?(descriptor.dependency, &Enum.member?(symbol_files, &1)) do

‎test/integration/v1_reflection_test.exs‎

Lines changed: 21 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,8 @@ defmodule GrpcReflection.V1ReflectionTest do
9090
assert {:ok, response} = run_request(message, ctx)
9191
assert_response(response)
9292

93-
# we pretend all modules are in different files, dependencies are listed
94-
assert response.dependency == [
95-
"helloworld.HelloRequest.proto",
96-
"helloworld.HelloReply.proto"
97-
]
93+
# we pretend each namespace is in a single file, dependencies are listed
94+
assert response.dependency == ["google.protobuf.proto"]
9895
end
9996

10097
test "reject filename that doesn't match a reflection module", ctx do
@@ -104,65 +101,31 @@ defmodule GrpcReflection.V1ReflectionTest do
104101
end
105102

106103
test "get replytype by filename", ctx do
107-
filename = "helloworld.HelloReply.proto"
104+
filename = "helloworld.proto"
108105
message = {:file_by_filename, filename}
109106
assert {:ok, response} = run_request(message, ctx)
110107
assert response.name == filename
111108
assert response.package == "helloworld"
112-
assert response.dependency == ["google.protobuf.Timestamp.proto"]
109+
assert response.dependency == ["google.protobuf.proto"]
113110

114111
assert [
115-
%Google.Protobuf.DescriptorProto{
116-
name: "HelloReply",
117-
field: [
118-
%Google.Protobuf.FieldDescriptorProto{
119-
name: "message",
120-
number: 1,
121-
label: :LABEL_OPTIONAL,
122-
type: :TYPE_STRING,
123-
json_name: "message"
124-
},
125-
%Google.Protobuf.FieldDescriptorProto{
126-
name: "today",
127-
number: 2,
128-
label: :LABEL_OPTIONAL,
129-
type: :TYPE_MESSAGE,
130-
type_name: ".google.protobuf.Timestamp",
131-
json_name: "today"
132-
}
133-
]
134-
}
112+
%Google.Protobuf.DescriptorProto{name: "HelloReply"},
113+
%Google.Protobuf.DescriptorProto{name: "HelloRequest"}
135114
] = response.message_type
136115
end
137116

138117
test "get external by filename", ctx do
139-
filename = "google.protobuf.Timestamp.proto"
118+
filename = "google.protobuf.proto"
140119
message = {:file_by_filename, filename}
141120
assert {:ok, response} = run_request(message, ctx)
142121
assert response.name == filename
143122
assert response.package == "google.protobuf"
144123
assert response.dependency == []
145124

146125
assert [
147-
%Google.Protobuf.DescriptorProto{
148-
field: [
149-
%Google.Protobuf.FieldDescriptorProto{
150-
json_name: "seconds",
151-
label: :LABEL_OPTIONAL,
152-
name: "seconds",
153-
number: 1,
154-
type: :TYPE_INT64
155-
},
156-
%Google.Protobuf.FieldDescriptorProto{
157-
json_name: "nanos",
158-
label: :LABEL_OPTIONAL,
159-
name: "nanos",
160-
number: 2,
161-
type: :TYPE_INT32
162-
}
163-
],
164-
name: "Timestamp"
165-
}
126+
%Google.Protobuf.DescriptorProto{name: "Any"},
127+
%Google.Protobuf.DescriptorProto{name: "StringValue"},
128+
%Google.Protobuf.DescriptorProto{name: "Timestamp"}
166129
] = response.message_type
167130
end
168131

@@ -173,11 +136,7 @@ defmodule GrpcReflection.V1ReflectionTest do
173136
assert response.name == filename
174137
assert response.package == "testserviceV3"
175138

176-
assert response.dependency == [
177-
"google.protobuf.Timestamp.proto",
178-
"google.protobuf.StringValue.proto",
179-
"google.protobuf.Any.proto"
180-
]
139+
assert response.dependency == ["google.protobuf.proto"]
181140
end
182141

183142
test "ensure exclusion of nested types in file descriptor dependencies", ctx do
@@ -187,11 +146,7 @@ defmodule GrpcReflection.V1ReflectionTest do
187146
assert response.name == filename
188147
assert response.package == "testserviceV3"
189148

190-
assert response.dependency == [
191-
"google.protobuf.Timestamp.proto",
192-
"google.protobuf.StringValue.proto",
193-
"google.protobuf.Any.proto"
194-
]
149+
assert response.dependency == ["google.protobuf.proto"]
195150
end
196151
end
197152

@@ -214,49 +169,18 @@ defmodule GrpcReflection.V1ReflectionTest do
214169
assert {:ok, response} = run_request(message, ctx)
215170
assert response.name == extendee <> "Extension.proto"
216171
assert response.package == "testserviceV2"
217-
assert response.dependency == [extendee <> ".proto"]
218-
219-
assert response.extension == [
220-
%Google.Protobuf.FieldDescriptorProto{
221-
name: "data",
222-
extendee: extendee,
223-
number: 10,
224-
label: :LABEL_OPTIONAL,
225-
type: :TYPE_STRING,
226-
type_name: nil
227-
},
228-
%Google.Protobuf.FieldDescriptorProto{
229-
name: "location",
230-
extendee: extendee,
231-
number: 11,
232-
label: :LABEL_OPTIONAL,
233-
type: :TYPE_MESSAGE,
234-
type_name: "testserviceV2.Location"
235-
}
236-
]
172+
assert response.dependency == ["testserviceV2.proto"]
173+
174+
assert [
175+
%Google.Protobuf.FieldDescriptorProto{name: "data"},
176+
%Google.Protobuf.FieldDescriptorProto{name: "location"}
177+
] = response.extension
237178

238-
assert response.message_type == [
179+
assert [
239180
%Google.Protobuf.DescriptorProto{
240-
name: "Location",
241-
field: [
242-
%Google.Protobuf.FieldDescriptorProto{
243-
name: "latitude",
244-
number: 1,
245-
label: :LABEL_OPTIONAL,
246-
type: :TYPE_DOUBLE,
247-
json_name: "latitude"
248-
},
249-
%Google.Protobuf.FieldDescriptorProto{
250-
name: "longitude",
251-
extendee: nil,
252-
number: 2,
253-
label: :LABEL_OPTIONAL,
254-
type: :TYPE_DOUBLE,
255-
json_name: "longitude"
256-
}
257-
]
181+
name: "Location"
258182
}
259-
]
183+
] = response.message_type
260184
end
261185
end
262186
end

‎test/integration/v1alpha_reflection_test.exs‎

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,7 @@ defmodule GrpcReflection.V1alphaReflectionTest do
9191
assert_response(response)
9292

9393
# we pretend all modules are in different files, dependencies are listed
94-
assert response.dependency == [
95-
"helloworld.HelloRequest.proto",
96-
"helloworld.HelloReply.proto"
97-
]
94+
assert response.dependency == ["google.protobuf.proto"]
9895
end
9996

10097
test "reject filename that doesn't match a reflection module", ctx do
@@ -104,65 +101,42 @@ defmodule GrpcReflection.V1alphaReflectionTest do
104101
end
105102

106103
test "get replytype by filename", ctx do
107-
filename = "helloworld.HelloReply.proto"
104+
filename = "helloworld.proto"
108105
message = {:file_by_filename, filename}
109106
assert {:ok, response} = run_request(message, ctx)
110107
assert response.name == filename
111108
assert response.package == "helloworld"
112-
assert response.dependency == ["google.protobuf.Timestamp.proto"]
109+
assert response.dependency == ["google.protobuf.proto"]
113110

114111
assert [
115112
%Google.Protobuf.DescriptorProto{
116113
name: "HelloReply",
117114
field: [
118-
%Google.Protobuf.FieldDescriptorProto{
119-
name: "message",
120-
number: 1,
121-
label: :LABEL_OPTIONAL,
122-
type: :TYPE_STRING,
123-
json_name: "message"
124-
},
125-
%Google.Protobuf.FieldDescriptorProto{
126-
name: "today",
127-
number: 2,
128-
label: :LABEL_OPTIONAL,
129-
type: :TYPE_MESSAGE,
130-
type_name: ".google.protobuf.Timestamp",
131-
json_name: "today"
132-
}
115+
%Google.Protobuf.FieldDescriptorProto{name: "message"},
116+
%Google.Protobuf.FieldDescriptorProto{name: "today"}
117+
]
118+
},
119+
%Google.Protobuf.DescriptorProto{
120+
name: "HelloRequest",
121+
field: [
122+
%Google.Protobuf.FieldDescriptorProto{name: "name"}
133123
]
134124
}
135125
] = response.message_type
136126
end
137127

138128
test "get external by filename", ctx do
139-
filename = "google.protobuf.Timestamp.proto"
129+
filename = "google.protobuf.proto"
140130
message = {:file_by_filename, filename}
141131
assert {:ok, response} = run_request(message, ctx)
142132
assert response.name == filename
143133
assert response.package == "google.protobuf"
144134
assert response.dependency == []
145135

146136
assert [
147-
%Google.Protobuf.DescriptorProto{
148-
field: [
149-
%Google.Protobuf.FieldDescriptorProto{
150-
json_name: "seconds",
151-
label: :LABEL_OPTIONAL,
152-
name: "seconds",
153-
number: 1,
154-
type: :TYPE_INT64
155-
},
156-
%Google.Protobuf.FieldDescriptorProto{
157-
json_name: "nanos",
158-
label: :LABEL_OPTIONAL,
159-
name: "nanos",
160-
number: 2,
161-
type: :TYPE_INT32
162-
}
163-
],
164-
name: "Timestamp"
165-
}
137+
%Google.Protobuf.DescriptorProto{name: "Any"},
138+
%Google.Protobuf.DescriptorProto{name: "StringValue"},
139+
%Google.Protobuf.DescriptorProto{name: "Timestamp"}
166140
] = response.message_type
167141
end
168142

@@ -173,11 +147,7 @@ defmodule GrpcReflection.V1alphaReflectionTest do
173147
assert response.name == filename
174148
assert response.package == "testserviceV3"
175149

176-
assert response.dependency == [
177-
"google.protobuf.Timestamp.proto",
178-
"google.protobuf.StringValue.proto",
179-
"google.protobuf.Any.proto"
180-
]
150+
assert response.dependency == ["google.protobuf.proto"]
181151
end
182152

183153
test "ensure exclusion of nested types in file descriptor dependencies", ctx do
@@ -187,11 +157,7 @@ defmodule GrpcReflection.V1alphaReflectionTest do
187157
assert response.name == filename
188158
assert response.package == "testserviceV3"
189159

190-
assert response.dependency == [
191-
"google.protobuf.Timestamp.proto",
192-
"google.protobuf.StringValue.proto",
193-
"google.protobuf.Any.proto"
194-
]
160+
assert response.dependency == ["google.protobuf.proto"]
195161
end
196162
end
197163

@@ -217,7 +183,7 @@ defmodule GrpcReflection.V1alphaReflectionTest do
217183
assert {:ok, response} = run_request(message, ctx)
218184
assert response.name == extendee <> "Extension.proto"
219185
assert response.package == "testserviceV2"
220-
assert response.dependency == [extendee <> ".proto"]
186+
assert response.dependency == ["testserviceV2.proto"]
221187

222188
assert response.extension == [
223189
%Google.Protobuf.FieldDescriptorProto{

‎test/service/builder_test.exs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ defmodule GrpcReflection.Service.BuilderTest do
1111
assert %State{services: [TestserviceV3.TestService.Service]} = tree
1212

1313
assert Map.keys(tree.files) == [
14-
"google.protobuf.Any.proto",
15-
"google.protobuf.StringValue.proto",
16-
"google.protobuf.Timestamp.proto",
14+
"google.protobuf.proto",
1715
"testserviceV3.proto"
1816
]
1917

@@ -42,8 +40,7 @@ defmodule GrpcReflection.Service.BuilderTest do
4240
assert %State{services: [TestserviceV2.TestService.Service]} = tree
4341

4442
assert Map.keys(tree.files) == [
45-
"google.protobuf.Any.proto",
46-
"google.protobuf.Timestamp.proto",
43+
"google.protobuf.proto",
4744
"testserviceV2.TestRequestExtension.proto",
4845
"testserviceV2.proto"
4946
]
@@ -101,8 +98,7 @@ defmodule GrpcReflection.Service.BuilderTest do
10198
names = Enum.map(Map.values(tree.files), & &1.name)
10299

103100
assert names == [
104-
"google.protobuf.Any.proto",
105-
"google.protobuf.Timestamp.proto",
101+
"google.protobuf.proto",
106102
"testserviceV2.TestRequestExtension.proto",
107103
"testserviceV2.proto"
108104
]

0 commit comments

Comments
 (0)