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

Objective-C and Any Datatype #1674

Closed
woigl opened this issue Jun 12, 2016 · 17 comments
Closed

Objective-C and Any Datatype #1674

woigl opened this issue Jun 12, 2016 · 17 comments
Assignees
Milestone

Comments

@woigl
Copy link

woigl commented Jun 12, 2016

I can not find anything about packing and unpacking of Any datatype in objective-c.

Is this implemented and how can I use it?

@thomasvl
Copy link
Contributor

There currently aren't any helpers for it. You can manually inspect the type information to decide if it is a package/message you can support, and then use the bytes in the NSData to call one of the parse methods.

@woigl
Copy link
Author

woigl commented Jun 15, 2016

I already wrote myself an extension in Swift for the GPBAny, which has Is<>() as pall as UnpackFrom(...) and PackTo(...).

Would you like me to re-write it in Objective-C and create a pull request?

@thomasvl
Copy link
Contributor

Sure, it was something we were hoping to look at doing, just hasn't bubbled up to the top of the queue yet. Adding a category to GPBWellKnownTypes was what we were thinking.

@woigl
Copy link
Author

woigl commented Jun 15, 2016

This is my current swift extension. Maybe you could have a look at it. Let me know your opinion.

//
//  AnyExtension.swift
//  TestAppRestClient1
//
//  Created by Stefan Walter on 14/06/16.
//  Copyright © 2016 Stefan Walter. All rights reserved.
//

import Foundation

extension GPBAny {

    public func Is<T:GPBMessage>(compareWith: T) -> Bool {
        return internalIs(compareWith.descriptor())
    }

    public func packFrom(message: GPBMessage) {
        packFrom(message, typeUrlPrefix: "type.googleapis.com/")
    }

    public func packFrom(message: GPBMessage, typeUrlPrefix: String) {
        typeURL = getTypeUrl(message.descriptor(), typeUrlPrefix: typeUrlPrefix)
        value = message.data()
    }

    public func unpackTo<T:GPBMessage>() -> T? {
        if (!internalIs(T.descriptor())) {
            return nil
        }

        if let temporary: T = T.parseFromData(value, error: nil) {
            return temporary
        }

        return nil
    }

    private func getTypeUrl(descriptor: GPBDescriptor, typeUrlPrefix: String) -> String {
        return typeUrlPrefix + getFullDescriptorName(descriptor)
    }

    private func getFullDescriptorName(descriptor: GPBDescriptor) -> String {
        return descriptor.file.package + "." + descriptor.name
    }

    private func internalIs(descriptor: GPBDescriptor) -> Bool {
        if let fullName = parseAnyTypeUrl(typeURL) {
            return (fullName == getFullDescriptorName(descriptor))
        } else {
            return false
        }
    }

    private func parseAnyTypeUrl(typeUrl: String) -> String? {
        let components = typeUrl.componentsSeparatedByString("/")

        if (components.isEmpty || components[components.count-1].characters.count==0) {
            return nil;
        }

        return components[components.count-1]
    }
}

@thomasvl
Copy link
Contributor

On quick look, I think the issue is going to the the type names. Since ObjC classes can get prefixes via the file option, the name currently in the descriptor doesn't always line up with what your would be using for type urls. My original thought was we'll likely need to expose some way to provide a registry to map the objc classes to the full type urls to work around it.

@woigl
Copy link
Author

woigl commented Jun 15, 2016

Well, but why can't we use in the objective-c implementation the same logic as in the c/c++ implementation? They are using descriptor->full_name().

@thomasvl
Copy link
Contributor

C++ captures the full proto definition as binary data, that's part of why the code foot print is larger. ObjC's "Descriptors" are just what the runtime needs to function. They don't capture everything the C++ one does.

@woigl
Copy link
Author

woigl commented Jun 15, 2016

Yes, I understand. But I think that parts of it would be sufficient (i.e. full-name).

@thomasvl
Copy link
Contributor

Yup, that's the constant balancing act. It would be nice to have it for Any, but for apps with hundreds of protos, the majority likely won't need the data, so it becomes bloat (and it ends up in the armv7 and arm64 segments, so it is duplicated).

@woigl
Copy link
Author

woigl commented Jun 15, 2016

Well, it is a again a valid point, but restricts the usage of protocol buffer more an more. Why can't that be a protoc compiler flag, which would define if the full name is set or not in the generated classes. That would allow the user if the support of Any should work fine or not.

@thomasvl
Copy link
Contributor

The catch is cascading protos. If you depend on protos from someone else, they might not generate with the option enabled.

Some of the other language Lite runtimes run into similar problems (not having a descriptor). So they are also looking at doing Any support by providing a registry to the calls to provide the mappings. So I was hoping we might be able to end up some what consistent with the other languages in similar situations.

@woigl
Copy link
Author

woigl commented Jun 15, 2016

Okay, but what time frame would that consume to get it done?

@thomasvl
Copy link
Contributor

I don't think we have a specific time table. Your extension sounds like it works fine for you for now. But since it relies on the messages not getting prefixed, it wouldn't be a fit for general use in the library. As covered here, there's a few possible ways to deal with it, but the correct decision is clear yet. I'm currently focused on #1457 because it is blocking a few folks from being able to build in general. But I'll try to get some status on the other languages facing this so see if we can reach a consensus and sort out what to do for ObjC.

@woigl
Copy link
Author

woigl commented Jun 15, 2016

Yes, it works fine for me at the moment. I'm already happy if my request would make it on the roadmap 👍

@thomasvl thomasvl self-assigned this Jun 15, 2016
@thomasvl
Copy link
Contributor

thomasvl commented Aug 9, 2016

Here's the current proposal, hopefully I'll be able to start crank this out this week to get it into a PR.

Background

C++: https://github.com/google/protobuf/blob/master/src/google/protobuf/any.cc
Java: https://github.com/google/protobuf/blob/master/src/google/protobuf/compiler/java/java_message.cc#L482 Based on it being an Any message they generate helpers directly.

Public API Additions

GPBWellKnownTypes

/** NSError domain used for errors. */
extern NSString *const GPBWellKnownTypesErrorDomain;

/** Error code for NSError with GPBWellKnownTypesErrorDomain. */
typedef NS_ENUM(NSInteger, GPBWellKnownTypesErrorCode) {
  /**
   * The type_url could not be computed for the requested
   * GPBMessage class.
   */
  GPBWellKnownTypesErrorCodeUnknownTypeURL = -100,
  /**
   * The type_url in the Any doesn’t match that of the requested
   * GPBMessage class.
   */
  GPBWellKnownTypesErrorCodeTypeURLMismatch = -101,
};

@interface GPBAny(GPBWellKnownTypes)

/**
 * Serializes message and returns a new instance to contain the
 * info. Uses the type.googleapis.com/ type_url prefix. On
 * failure returns nil and fills in the optional outError
 * with the details. The error can be a GPBWellKnownTypesErrorDomain
 * error or a general message serialization error.
 */
+ (nullable instancetype)anyWithMessage:(nonnull GPBMessage *)message
                                  error:(nullable NSError **)outError;

/**
 * Serializes message and returns a new instance to contain the
 * info. On failure returns nil and fills in the optional outError
 * with the details. The error can be a GPBWellKnownTypesErrorDomain
 * error or a general message serialization error.
 */
+ (nullable instancetype)anyWithMessage:(nonnull GPBMessage *)message
                          typeURLprefix:(nonnull NSString *)typeURLPrefix
                                  error:(nullable NSError **)outError;

/**
 * Serializes message and initializes the instance to contain the
 * info. Uses the type.googleapis.com/ type_url prefix.  On
 * failure returns nil and fills in the optional outError
 * with the details. The error can be a GPBWellKnownTypesErrorDomain
 * error or a general message serialization error.
 */
- (nullable instancetype)initWithMessage:(nonnull GPBMessage *)message
                                   error:(nullable NSError **)outError;

/**
 * Serializes message and initializes the instance to contain the
 * info. On failure returns nil and fills in the optional outError
 * with the details. The error can be a GPBWellKnownTypesErrorDomain
 * error or a general message serialization error.
 */
- (nullable instancetype)initWithMessage:(nonnull GPBMessage *)message
                           typeURLprefix:(nonnull NSString *)typeURLPrefix
                                   error:(nullable NSError **)outError;

/**
 * Serializes message and sets the current instance to contain the
 * info. Uses the type.googleapis.com/ type_url prefix. Returns
 * success/failure, and on failure fills in the optional outError
 * with the details. The error can be a GPBWellKnownTypesErrorDomain
 * error or a general message serialization error.
 */
- (BOOL)packWithMessage:(nonnull GPBMessage *)message
                  error:(nullable NSError **)outError;

/**
 * Serializes message and sets the current instance to contain the
 * info. Returns success/failure, and on failure fills in the
 * optional outError the the details. The error can be a
 * GPBWellKnownTypesErrorDomain error or a general message
 * serialization error (see GPBMessage).
 */
- (BOOL)packWithMessage:(nonnull GPBMessage *)message
          typeURLprefix:(nonnull NSString *)typeURLPrefix
                  error:(nullable NSError **)outError;

/**
 * Checks if the type_url matches the requested class and then
 * parses the data returning the message on success. On failure
 * nil is returned and the optional outError will be filled in;
 * the error could be a GPBWellKnownTypesErrorDomain error or a
 * general message parsing error (see GPBMessage and
 * GPBCodedInputStream).
 */
- (nullable GPBMessage *)unpackMessageClass:(nonnull Class *)messageClass
                          error:(nullable NSError **)outError;

GPBDescriptor Class

/**
 * Returns the full name of this message class (package.message).
 * Can return nil if the runtime is unable to compute the value
 * for some reason.
 */
@property (readonly, nullable) NSString *fullName;

/**
 * If this message was nested under another message, it will return
 * that containing descriptor.
 */
@property (readonly, nullable) GPBDescriptor *containingType;

Implementation Notes

Generation Time

The plugin will:

  • Record the prefix used for the given package within the GPBFileDescriptor. Note that this
    won’t be exposed in the public API of the GPBFileDescriptor at this time.
  • If a message class has a parent message class, it will track the name of the parent class.

Runtime Support

GPBAny WellKnownTypes Category

These implementations will follow what Java and C++ do, the host is ignored when unpacking. The package.message part of type_url can be checked via the new methods on GPB*Descriptors.

GPBFileDescriptor

Private method to get the objc prefix recorded at generation.

GPBDescriptor

  • Since containment isn’t common, it doesn’t seem worth using an ivar in the descriptor, so the
    class name will get resolved and the descriptor will use objc_[gs]etAssociatedObject to track
    the relationship.
  • -fullName will use -file & -containingType to compute the string.
  • -containingTypes lets it find the class name to strip off the front of it’s name (and the
    underscore joining them)
  • -file lets it find the prefix for this package to confirm it exists on the class name and strip it.

APIs Deferred/Not Being Done

  • Some languages have extended their TextFormat generation to decode an Any and print the
    content. To support that we’d have to build a registry of some kind for all GPBMessage
    subclasses to do the lookup. We are not doing this since the cost there depends on the total
    number of ObjC classes (not just Message classes) which could be very expensive.
  • With the new information captured, there may now be enough support to generate correct
    TextFormat output for extensions. That isn’t being examined at this time.

@thomasvl thomasvl added this to the v3.1.0 milestone Aug 17, 2016
@TeBoring
Copy link
Contributor

What's the status of this now?

@thomasvl
Copy link
Contributor

I've got a draft, but it will cause a incompatibility with the current generated sources, so I've targeted this at 3.1 where that would be allowed.

thomasvl added a commit to thomasvl/protobuf that referenced this issue Sep 8, 2016
- Capture the ObjC prefix used when generating the the file.
- Track the containing type on descriptors.
- Mark descriptors where the message class name got a suffix added to it.
- Expose a fullName property on Descriptors.
- Add helpers for packing/unpacking Any messages.
- Bump the ObjC runtime version number. Since we added methods and invoke them
  in the generated code, ensure the code is running against a matching version.
  Otherwise, someone could compile against headers, but run with a framework
  that is older and get unknown selector failures.  This should trip clearer
  messaging.

Fixes protocolbuffers#1674
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants