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

Use Uint8List for bytes fields #729

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
9 changes: 4 additions & 5 deletions protobuf/lib/src/protobuf/coded_buffer_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,10 @@ class CodedBufferWriter {
_writeVarint32(value ? 1 : 0);
break;
case PbFieldType._BYTES_BIT:
_writeBytesNoTag(
value is TypedData ? value : Uint8List.fromList(value));
_writeBytesNoTag(value);
break;
case PbFieldType._STRING_BIT:
_writeBytesNoTag(_utf8.encode(value));
_writeBytesNoTag(_utf8.encode(value) as dynamic);
break;
case PbFieldType._DOUBLE_BIT:
_writeDouble(value);
Expand Down Expand Up @@ -396,8 +395,8 @@ class CodedBufferWriter {
}
}

void _writeBytesNoTag(dynamic value) {
writeInt32NoTag(value.length);
void _writeBytesNoTag(Uint8List value) {
writeInt32NoTag(value.lengthInBytes);
writeRawBytes(value);
}

Expand Down
2 changes: 1 addition & 1 deletion protobuf/lib/src/protobuf/field_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class PbFieldType {

// Closures commonly used by initializers.
static String _STRING_EMPTY() => '';
static List<int> _BYTES_EMPTY() => <int>[];
static Uint8List _BYTES_EMPTY() => Uint8List(0);
static bool _BOOL_FALSE() => false;
static int _INT_ZERO() => 0;
static double _DOUBLE_ZERO() => 0.0;
Expand Down
2 changes: 1 addition & 1 deletion protobuf/lib/src/protobuf/generated_message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ abstract class GeneratedMessage {

/// For generated code only.
/// @nodoc
void $_setBytes(int index, List<int> value) => _fieldSet._$set(index, value);
void $_setBytes(int index, Uint8List value) => _fieldSet._$set(index, value);

/// For generated code only.
/// @nodoc
Expand Down
9 changes: 5 additions & 4 deletions protobuf/lib/src/protobuf/mixins/well_known.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:convert';
import 'dart:typed_data';

import 'package:fixnum/fixnum.dart';

Expand All @@ -12,8 +13,8 @@ import '../json_parsing_context.dart';
abstract class AnyMixin implements GeneratedMessage {
String get typeUrl;
set typeUrl(String value);
List<int> get value;
set value(List<int> value);
Uint8List get value;
set value(Uint8List value);

/// Returns `true` if the encoded message matches the type of [instance].
///
Expand Down Expand Up @@ -713,8 +714,8 @@ abstract class StringValueMixin {
}

abstract class BytesValueMixin {
List<int> get value;
set value(List<int> value);
Uint8List get value;
set value(Uint8List value);

// From google/protobuf/wrappers.proto:
// The JSON representation for `BytesValue` is JSON string.
Expand Down
8 changes: 2 additions & 6 deletions protoc_plugin/lib/src/base_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,8 @@ class BaseType {
return const BaseType._raw(FieldDescriptorProto_Type.TYPE_STRING, 'S',
'$coreImportPrefix.String', r'$_setString', null);
case FieldDescriptorProto_Type.TYPE_BYTES:
return const BaseType._raw(
FieldDescriptorProto_Type.TYPE_BYTES,
'Y',
'$coreImportPrefix.List<$coreImportPrefix.int>',
r'$_setBytes',
null);
return const BaseType._raw(FieldDescriptorProto_Type.TYPE_BYTES, 'Y',
'$_typedDataImportPrefix.Uint8List', r'$_setBytes', null);

case FieldDescriptorProto_Type.TYPE_GROUP:
constSuffix = 'G';
Expand Down
5 changes: 5 additions & 0 deletions protoc_plugin/lib/src/extension_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ class ExtensionGenerator {
return _field.needsFixnumImport;
}

bool get needsTypedDataImport {
if (!_resolved) throw StateError('resolve not called');
return _field.needsTypedDataImport;
}

/// Adds dependencies of [generate] to [imports].
///
/// For each .pb.dart file that the generated code needs to import,
Expand Down
15 changes: 15 additions & 0 deletions protoc_plugin/lib/src/file_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,11 @@ class FileGenerator extends ProtobufContainer {
}

out.println(_coreImport);

if (_needsTypedDataImport) {
out.println(_typedDataImport);
}

out.println();

if (_needsFixnumImport) {
Expand Down Expand Up @@ -379,6 +384,16 @@ class FileGenerator extends ProtobufContainer {
return false;
}

bool get _needsTypedDataImport {
for (var m in messageGenerators) {
if (m.needsTypedDataImport) return true;
}
for (var x in extensionGenerators) {
if (x.needsTypedDataImport) return true;
}
return false;
}

bool get _needsProtobufImport =>
messageGenerators.isNotEmpty ||
extensionGenerators.isNotEmpty ||
Expand Down
5 changes: 3 additions & 2 deletions protoc_plugin/lib/src/generated/descriptor.pb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// ignore_for_file: annotate_overrides,camel_case_types,constant_identifier_names,directives_ordering,library_prefixes,non_constant_identifier_names,prefer_final_fields,return_of_invalid_type,unnecessary_const,unnecessary_import,unnecessary_this,unused_import,unused_shown_name

import 'dart:core' as $core;
import 'dart:typed_data' as $typed_data;

import 'package:fixnum/fixnum.dart' as $fixnum;
import 'package:protobuf/protobuf.dart' as $pb;
Expand Down Expand Up @@ -2765,9 +2766,9 @@ class UninterpretedOption extends $pb.GeneratedMessage {
void clearDoubleValue() => clearField(6);

@$pb.TagNumber(7)
$core.List<$core.int> get stringValue => $_getN(5);
$typed_data.Uint8List get stringValue => $_getN(5);
@$pb.TagNumber(7)
set stringValue($core.List<$core.int> v) {
set stringValue($typed_data.Uint8List v) {
$_setBytes(5, v);
}

Expand Down
14 changes: 14 additions & 0 deletions protoc_plugin/lib/src/message_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,20 @@ class MessageGenerator extends ProtobufContainer {
return false;
}

bool get needsTypedDataImport {
checkResolved();
for (var field in _fieldList) {
if (field.needsTypedDataImport) return true;
}
for (var m in _messageGenerators) {
if (m.needsTypedDataImport) return true;
}
for (var x in _extensionGenerators) {
if (x.needsTypedDataImport) return true;
}
return false;
}

/// Adds dependencies of [generate] to [imports].
///
/// For each .pb.dart file that the generated code needs to import,
Expand Down
14 changes: 10 additions & 4 deletions protoc_plugin/lib/src/protobuf_field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,18 @@ class ProtobufField {
bool get overridesClearMethod =>
_hasBooleanOption(Dart_options.overrideClearMethod);

/// True if this field uses the Int64 from the fixnum package.
/// Whether the field uses `Int64` from `fixnum`
bool get needsFixnumImport =>
baseType.unprefixed == '$_fixnumImportPrefix.Int64';

/// True if this field is a map field definition:
/// `map<key_type, value_type> map_field = N`.
/// Whether the field uses `Uint8List` from `dart:typed_data`
bool get needsTypedDataImport =>
baseType.unprefixed == '$_typedDataImportPrefix.Uint8List';

/// Whether the field is a map field definition:
/// ```
/// map<key_type, value_type> map_field = N
/// ```
bool get isMapField {
if (!isRepeated || !baseType.isMessage) return false;
final generator = baseType.generator as MessageGenerator;
Expand Down Expand Up @@ -374,7 +380,7 @@ class ProtobufField {
var byteList = descriptor.defaultValue.codeUnits
.map((b) => '0x${b.toRadixString(16)}')
.join(',');
return '() => <$coreImportPrefix.int>[$byteList]';
return '() => $_typedDataImportPrefix.Uint8List.fromList([$byteList])';
case FieldDescriptorProto_Type.TYPE_GROUP:
case FieldDescriptorProto_Type.TYPE_MESSAGE:
return '${baseType.getDartType(parent.fileGen!)}.getDefault';
Expand Down
2 changes: 1 addition & 1 deletion protoc_plugin/lib/src/shared.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

const protobufImportPrefix = r'$pb';
const asyncImportPrefix = r'$async';
const coreImportPrefix = r'$core';
const grpcImportPrefix = r'$grpc';
const mixinImportPrefix = r'$mixin';
const protobufImportPrefix = r'$pb';
6 changes: 4 additions & 2 deletions protoc_plugin/test/coded_buffer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:typed_data';

import 'package:test/test.dart';

import '../out/protos/entity.pb.dart';

void main() {
test('Does not reuse input buffer for bytes fields', () {
var message = BytesEntity()..value = [1, 2, 3];
var message = BytesEntity()..value = Uint8List.fromList([1, 2, 3]);
var bytes = message.writeToBuffer();
var deserialized1 = BytesEntity()..mergeFromBuffer(bytes);
var deserialized2 = BytesEntity()..mergeFromBuffer(bytes);
Expand All @@ -18,7 +20,7 @@ void main() {
});

test('Does not reuse input buffer for repeated bytes fields', () {
var message = BytesEntity()..values.add([1, 2, 3]);
var message = BytesEntity()..values.add(Uint8List.fromList([1, 2, 3]));
var bytes = message.writeToBuffer();
var deserialized1 = BytesEntity()..mergeFromBuffer(bytes);
var deserialized2 = BytesEntity()..mergeFromBuffer(bytes);
Expand Down
7 changes: 5 additions & 2 deletions protoc_plugin/test/extension_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:typed_data';

import 'package:protobuf/protobuf.dart';
import 'package:test/test.dart';

Expand All @@ -25,7 +27,8 @@ final withExtensions = TestAllExtensions()
..setExtension(
Unittest.optionalForeignMessageExtension, ForeignMessage()..c = 3)
..setExtension(Unittest.defaultStringExtension, 'bar')
..getExtension(Unittest.repeatedBytesExtension).add('pop'.codeUnits)
..getExtension(Unittest.repeatedBytesExtension)
.add(Uint8List.fromList('pop'.codeUnits))
..setExtension(
Extend_unittest.outer,
Outer()
Expand Down Expand Up @@ -287,7 +290,7 @@ void main() {
reason:
'ExtensionRegistry.reparseMessage does not leave reparsed fields in unknownFields');
expect(reparsed.getExtension(Unittest.repeatedBytesExtension),
['pop'.codeUnits]);
[Uint8List.fromList('pop'.codeUnits)]);

expect(
reparsed.getExtension(Unittest.optionalForeignMessageExtension).c, 3);
Expand Down
15 changes: 10 additions & 5 deletions protoc_plugin/test/generated_message_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

// @dart=2.11

import 'dart:typed_data';

import 'package:protobuf/protobuf.dart';
import 'package:test/test.dart';

Expand Down Expand Up @@ -115,7 +117,10 @@ void main() {
message.repeatedString[1] = null;
}, throwsArgumentError);

message.repeatedBytes.addAll(['one'.codeUnits, 'two'.codeUnits]);
message.repeatedBytes.addAll([
Uint8List.fromList('one'.codeUnits),
Uint8List.fromList('two'.codeUnits)
]);
expect(() {
message.repeatedBytes[1] = null;
}, throwsArgumentError);
Expand Down Expand Up @@ -163,7 +168,7 @@ void main() {
}, throwsArgumentError);

expect(() {
message.repeatedBytes.addAll(['one'.codeUnits, null]);
message.repeatedBytes.addAll([Uint8List.fromList('one'.codeUnits), null]);
}, throwsArgumentError);
});

Expand Down Expand Up @@ -306,7 +311,7 @@ void main() {

test('testReadHugeBlob', () {
// Allocate and initialize a 1MB blob.
var blob = List<int>.generate(1 << 20, (i) => i % 256);
var blob = Uint8List.fromList(List<int>.generate(1 << 20, (i) => i % 256));

// Make a message containing it.
var message = getAllSet();
Expand Down Expand Up @@ -819,8 +824,8 @@ void main() {
});

test('operator== and hashCode works for bytes', () {
final t1 = TestAllTypes()..optionalBytes = [1];
final t2 = TestAllTypes()..optionalBytes = [1];
final t1 = TestAllTypes()..optionalBytes = Uint8List.fromList([1]);
final t2 = TestAllTypes()..optionalBytes = Uint8List.fromList([1]);
final t3 = TestAllTypes.fromBuffer(t1.writeToBuffer());
expect(t1, equals(t2));
expect(t1.hashCode, equals(t2.hashCode));
Expand Down
19 changes: 14 additions & 5 deletions protoc_plugin/test/json_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:typed_data';

import 'package:fixnum/fixnum.dart';
import 'package:protobuf/protobuf.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -78,19 +80,26 @@ void main() {
});

test('testBase64Encode', () {
expect(getAllSet()..optionalBytes = 'Hello, world'.codeUnits,
expect(
getAllSet()
..optionalBytes = Uint8List.fromList('Hello, world'.codeUnits),
expectedJson(':"MTE2",', ':"SGVsbG8sIHdvcmxk",'));

expect(getAllSet()..optionalBytes = 'Hello, world!'.codeUnits,
expect(
getAllSet()
..optionalBytes = Uint8List.fromList('Hello, world!'.codeUnits),
expectedJson(':"MTE2",', ':"SGVsbG8sIHdvcmxkIQ==",'));

expect(getAllSet()..optionalBytes = 'Hello, world!!'.codeUnits,
expect(
getAllSet()
..optionalBytes = Uint8List.fromList('Hello, world!!'.codeUnits),
expectedJson(':"MTE2",', ':"SGVsbG8sIHdvcmxkISE=",'));

// An empty list should not appear in the output.
expect(getAllSet()..optionalBytes = [], expectedJson('"15":"MTE2",', ''));
expect(getAllSet()..optionalBytes = Uint8List(0),
expectedJson('"15":"MTE2",', ''));

expect(getAllSet()..optionalBytes = 'a'.codeUnits,
expect(getAllSet()..optionalBytes = Uint8List.fromList('a'.codeUnits),
expectedJson(':"MTE2",', ':"YQ==",'));
});

Expand Down
11 changes: 6 additions & 5 deletions protoc_plugin/test/map_field_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:convert';
import 'dart:typed_data';

import 'package:protobuf/protobuf.dart';
import 'package:test/test.dart';
Expand All @@ -18,9 +19,9 @@ void main() {
..int32ToStringField[1] = '11'
..int32ToStringField[2] = '22'
..int32ToStringField[3] = '33'
..int32ToBytesField[1] = utf8.encode('11')
..int32ToBytesField[2] = utf8.encode('22')
..int32ToBytesField[3] = utf8.encode('33')
..int32ToBytesField[1] = Uint8List.fromList(utf8.encode('11'))
..int32ToBytesField[2] = Uint8List.fromList(utf8.encode('22'))
..int32ToBytesField[3] = Uint8List.fromList(utf8.encode('33'))
..int32ToEnumField[1] = TestMap_EnumValue.DEFAULT
..int32ToEnumField[2] = TestMap_EnumValue.BAR
..int32ToEnumField[3] = TestMap_EnumValue.BAZ
Expand All @@ -40,9 +41,9 @@ void main() {
..int32ToStringField[1] = '111'
..int32ToStringField.remove(2)
..int32ToStringField[4] = '44'
..int32ToBytesField[1] = utf8.encode('111')
..int32ToBytesField[1] = Uint8List.fromList(utf8.encode('111'))
..int32ToBytesField.remove(2)
..int32ToBytesField[4] = utf8.encode('44')
..int32ToBytesField[4] = Uint8List.fromList(utf8.encode('44'))
..int32ToEnumField[1] = TestMap_EnumValue.BAR
..int32ToEnumField.remove(2)
..int32ToEnumField[4] = TestMap_EnumValue.ZOP
Expand Down
Loading