Skip to content

Commit

Permalink
Remove event support (#880)
Browse files Browse the repository at this point in the history
Event support is currently unused internally and we don't want to introduce new
uses of it, to keep the API surface small and the library easier to update, and
to make it easier to migrate to another library.

`createRepeatedField` and `createMapField` existed to support events (as
`PbList` and `PbMap` don't support this), so with the event mixin removed we
remove these members as well.

Removing these members give us other opportunities: we now have full control
over the field value types. This allows, for example, refactoring `PbMap` and
`PbList` types for marking them as frozen without visiting the elements, which
makes it possible to implement decoders that create frozen objects without
having to make another pass after decoding to mark every object as frozen.

Closes #738.

cl/571893384
  • Loading branch information
osa1 authored Oct 12, 2023
1 parent c559fe5 commit 96d9522
Show file tree
Hide file tree
Showing 17 changed files with 63 additions and 544 deletions.
16 changes: 16 additions & 0 deletions protobuf/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
## 4.0.0-dev

* The following types and members are now removed:

- `PbEventMixin`
- `PbFieldChange`
- `EventBuffer`
- `GeneratedMessage.createRepeatedField`
- `GeneratedMessage.createMapField`

These were used to implement events, which are unused internally. To keep API
surface small (to make it easier to change the library or migrate to another
library) these types and members are removed. ([#738])

[#738]: https://github.com/google/protobuf.dart/issues/738

## 3.1.0

* `CodedBufferReader` `readBytes` now copies the returned bytes to avoid
Expand Down
3 changes: 0 additions & 3 deletions protobuf/lib/meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ const GeneratedMessage_reservedNames = <String>[
'clone',
'copyWith',
'createEmptyInstance',
'createMapField',
'createRepeatedField',
'eventPlugin',
'extensionsAreInitialized',
'freeze',
'getDefaultForField',
Expand Down
1 change: 0 additions & 1 deletion protobuf/lib/protobuf.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ part 'src/protobuf/coded_buffer.dart';
part 'src/protobuf/coded_buffer_reader.dart';
part 'src/protobuf/coded_buffer_writer.dart';
part 'src/protobuf/consts.dart';
part 'src/protobuf/event_plugin.dart';
part 'src/protobuf/exceptions.dart';
part 'src/protobuf/extension.dart';
part 'src/protobuf/extension_field_set.dart';
Expand Down
31 changes: 0 additions & 31 deletions protobuf/lib/src/protobuf/event_plugin.dart

This file was deleted.

18 changes: 5 additions & 13 deletions protobuf/lib/src/protobuf/extension_field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class _ExtensionFieldSet {
///
/// If it doesn't exist, creates the list and saves the extension.
/// Suitable for public API and decoders.
List<T> _ensureRepeatedField<T>(Extension<T> fi) {
PbList<T> _ensureRepeatedField<T>(Extension<T> fi) {
assert(!_isReadOnly);
assert(fi.isRepeated);
assert(fi.extendee == '' || fi.extendee == _parent._messageName);
Expand All @@ -50,17 +50,17 @@ class _ExtensionFieldSet {
return _addInfoAndCreateList(fi);
}

List<T> _getList<T>(Extension<T> fi) {
PbList<T> _getList<T>(Extension<T> fi) {
final value = _values[fi.tagNumber];
if (value != null) return value;
_checkNotInUnknown(fi);
if (_isReadOnly) return List<T>.unmodifiable(const []);
if (_isReadOnly) return PbList<T>.unmodifiable();
return _addInfoAndCreateList<T>(fi);
}

List<T> _addInfoAndCreateList<T>(Extension<T> fi) {
PbList<T> _addInfoAndCreateList<T>(Extension<T> fi) {
_validateInfo(fi);
final newList = fi._createRepeatedField(_parent._message!);
final newList = fi._createRepeatedField();
_addInfoUnchecked(fi);
_setFieldUnchecked(fi, newList);
return newList;
Expand All @@ -76,10 +76,6 @@ class _ExtensionFieldSet {
void _clearField(Extension fi) {
_ensureWritable();
_validateInfo(fi);
final eventPlugin = _parent._eventPlugin;
if (eventPlugin != null && eventPlugin.hasObservers) {
eventPlugin.beforeClearField(fi);
}
_values.remove(fi.tagNumber);
}

Expand Down Expand Up @@ -134,10 +130,6 @@ class _ExtensionFieldSet {
}

void _setFieldUnchecked(Extension fi, value) {
final eventPlugin = _parent._eventPlugin;
if (eventPlugin != null && eventPlugin.hasObservers) {
eventPlugin.beforeSetField(fi, value);
}
// If there was already an unknown field with the same tag number,
// overwrite it.
_parent._unknownFields?.clearField(fi.tagNumber);
Expand Down
23 changes: 10 additions & 13 deletions protobuf/lib/src/protobuf/field_info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -215,23 +215,20 @@ class FieldInfo<T> {
}
}

/// Creates a repeated field to be attached to the given message.
///
/// Delegates actual list creation to the message, so that it can
/// be overridden by a mixin.
List<T> _createRepeatedField(GeneratedMessage m) {
/// Creates a repeated field.
PbList<T> _createRepeatedField() {
assert(isRepeated);
return m.createRepeatedField<T>(tagNumber, this);
return PbList<T>(check: check!);
}

/// Same as above, but allow a tighter typed List to be created.
List<S> _createRepeatedFieldWithType<S extends T>(GeneratedMessage m) {
/// Same as above, but allow a tighter typed [PbList] to be created.
PbList<S> _createRepeatedFieldWithType<S extends T>() {
assert(isRepeated);
return m.createRepeatedField<S>(tagNumber, this as FieldInfo<S>);
return PbList<S>(check: check!);
}

/// Convenience method to thread this FieldInfo's reified type parameter to
/// _FieldSet._ensureRepeatedField.
/// `_FieldSet._ensureRepeatedField`.
List<T> _ensureRepeatedField(BuilderInfo meta, _FieldSet fs) {
return fs._ensureRepeatedField<T>(meta, this);
}
Expand Down Expand Up @@ -292,12 +289,12 @@ class MapFieldInfo<K, V> extends FieldInfo<PbMap<K, V>?> {
FieldInfo get valueFieldInfo =>
mapEntryBuilderInfo.fieldInfo[PbMap._valueFieldNumber]!;

Map<K, V> _ensureMapField(BuilderInfo meta, _FieldSet fs) {
PbMap<K, V> _ensureMapField(BuilderInfo meta, _FieldSet fs) {
return fs._ensureMapField<K, V>(meta, this);
}

Map<K, V> _createMapField(GeneratedMessage m) {
PbMap<K, V> _createMapField() {
assert(isMapField);
return m.createMapField<K, V>(tagNumber, this);
return PbMap<K, V>(keyFieldType, valueFieldType);
}
}
56 changes: 10 additions & 46 deletions protobuf/lib/src/protobuf/field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ void _throwFrozenMessageModificationError(String messageName,
/// JavaScript.
class _FieldSet {
final GeneratedMessage? _message;
final EventPlugin? _eventPlugin;

/// The value of each non-extension field in a fixed-length array.
/// The index of a field can be found in [FieldInfo.index].
Expand Down Expand Up @@ -74,7 +73,7 @@ class _FieldSet {
/// the index is not present, the oneof field is unset.
final Map<int, int>? _oneofCases;

_FieldSet(this._message, BuilderInfo meta, this._eventPlugin)
_FieldSet(this._message, BuilderInfo meta)
: _values = _makeValueList(meta.byIndex.length),
_oneofCases = meta.oneofs.isEmpty ? null : <int, int>{};

Expand Down Expand Up @@ -227,10 +226,6 @@ class _FieldSet {
assert(tagNumber == fi.tagNumber);

// Clear a non-extension field
final eventPlugin = _eventPlugin;
if (eventPlugin != null && eventPlugin.hasObservers) {
eventPlugin.beforeClearField(fi);
}
_values[fi.index!] = null;

final oneofIndex = meta.oneofs[tagNumber];
Expand Down Expand Up @@ -302,7 +297,7 @@ class _FieldSet {
/// Creates and stores the repeated field if it doesn't exist.
/// If it's an extension and the list doesn't exist, validates and stores it.
/// Suitable for decoders.
List<T> _ensureRepeatedField<T>(BuilderInfo meta, FieldInfo<T> fi) {
PbList<T> _ensureRepeatedField<T>(BuilderInfo meta, FieldInfo<T> fi) {
assert(!_isReadOnly);
assert(fi.isRepeated);
if (fi.index == null) {
Expand All @@ -311,7 +306,7 @@ class _FieldSet {
final value = _getFieldOrNull(fi);
if (value != null) return value;

final newValue = fi._createRepeatedField(_message!);
final newValue = fi._createRepeatedField();
_setNonExtensionFieldUnchecked(meta, fi, newValue);
return newValue;
}
Expand All @@ -324,9 +319,9 @@ class _FieldSet {
final value = _getFieldOrNull(fi);
if (value != null) return value;

final newValue = fi._createMapField(_message!);
final newValue = fi._createMapField();
_setNonExtensionFieldUnchecked(meta, fi, newValue);
return newValue as PbMap<K, V>;
return newValue;
}

/// Sets a non-extended field and fires events.
Expand All @@ -341,14 +336,6 @@ class _FieldSet {
_oneofCases![oneofIndex] = tag;
}

// It is important that the callback to the observers is not moved to the
// beginning of this method but happens just before the value is set.
// Otherwise the observers will be notified about 'clearField' and
// 'setField' events in an incorrect order.
final eventPlugin = _eventPlugin;
if (eventPlugin != null && eventPlugin.hasObservers) {
eventPlugin.beforeSetField(fi, value);
}
_values[fi.index!] = value;
}

Expand Down Expand Up @@ -394,7 +381,7 @@ class _FieldSet {
return fi.readonlyDefault;
}

final list = fi._createRepeatedFieldWithType<T>(_message!);
final list = fi._createRepeatedFieldWithType<T>();
_setNonExtensionFieldUnchecked(_meta, fi, list);
return list;
}
Expand All @@ -412,7 +399,7 @@ class _FieldSet {
PbMap<K, V>(fi.keyFieldType, fi.valueFieldType));
}

final map = fi._createMapField(_message!);
final map = fi._createMapField();
_setNonExtensionFieldUnchecked(_meta, fi, map);
return map;
}
Expand Down Expand Up @@ -486,10 +473,6 @@ class _FieldSet {
if (value == null) {
_$check(index, value); // throw exception for null value
}
final eventPlugin = _eventPlugin;
if (eventPlugin != null && eventPlugin.hasObservers) {
eventPlugin.beforeSetField(_nonExtensionInfoByIndex(index), value);
}
final meta = _meta;
final tag = meta.byIndex[index].tagNumber;
final oneofIndex = meta.oneofs[tag];
Expand All @@ -516,25 +499,8 @@ class _FieldSet {
if (_unknownFields != null) {
_unknownFields!.clear();
}

final extensions = _extensions;

final eventPlugin = _eventPlugin;
if (eventPlugin != null && eventPlugin.hasObservers) {
for (final fi in _infos) {
if (_values[fi.index!] != null) {
eventPlugin.beforeClearField(fi);
}
}
if (extensions != null) {
for (final key in extensions._tagNumbers) {
final fi = extensions._getInfoOrNull(key)!;
eventPlugin.beforeClearField(fi);
}
}
}
if (_values.isNotEmpty) _values.fillRange(0, _values.length, null);
extensions?._clearValues();
_extensions?._clearValues();
}

bool _equals(_FieldSet o) {
Expand Down Expand Up @@ -882,15 +848,13 @@ class _FieldSet {
if (fieldInfo.isMapField) {
final PbMap? map = _values[index];
if (map != null) {
_values[index] = (fieldInfo as MapFieldInfo)
._createMapField(_message!)
_values[index] = (fieldInfo as MapFieldInfo)._createMapField()
..addAll(map);
}
} else if (fieldInfo.isRepeated) {
final PbList? list = _values[index];
if (list != null) {
_values[index] = fieldInfo._createRepeatedField(_message!)
..addAll(list);
_values[index] = fieldInfo._createRepeatedField()..addAll(list);
}
}
}
Expand Down
30 changes: 11 additions & 19 deletions protobuf/lib/src/protobuf/generated_message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,22 @@ abstract class GeneratedMessage {
_FieldSet get _fieldSet => __fieldSet!;

GeneratedMessage() {
__fieldSet = _FieldSet(this, info_, eventPlugin);
if (eventPlugin != null) eventPlugin!.attach(this);
__fieldSet = _FieldSet(this, info_);

// The following two returns confuse dart2js into avoiding inlining the
// constructor *body*. A `@pragma('dart2js:never-inline')` annotation on
// the constructor affects inlining of the generative constructor factory,
// not the constructor body that is called from all the subclasses.
//
// TODO(http://dartbug.com/49475): Remove this when there is an annotation
// that will give the desired result.
return;
return; // ignore: dead_code
}

// Overridden by subclasses.
BuilderInfo get info_;

/// Subclasses can override this getter to be notified of changes
/// to protobuf fields.
EventPlugin? get eventPlugin => null;

/// Creates a deep copy of the fields in this message.
/// (The generated code uses [mergeFromMessage].)
@Deprecated('Using this can add significant size overhead to your binary. '
Expand Down Expand Up @@ -331,19 +336,6 @@ abstract class GeneratedMessage {
/// default value if it is not set.
dynamic getField(int tagNumber) => _fieldSet._getField(tagNumber);

/// Creates List implementing a mutable repeated field.
///
/// Mixins may override this method to change the List type. To ensure
/// that the protobuf can be encoded correctly, the returned List must
/// validate all items added to it. This can most easily be done
/// using the [FieldInfo.check] function.
List<T> createRepeatedField<T>(int tagNumber, FieldInfo<T> fi) =>
PbList<T>(check: fi.check!);

/// Creates a Map representing a map field.
Map<K, V> createMapField<K, V>(int tagNumber, MapFieldInfo<K, V> fi) =>
PbMap<K, V>(fi.keyFieldType, fi.valueFieldType);

/// Returns the value of a field, ignoring any defaults.
///
/// For unset or cleared fields, returns null.
Expand Down
4 changes: 2 additions & 2 deletions protobuf/lib/src/protobuf/json.dart
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@ void _appendJsonList(BuilderInfo meta, _FieldSet fs, List jsonList,
void _appendJsonMap(BuilderInfo meta, _FieldSet fs, List jsonList,
MapFieldInfo fi, ExtensionRegistry? registry) {
final entryMeta = fi.mapEntryBuilderInfo;
final map = fi._ensureMapField(meta, fs) as PbMap<dynamic, dynamic>;
final map = fi._ensureMapField(meta, fs);
for (final jsonEntryDynamic in jsonList) {
final jsonEntry = jsonEntryDynamic as Map<String, dynamic>;
final entryFieldSet = _FieldSet(null, entryMeta, null);
final entryFieldSet = _FieldSet(null, entryMeta);
final convertedKey = _convertJsonValue(
entryMeta,
entryFieldSet,
Expand Down
Loading

0 comments on commit 96d9522

Please sign in to comment.