This addresses issue #1008, by creating a JsonFormatter which is private and only different
to JsonFormatter.Default in terms of reference equality.
Other plausible designs:
- The same, but expose the diagnostic-only formatter
- Add something to settings to say "I don't have a type registry at all"
- Change the behaviour of JsonFormatter.Default (bad idea IMO, as we really *don't* want the result of this used as regular JSON to be parsed)
Note that just trying to find a separate fix to issue #933 and using that to override Any.ToString() differently wouldn't work for messages that *contain* an Any.
Generated code changes follow in the next commit.
This required a rework of the tokenizer to allow for a "replaying" tokenizer, basically in case the @type value comes after the data itself. This rework is nice in some ways (all the pushback and object depth logic in one place) but is a little fragile in terms of token push-back when using the replay tokenizer. It'll be fine for the scenario we need it for, but we should be careful...
There are corner cases where MessageDescriptor.{ClrType,Parser} will return null, and these are now documented. However, normally they *should* be implemented, even for descriptors of for dynamic messages. Ditto FieldDescriptor.Accessor.
We'll still need a fair amount of work to implement dynamic messages, but this change means that the public API will be remain intact.
Additionally, this change starts making use of C# 6 features in the files that it touches. This is far from exhaustive, and later PRs will have more.
Generated code changes coming in the next commit.
Generated code coming in next commit - in a subsequent PR I want to do a bit of renaming and redocumenting around this, in anticipation of DynamicMessage.
This is only thrown directly by JsonTokenizer, but surfaces from JsonParser as well. I've added doc comments to hopefully make everything clear.
The exception is actually thrown by the reader within JsonTokenizer, in anticipation of keeping track of the location within the document, but that change is not within this PR.
This includes all the well-known types except Any.
Some aspects are likely to require further work when the details of the JSON parsing expectations are hammered out in more detail. Some of these have "ignored" tests already.
Note that the choice *not* to use Json.NET was made for two reasons:
- Going from 0 dependencies to 1 dependency is a big hit, and there's not much benefit here
- Json.NET parses more leniently than we'd want; accommodating that would be nearly as much work as writing the tokenizer
This only really affects the JsonTokenizer, which could be replaced by Json.NET. The JsonParser code would be about the same length with Json.NET... but I wouldn't be as confident in it.
This changes how we approach JSON formatting in general - instead of looking at the field a value came from, we just look at the type of the value. It's possible this *could* be slightly inefficient, but if we start caring about JSON performance deeply, we'll probably want to rewrite all of this anyway. It's definitely simpler this way.
When we support dynamic messages, we'll need to modify JsonFormatter to handle enum values, as they won't come be "real" .NET enums at that point. It shouldn't be hard to do though.
There are now summaries for:
- The Types nested class (which holds nested types)
- The file descriptor class for each proto
- The enum generated for each oneof
(Also fixed two typos.)
Generated code in next commit.
We still need the JSON representation, which relies on something like a DescriptorPool to fetch message types from based on the type URL. That will come a bit later.
(The DescriptorPool comment in this commit is just a note which will prove useful if we use DescriptorPool itself.)
This introduces a new C# option, base_namespace.
If the option is not specified, the behaviour is as before: no directories are generated.
If the option *is* specified, all C# namespaces must be relative to the base namespace, and the directories are generated relative to that namespace.
Example:
- Any.proto declares csharp_namespace = "Google.Protobuf.WellKnownTypes"
- We build with --csharp_out=Google.Protobuf --csharp_opt=base_namespace=Google.Protobuf
- The Any.cs file is generated in Google.Protobuf/WellKnownTypes (where it currently lives)
We need a change to descriptor.proto before this will all work (it wasn't in the right C# namespace) but that needs the other descriptors to be regenerated too. See next commit...
We now do this in protoc instead of the generation simpler.
Benefits:
- Generation script is simpler
- Detection is simpler as we now only need to care about one filename
- The embedded descriptor knows itself as "google/protobuf/descriptor.proto" avoiding dependency issues
This PR also makes the "invalid dependency" exception clearer in terms of expected and actual dependencies.
With this in place, generating APIs on github.com/google/googleapis works - previously annotations.proto failed.
Currently there's no access to the annotations (stored as extensions) but we could potentially expose those at a later date.
- Removed a TODO without change in DescriptorPool.LookupSymbol - the TODOs were around performance, and this is only used during descriptor initialization
- Make the CodedInputStream limits read-only, adding a static factory method for the rare cases when this is useful
- Extracted IDeepCloneable into its own file.
This is a bit of a grotty hack, as we need to sort of fake proto2 field presence, but with only a proto3 version of the descriptor messages (a bit like oneof detection).
Should be okay, but will need to be careful of this if we ever implement proto2.
Now the generated code doesn't need to check for end group tags, as it will skip whole groups at a time.
Currently it will ignore extraneous end group tags, which may or may not be a good thing.
Renamed ConsumeLastField to SkipLastField as it felt more natural.
Removed WireFormat.IsEndGroupTag as it's no longer useful.
This mostly fixes issue 688.
(Generated code changes coming in next commit.)
This is taking an approach of putting all the logic in JsonFormatter. That's helpful in terms of concealing the details of whether or not to wrap the value in quotes, but it does lack flexibility. I don't *think* we want to allow user-defined formatting of messages, so that much shouldn't be a problem.
While I've provided operators, I haven't yet provided the method equivalents. It's not clear to me that
they're actually a good idea, while we're really targeting C# developers who definitely *can* use the user-defined operators.
Additionally, change it to return the value passed, and make it generic with a class constraint.
A separate method doesn't have the class constraint, for more unusual scenarios.
- Fix nupec paths
- Remove an obsolete part of the JSON build
- Add documentation and tests to reflection extension methods, and improve implementations
This requires .NET 4.5, and there are a few compatibility changes required around reflection.
Creating a PR from this to see how our CI systems handle it. Will want to add more documentation,
validation and probably tests before merging.
This is in aid of issue #590.
I think Jan was actually suggesting keeping both, but that feels redundant to me. The test diff is misleading here IMO, because I wouldn't expect real code using reflection to use several accessors one after another like this, unless it was within a loop. Evidence to the contrary would be welcome :)
This change also incidentally goes part way to fixing the issue of the JSON formatter not writing out the fields in field number order - with this change, it does except for oneofs, which we can fix in a follow-up change.
I haven't actually added a test with a message with fields deliberately out of order - I'm happy to do so though. It feels like it would make sense to be in google/src/protobuf, but it's not entirely clear what the rules of engagement are for adding new messages there. (unittest_proto3.proto?)
This is definitely not ready to ship - I'm "troubled" by the disconnect between a list of fields in declaration order, and a mapping of field accessors by field number/name. Discussion required, but I find that easier when we've got code to look at :)
Changes in brief:
1. Descriptor is now the entry point for all reflection.
2. IReflectedMessage has gone; there's now a Descriptor property in IMessage, which is explicitly implemented (due to the static property).
3. FieldAccessorTable has gone away
4. IFieldAccessor and OneofFieldAccessor still exist; we *could* put the functionality straight into FieldDescriptor and OneofDescriptor... I'm unsure about that.
5. There's a temporary property MessageDescriptor.FieldAccessorsByFieldNumber to make the test changes small - we probably want this to go away
6. Discovery for delegates is now via attributes applied to properties and the Clear method of a oneof
I'm happy with 1-3.
4 I'm unsure about - feedback welcome.
5 will go away
6 I'm unsure about, both in design and implementation. Should we have a ProtobufMessageAttribute too? Should we find all the relevant attributes in MessageDescriptor and pass them down, to avoid an O(N^2) scenario?
Generated code changes coming in the next commit.
- We do still generate the message types, as otherwise reflection breaks, even though it doesn't actually use those types.
- JSON handling hasn't been implemented yet
We don't use it in the runtime or generated code anywhere now, so the extra small performance boost isn't as critical, and it has some undesirable consequences.
The tests have needed to change as iterator block enumerators don't throw when we might expect them to.
This involves:
- Specifying a namespace in each proto (including ones we'd previously missed)
- Updating the generation script
- Changing codegen to implement IReflectedMessage.Fields explicitly (a good thing anyway)
- Changing reflection tests to take account of the explicit interface implementation
Non-generated code in this commit; generated code to follow
Change the C# namespace in descriptor.proto to Google.Protobuf.Reflection.
This then means changing where the generated code lives, which means updating the project file...
It also involves regenerating the C++ - which has updated the well-known types as well,
for no terribly obvious reason...
- The protos are no longer publicly exposed at all
- Oneof detection now works (as we default to -1, not 0)
- OneofDescriptor exposes the fields in the oneof
- Removed unnecessary code for replacing protos - remnant of extensions
- There's now just the non-generic form of IDescriptor
Note that now we need a proto3 version of addressbook.proto. This may affect other platforms, and could do with an overhaul to follow proto3 conventions anyway (e.g. repeated field names). Will need to think about that carefully before merging into master. Raised issue #565 for this.
- FieldAccessorTable is now non-generic
- We don't have a static field per message type in the umbrella class. (Message descriptors are accessed via the file descriptor.)
- Removed the "descriptor assigner" complication from the descriptor fixup; without extensions, we don't need it
- MapField implements IDictionary (more tests would be good...)
- RepeatedField implements IList (more tests would be good)
- Use expression trees to build accessors. (Will need to test this on various platforms... probably need a fallback strategy just using reflection directly.)
- Added FieldDescriptor.IsMap
- Added tests for reflection with generated messages
Changes to generated code coming in next commit.
- Added new line at the end of SampleEnum
- Moved GeneratedMessageTest.GetSampleMessage to a new class, SampleMessages, and renamed it to CreateFullTestAllTypes.
