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.

Sign up for GitHub

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

Jump to bottom

Upgrading Dependencies #47

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Upgrading Dependencies #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
006b9db
Upgrading used Java Version from 11 to 17
Anrufliste Jan 29, 2024
7ad5591
fixing maven-compiler-plugin
Anrufliste Jan 29, 2024
1ef549c
fixing github action to use jave 17
Anrufliste Jan 29, 2024
6146a98
version jump to groovy 4
Anrufliste Jan 30, 2024
3a6ebb8
moving more version numbers into property section and upgrading all b…
Anrufliste Jan 30, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Set up Maven Central Repository
uses: actions/setup-java@v1
with:
java-version: '11'
java-version: '17'
server-id: ossrh
server-username: MAVEN_USERNAME
server-password: MAVEN_PASSWORD
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/maven.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ jobs:

steps:
- uses: actions/checkout@v3
- name: Set up JDK 11
- name: Set up JDK 17
uses: actions/setup-java@v3
with:
java-version: '11'
java-version: '17'
distribution: 'temurin'
cache: maven
- name: Build with Maven
Expand Down
36 changes: 18 additions & 18 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

With the phonenumber-normalizer library, you can normalize phone numbers to the E164 format and national format, taking into account the specific complexities of the German number plan. The library can also differentiate between short numbers and those with NDCs and NACs, and can be useful for handling phone numbers in fixed-line contexts.

While Google's [PhoneLib](https://github.com/google/libphonenumber) is a general-purpose library for handling phone numbers worldwide, phonenumber-normalizer a wrapper around it, which is specifically tailored to handle the complexities of the German number plan. Especially, phonenumber-normalizer can differentiate between short numbers and those with NDCs and NACs. When PhoneLib is not able to make this distinction, our wrapping corrects the result.
While Google's [LibPhoneNumber](https://github.com/google/libphonenumber) is a general-purpose library for handling phone numbers worldwide, phonenumber-normalizer a wrapper around it, which is specifically tailored to handle the complexities of the German number plan. Especially, phonenumber-normalizer can differentiate between short numbers and those with NDCs and NACs. When LibPhoneNumber is not able to make this distinction, our wrapping corrects the result.

![Phone Number Normalizer](https://user-images.githubusercontent.com/3244965/235174029-e58fab4c-37e9-49ba-834e-067b50082abb.png)

## Problem Statement(s)

If you have to deal with the complexity of telephone numbers your natural choice of handling it, should be the great [PhoneLib](https://github.com/google/libphonenumber) of [Google](https://opensource.google).
If you have to deal with the complexity of telephone numbers your natural choice of handling it, should be the great [LibPhoneNumber](https://github.com/google/libphonenumber) of [Google](https://opensource.google).

That's what we did for some projects.
And when we found bugs, we [returned that as feedback](./REPORTED_ISSUES.md) in good open-source tradition.
Expand Down Expand Up @@ -41,7 +41,7 @@ c) a SN with NDC and NAC
d) a SN with NDC and CC (but without NAC in Germany but with in Italy)

We need that differentiation upfront normalization to E164 (which is d).
Currently, PhoneLib does not identify this situation and adds the CC infront a number b-style, which represents a complete different phone line.
Currently, LibPhoneNumber does not identify this situation and adds the CC infront a number b-style, which represents a complete different phone line.

### Example of failing normalization

Expand All @@ -62,10 +62,10 @@ Additionally, we want to normalize a number with a default NDC, because we might
### Root Cause

We think the problem arises from the following situation:
- PhoneLib is storing the National Significant Number (NSN) as the number, which is the combination of NDC + SN.
- PhoneLib is storing the number as uint64 which does not allow to store (a) leading Zero(s)
- PhoneLib is storing leading Zeros in hasNumberOfLeadingZeros and getNumberOfLeadingZeros
- PhoneLib checks "IS_POSSIBLE_LOCAL_ONLY" only by comparing the length of a number, which might work in a fix length number plan like the North American Number plan, but not with variable number length and optional NDC as in Germany.
- LibPhoneNumber is storing the National Significant Number (NSN) as the number, which is the combination of NDC + SN.
- LibPhoneNumber is storing the number as uint64 which does not allow to store (a) leading Zero(s)
- LibPhoneNumber is storing leading Zeros in hasNumberOfLeadingZeros and getNumberOfLeadingZeros
- LibPhoneNumber checks "IS_POSSIBLE_LOCAL_ONLY" only by comparing the length of a number, which might work in a fix length number plan like the North American Number plan, but not with variable number length and optional NDC as in Germany.

What needs to be done:
- Checking if the Country is supporting optional NDC
Expand All @@ -75,13 +75,13 @@ What needs to be done:
- if that all applies, such an input needs to be treated as a short number and not be changed for E164 or National format (see short number [normalization of 116116](https://libphonenumber.appspot.com/phonenumberparser?number=116116&country=DE))

The problem has been addressed but rejected like with [issue tracker 180311606](https://issuetracker.google.com/issues/180311606).
Reasons are either PhoneLib just make best efforts for formatting and not format checking or that they focus on mobile context while most problems happens in fixed-line context.
Reasons are either LibPhoneNumber just make best efforts for formatting and not format checking or that they focus on mobile context while most problems happens in fixed-line context.

## State of Our Implementation

### Code

As a wrapper we did not change any code of PhoneLib itself, so an [upgrade to the newest version should be possible by just updating the version in the dependency POM definition](UPDATE_FOR_NEW_PHONELIB.md).
As a wrapper we did not change any code of LibPhoneNumber itself, so an [upgrade to the newest version should be possible by just updating the version in the dependency POM definition](UPDATE_FOR_NEW_PHONELIB.md).

You could either take the sourcecode directly from the repository or use Maven dependency management by adding:
```
Expand Down Expand Up @@ -117,37 +117,37 @@ Now we get a E164 formatted number, because now we know, how which NDC has to be

### Use Of Reflection

To check if a number plan of a country is using an optional NDC and NAC, we need to get the countries region metadata from PhoneLib.
To check if a number plan of a country is using an optional NDC and NAC, we need to get the countries region metadata from LibPhoneNumber.

With getMetadataForRegion there is a method on the PhoneNumberUtil class, but it is not public.

So we are forced to [use reflection and override its accessibility](https://github.com/telekom/phonenumber-normalizer/blob/main/src/main/java/de/telekom/phonenumbernormalizer/numberplans/PhoneLibWrapper.java#L280).

If you are using AOT (ahead of time) compiler, you need to take care of this.
(While it is used indirectly with the normal PhoneLib use of the wrapper, it might not be safe for all AOT compilers).
(While it is used indirectly with the normal LibPhoneNumber use of the wrapper, it might not be safe for all AOT compilers).

### Use of Own ShortNumber Recognition

When we started with the wrapper, PhoneLib did not recognize some phone assistant services as short numbers.
When we started with the wrapper, LibPhoneNumber did not recognize some phone assistant services as short numbers.
This as been fixed by [issue tracker 182490059](https://issuetracker.google.com/u/1/issues/182490059).

But for the EU Social Service Number Range 116xxx, PhoneLib is only checking the assigned number 116116 in Germany.
But for the EU Social Service Number Range 116xxx, LibPhoneNumber is only checking the assigned number 116116 in Germany.
This is in contradiction to normal validation, where the range is checked and assignment checks are explicitly not in scope.
But for the [issue 183669955](https://issuetracker.google.com/u/1/issues/183669955), they insist on assignment, since they need it for free call checks.
But for other EU states PhoneLib is using the full raneg (e.g. [CZ](https://github.com/google/libphonenumber/blob/4c532d93587d2f9d16dc7a536df55bf179158210/resources/ShortNumberMetadata.xml#L3342))
But for other EU states LibPhoneNumber is using the full raneg (e.g. [CZ](https://github.com/google/libphonenumber/blob/4c532d93587d2f9d16dc7a536df55bf179158210/resources/ShortNumberMetadata.xml#L3342))

### One More Thing: Area Gecode Label

There is a table of names for the area code from the BNetzA, which is using non-common abbreviations, which wouldn't be understood by end users.
But PhoneLib is using those and refusing the long transcription, because they only trust the BNetzA document (see [issue tracker 183383466](https://issuetracker.google.com/issues/183383466)).
But LibPhoneNumber is using those and refusing the long transcription, because they only trust the BNetzA document (see [issue tracker 183383466](https://issuetracker.google.com/issues/183383466)).
In addition, the BNetzA has published a [document on the ITU](https://www.itu.int/dms_pub/itu-t/oth/02/02/T02020000510006PDFE.pdf) and [its own website](https://www.bundesnetzagentur.de/SharedDocs/Downloads/DE/Sachgebiete/Telekommunikation/Unternehmen_Institutionen/Nummerierung/Rufnummern/ONRufnr/Vorwahlverzeichnis_ONB.zip.zip?__blob=publicationFile&v=298), which differ for some values, too.

We provide resolution for each country but also for cities in Germany and states in the US.
Our engine is using easy to generate [JSON data](./src/main/resources/arealabels/nationallabels/), so you could also provide your own resolution.

We only rely on the number analysis / formation logic of PhoneLib.
We only rely on the number analysis / formation logic of LibPhoneNumber.
Their GeoCoder is not used for your labeling.
We only use the PhoneLib's GeoCoder in some testcases, to check if ours and their labeling against each other.
We only use the LibPhoneNumber's GeoCoder in some testcases, to check if ours and their labeling against each other.

```
String normalizedNumber = "+493020355555";
Expand All @@ -163,7 +163,7 @@ This project has adopted the [Contributor Covenant](https://www.contributor-cove

By participating in this project, you agree to abide by its [Code of Conduct](./CODE_OF_CONDUCT.md) at all times.

A simple regular task is to adapt the project to use a new versions of Google's [PhoneLib](https://github.com/google/libphonenumber). We also have a [guide](UPDATE_FOR_NEW_PHONELIB.md) for this.
A simple regular task is to adapt the project to use a new versions of Google's [LibPhoneNumber](https://github.com/google/libphonenumber). We also have a [guide](UPDATE_FOR_NEW_PHONELIB.md) for this.

## Licensing

Expand Down
4 changes: 2 additions & 2 deletions REPORTED_ISSUES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ This issue has been resolved.

### 2021-03-25 - [Germany (DE, +49): 116xxx Short Number valid vs. assigned](https://issuetracker.google.com/issues/183669955)

This issue pertains to the EU-wide special social number short code definition. Although the regulation clearly defines a range, PhoneLib is not validating against that range, but against a list of currently assigned/operated numbers. At least for the German number space, as mentioned in the initial issue discussion (see first one above), the library is only partly or even completely checking the whole range in other EU number spaces.
This issue pertains to the EU-wide special social number short code definition. Although the regulation clearly defines a range, LibPhoneNumber is not validating against that range, but against a list of currently assigned/operated numbers. At least for the German number space, as mentioned in the initial issue discussion (see first one above), the library is only partly or even completely checking the whole range in other EU number spaces.

On the one hand, the [FAQ](https://github.com/google/libphonenumber/blob/master/FAQ.md#what_is_valid)(https://github.com/google/libphonenumber/blob/master/FAQ.md) states that “valid” does not mean “numbers are currently assigned to a specific user and reachable.”
On the other hand, it states that “a valid number range is one from which numbers can be freely assigned by carriers to users,” which is not the case for EU-wide numbers that require special clearance.
Expand All @@ -25,7 +25,7 @@ While it seems that the last point is the argument for rejecting the issue (whic
## Internal Implementation

After a long discussion and closing the issues as stated above, we decided to implement a minimal fix for normalizing German Phonenumbers for our internal use, to support the phoning capability of Deutsche Telekom Smart Speaker.
We also set up [test cases to verify if PhoneLib behavior changes to correctly normalize the currently failing cases](https://github.com/telekom/phonenumber-normalizer/blob/main/src/test/groovy/de/telekom/phonenumbernormalizer/extern/libphonenumber/PhoneNumberUtilTest.groovy), so that our implementation would become unnecessary.
We also set up [test cases to verify if LibPhoneNumber behavior changes to correctly normalize the currently failing cases](https://github.com/telekom/phonenumber-normalizer/blob/main/src/test/groovy/de/telekom/phonenumbernormalizer/extern/libphonenumber/PhoneNumberUtilTest.groovy), so that our implementation would become unnecessary.

We also discovered, during its development, that the geolocation method uses BenetzA given labels, which include abbreviations.
[For a smart speaker, we needed a “speakable” label - so we created the following issue and added our own list to our interim solution](https://github.com/telekom/phonenumber-normalizer/blob/main/src/main/resources/arealabels/nationallabels/de.json).
Expand Down
29 changes: 10 additions & 19 deletions UPDATE_FOR_NEW_PHONELIB.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,22 @@
# How to adapt to a new Version of PhoneLib
# How to adapt to a new Version of Google's LibPhoneNumber

If Google updates its [PhoneLib](https://github.com/google/libphonenumber), this project should be updated to use that new version. This file is a step by step instruction how to do this:
If Google updates its [LibPhoneNumber](https://github.com/google/libphonenumber), this project should be updated to use that new version. This file is a step by step instruction how to do this:

1. Create a new local branch - best name it ```phonelib/X_YY_ZZ``` so it is easily seen that this branch is just an update for the version X.YY.ZZ, without any new features.

2. Update [pom.xml](pom.xml) to use the new phonelib version:
2. Update [pom.xml](pom.xml) to use the new LibPhoneNumber version in the properties section:
```
<dependency>
<groupId>com.googlecode.libphonenumber</groupId>
<artifactId>libphonenumber</artifactId>
<version>X.YY.ZZ</version>
</dependency>
<libphonenumber.version>X.YY.ZZ</libphonenumber.version>
```

3. Check on Maven Central ```https://central.sonatype.com/artifact/com.googlecode.libphonenumber/libphonenumber/X.YY.ZZ/dependents``` the version number for ```geocoder``` (referred as A.BBB).

4. Update [pom.xml](pom.xml) to use the new geocoder version in testing:
4. Update [pom.xml](pom.xml) to use the new geocoder version in testing in the properties section:
```
<dependency>
<groupId>com.googlecode.libphonenumber</groupId>
<artifactId>geocoder</artifactId>
<version>A.BBB</version>
<scope>test</scope>
</dependency>
<geocoder.version>A.BBB</geocoder.version>
```

5. Run all unit test and check log messages if Phonelib still is not correctly:
5. Run all unit test and check log messages if LibPhoneNumber still is not correctly:
a) normalizing specific number -> this project is still necessary
b) labeling specific numbers -> own area labels for DE still necessary
if there are corrections or additional mismatches listed - name those in the commit message and update tests.
Expand All @@ -34,7 +25,7 @@ If Google updates its [PhoneLib](https://github.com/google/libphonenumber), this

7. Commit & Push the Snapshot with a message like:
```
Use PhoneLib X.YY.ZZ and prepare release
Use LibPhoneNumber X.YY.ZZ and prepare release
```

8. Go to Github and create a pull request for the branch.
Expand All @@ -43,7 +34,7 @@ If Google updates its [PhoneLib](https://github.com/google/libphonenumber), this

10. After merge has finished, draft a new Release. Use as tag the ```v```+ the version number of the pom, where you removed ```-SNAPSHOT```. As Release title use ```PhoneLib X.YY.ZZ``` and add a message like:
```
Use the latest PhoneLib version from four days ago.
Use the latest LibPhoneNumber version from four days ago.
```
Keep the flag "Set as the latest release" and press Publish release.

Expand All @@ -62,4 +53,4 @@ If Google updates its [PhoneLib](https://github.com/google/libphonenumber), this

16. Delete the branch ```phonelib/X_YY_ZZ```.

Congratulation! You have updated the project to the [current PhoneLib version](https://central.sonatype.com/artifact/com.googlecode.libphonenumber/libphonenumber).
Congratulation! You have updated the project to the [current LibPhoneNumber version](https://central.sonatype.com/artifact/com.googlecode.libphonenumber/libphonenumber).
Loading
Loading