Skip to content

Protobuf / gRPC rewrite, unit-test updates, and other improvements #142

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

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

gunnar-mb
Copy link
Collaborator

@gunnar-mb gunnar-mb commented Jun 9, 2025

Author: Gunnar Andersson <[email protected]>, MBition GmbH.

Protobuf/gRPC rewrite

This is a major update of the Protobuf/gRPC support.

  • Improved parser, including changes to the grammar and AST model
  • Rewritten translation into IFEX, using the table-based / rule_translator method, plus new find-methods.
  • Unit-tests to test parser against all unit-test files found in Google protobuf and gRPC projects
  • Added new test-helper "framework" for quicker/easier definition of tests
  • Refactored several parts, reducing duplication and introducing utils.py for common functions.

The program was tested solely for our own use cases, which might differ from yours.

The submission is provided under the main project license (LICENSE file in root of project).

Provider Information

gunnar-mb and others added 28 commits May 21, 2025 08:54
Calls to gen() failed on StructuredOption because no such template
existed.  For this simple case no separate template created and
instead the required syntax was included in the Option template.

Signed-off-by: Gunnar Andersson <[email protected]>
If attribute in target object is a list, we normally append
to it, but if the new value is None, we don't want it appended.

This seems to remove some issue, but there is a small risk doing this
will instead hide other bugs (None value should not appear).  Let's see
how it works out.

Signed-off-by: Gunnar Andersson <[email protected]>
A major rewrite of the rule_translator.  Main details:

1. global_attribute_map is now removed and instead handled with a key
   named "Default" in the main type_map.  This simplifies the concept
   and looks nicer.
2. For better clarity, same-named attributes ARE NO LONGER(!)
   automatically copied over between input and output.  Put each such
   required mapping into the Default instead, e.g. :  ('name', 'name')
3. The structure of the handling of global/Default rules was changed
   and is now much more similar to the handling of all specific rules.
   This opens up for another refactoring and removal of the duplicated
   code in a later commit.
4. When a function/callable is defined it now gets more of the
   processing context passed to it (input and output objects), which can
   be ignored if not needed, but may also be used for something clever.
5. More logging.

Everything is not fully cleaned-up but since I anticipate a
refactoring to remove duplication, it can wait until then.

Signed-off-by: Gunnar Andersson <[email protected]>
Use the rule_translator engine and a translation table
instead of the previous imperative approach.

For now, named protobuf_to_ifex2 to distinguish it.
The old one likely removed next.

Signed-off-by: Gunnar Andersson <[email protected]>
In grpc/protobuf IDL, enumerations can be defined inside of messages.
We primarily treat each message as a datatype definition and therefore
convert them to a Struct type.

A type-definition inside, like an enumeration, does not belong there in
the IFEX way of modeling.  It cannot be mapped into something equivalent
inside of a Struct, so the enumeration definitions are instead collected
separately and placed in an appropriate location for the IFEX model.

Signed-off-by: Gunnar Andersson <[email protected]>
This includes new functions to search/find nodes and manipulate AST
trees, and it consolidates previous AST-related helper functions.

Some previous helper functions that were specifically in the IFEX model
implementation can be used for any AST definition built in the same
way.

Signed-off-by: Gunnar Andersson <[email protected]>
Utility functions previously in IFEX-specific files are actually generic
and can be used for multiple similar ASTs.  The common/ast_utils file is
the current place to bring them together.

After this, the ifex_ast_construction file is obsolete -> deleted
and the type_checking_constructor_mixin is moved to common area.

Signed-off-by: Gunnar Andersson <[email protected]>
Signed-off-by: Gunnar Andersson <[email protected]>
This simplifies the implementation of TemplateDir so that it can
easily get the node names to match them against template names.
(the walk function was clearly overkill for this task).

Signed-off-by: Gunnar Andersson <gunnar_dev@[email protected]>
The special stuff caused issues during refactoring and changing
imports between files - the whole project then refused to install
due to some circular-dependencies or other import issues caused
by this in the setup.py

I believe we don't need the special handling of the templatedir anymore.
If we do, it should be done differently anyway.

Signed-off-by: Gunnar Andersson <[email protected]>
There were more generic functions in ifex_ast_introspect.  The functions
to build/investigate/manipulate AST trees are general for all similar
ASTs so they are now also moved to the common utils file.

There may be a few duplicates for what we already moved from
ifex_ast_construction file, but they can be cleaned up later.

Signed-off-by: Gunnar Andersson <[email protected]>
There was a circular import dependency between
type_checking_constructor_mixin and ast_utils because ast_utils had a
wrapper function around type_checking_constructor_mixin which uses many
of the other utils. This breaks the cycle and creates a clean separation.

Signed-off-by: Gunnar Andersson <[email protected]>
The input files from the unit tests used by the protobuf project itself
have been tested, and errors fixed accordingly.  Several fixes and
some resulting clean-up done.  A more comprehensive clean-up of the
grammar (and parser code) is expected when automated unit/regression
testing is more complete.

Signed-off-by: Gunnar Andersson <[email protected]>
Follows from the previous commit.  Cleanup and refactoring is expected
when better regression tests are set up.

Signed-off-by: Gunnar Andersson <[email protected]>
Signed-off-by: Gunnar Andersson <[email protected]>
These unit-test files are from the Google protobuf and gRPC original
implementations.  License information is provided.

For reasons possibly explained elsewhere we were unable to effectively
use the actual Google Protobuf parser implementation.  But checking the
IFEX project's own parser's ability to read all provided test files
should hopefully give some confidence of compatibility and it being full
featured

The pytest ensures that the parser can successfully consume the proto3
formatted test files.  A few proto2 files have been omitted due to
deprecated syntax and features we don't intend to support at this time.

Signed-off-by: Gunnar Andersson <[email protected]>
During development we need to process files that are parsed correctly
but not supported for translation in a certain case.

If the log level is unspecified, the default is ERROR. It is better if
the program stays quiet about this in development.  During actual use,
it is possible to set a lower threshold to get notified of that log
level..

Signed-off-by: Gunnar Andersson <[email protected]>
A new small "framework" for test definitions that allows easy
definition of:
 - program or function to run
 - input files
 - expected results or outputs

There is a wrapper function that will execute it also in pytest, as
usual.

Signed-off-by: Gunnar Andersson <[email protected]>
@gunnar-mb gunnar-mb changed the title Protobuf rewrite Protobuf / gRPC rewrite, unit-test updates, and other improvements Jun 9, 2025
@mbenelli mbenelli self-requested a review June 10, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants