Skip to content
This repository has been archived by the owner on Jun 28, 2022. It is now read-only.

Sample: Use slashes instead of underscores in PHP namespaces for nested enums and messages #2757

Merged
merged 14 commits into from
May 29, 2019
Merged

Sample: Use slashes instead of underscores in PHP namespaces for nested enums and messages #2757

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ab4d4e1
PHP nested message/enum from _ to /
yihanzhen May 7, 2019
c16ae1d
add more tests
yihanzhen May 20, 2019
e20da11
Merge branch 'master' into php-nested
yihanzhen May 20, 2019
a9c88e0
Merge branch 'master' into php-nested
yihanzhen May 22, 2019
d874de3
add more tests
yihanzhen May 22, 2019
5920a1c
Merge branch 'php-nested' of github.com:hzyi-google/gapic-generator i…
yihanzhen May 22, 2019
0a33ce0
more tests
yihanzhen May 22, 2019
fcde582
add more tests
yihanzhen May 24, 2019
a10f208
Merge branch 'master' into php-nested
yihanzhen May 24, 2019
f9b09ad
conflicts
yihanzhen May 24, 2019
ab551c1
update assertThat
yihanzhen May 24, 2019
8269638
review
yihanzhen May 24, 2019
36fb70e
end of file blank line
yihanzhen May 24, 2019
3ad0953
Merge branch 'master' into php-nested
yihanzhen May 29, 2019
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
80 changes: 30 additions & 50 deletions src/main/java/com/google/api/codegen/transformer/php/PhpModelTypeNameConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
import com.google.api.codegen.util.TypedValue;
import com.google.api.codegen.util.php.PhpTypeTable;
import com.google.api.tools.framework.model.EnumValue;
import com.google.api.tools.framework.model.MessageType;
import com.google.api.tools.framework.model.ProtoElement;
import com.google.api.tools.framework.model.ProtoFile;
import com.google.api.tools.framework.model.TypeRef;
import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.protobuf.DescriptorProtos.FieldDescriptorProto.Type;
import java.util.LinkedList;

public class PhpModelTypeNameConverter extends ModelTypeNameConverter {

Expand Down Expand Up @@ -140,9 +140,8 @@ public TypeName getTypeName(ProtoElement elem) {
}

/**
* This function recursively determines the PHP type name for a proto message. Recursion is
* required because in PHP nested messages are handled differently from non-nested messages. For
* example, the following proto definition: <code>
* This function determines the PHP type name for a proto message. For example, the following
* proto definition: <code>
* package example;
* message Top {
* message Nested {
Expand All @@ -153,67 +152,48 @@ public TypeName getTypeName(ProtoElement elem) {
*
* <ul>
* <li>\Example\Top
* <li>\Example\Top_Nested
* <li>\Example\Top_Nested_DeepNested
* <li>\Example\Top\Nested
* <li>\Example\Top\Nested\DeepNested
* </ul>
*
* <p>To correctly output these type names, we need to check whether the parent of a proto element
* is a message, and if so use '_' as a separator.
*/
private TypeName getTypeName(ProtoElement elem, int maxDepth) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maxDepth is unused now - can we remove it?

ProtoElement parent = elem.getParent();
if (parent != null && parent instanceof MessageType) {
MessageType parentMessage = (MessageType) parent;
if (parentMessage.isCyclic()) {
throw new IllegalStateException(
"Cannot determine type for cyclic message: " + parentMessage);
}
if (maxDepth == 0) {
throw new IllegalStateException("Cannot determine type for deeply nested message");
}

String parentFullName = getTypeName(parent, maxDepth - 1).getFullName();
String fullName = String.format("%s_%s", parentFullName, elem.getSimpleName());
String nickName = fullName.substring(fullName.lastIndexOf("\\") + 1);

return new TypeName(fullName, nickName);
}
return typeNameConverter.getTypeName(getTypeNameString(elem));
}

/**
* This function determines the type name as follows: If the proto type name is in TYPE_NAME_MAP,
* return that value. Else, split on ".", prepend '\' and capitalize each component of the
* namespace except the message name
*/
private static String getTypeNameString(ProtoElement elem) {
String fullName = elem.getFullName();
if (TYPE_NAME_MAP.containsKey(fullName)) {
return TYPE_NAME_MAP.get(fullName);
return typeNameConverter.getTypeName(TYPE_NAME_MAP.get(fullName));
}
String[] components = fullName.split("\\.");
String shortName = components[components.length - 1];

StringBuilder builder = new StringBuilder();
LinkedList<String> components = new LinkedList<>();

while (elem != null && !(elem instanceof ProtoFile)) {
components.addFirst(elem.getSimpleName());
elem = elem.getParent();
}

ProtoElement parentElem = elem.getParent();
if (parentElem != null && parentElem instanceof ProtoFile) {
ProtoFile protoFile = (ProtoFile) parentElem;
// Create the type name based on protobuf PHP namespace
if (elem != null && elem instanceof ProtoFile) {
ProtoFile protoFile = (ProtoFile) elem;
String namespace = protoFile.getProto().getOptions().getPhpNamespace();
if (Strings.isNullOrEmpty(namespace)) {
for (int index = 0; index < components.length - 1; index++) {
if (!Strings.isNullOrEmpty(namespace)) {
builder.append('\\').append(CharMatcher.is('\\').trimFrom(namespace));
for (String component : components) {
builder
.append('\\')
.append(components[index].substring(0, 1).toUpperCase())
.append(components[index].substring(1));
.append("\\")
.append(component.substring(0, 1).toUpperCase())
.append(component.substring(1));
return typeNameConverter.getTypeName(builder.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of this for loop if we always return on the first element?

}
} else {
builder.append('\\').append(CharMatcher.is('\\').trimFrom(namespace));
}
}

builder.append('\\').append(shortName);
return builder.toString();
// Create the type name based on the element's full name
for (String component : fullName.split("\\.")) {
builder
.append("\\")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step of splitting, appending \, capitalizing, and rejoining is duplicated. Consider the following refactor: create a helper function:

private static String makeNamespacePiece(String piece) {
  return "\\" + piece.substring(0, 1).toUpperCase() + piece.substring(1);
}

Then use streams:

String s = fullName.split("\\.").stream().transform(makeNamespacePiece).joinStrings();

(I don't think joinStrings() is a thing, but use a collector to join the strings in some way).

.append(component.substring(0, 1).toUpperCase())
.append(component.substring(1));
}
return typeNameConverter.getTypeName(builder.toString());
}

/**
Expand Down
74 changes: 37 additions & 37 deletions src/test/java/com/google/api/codegen/gapic/testdata/php/php_library.baseline
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ sampleGetBigNothing();
require __DIR__ . '/../../vendor/autoload.php';

use Google\Cloud\Example\Library\V1\LibraryServiceClient;
use Google\Example\Library\V1\SomeMessage_Alignment;
use Google\Example\Library\V1\SomeMessage\Alignment;

function sampleGetBook()
{
Expand All @@ -1057,7 +1057,7 @@ function sampleGetBook()
$booleanKeyVal = $response->getMapBoolKey()[true];
$mapValueField = $response->getMapMessageValue()['key']->getField();
printf("Test printing map fields: %s" . PHP_EOL, print_r($response->getMapListValueValue()['quoted_key'], true));
printf("Test printing enum fields of a map value: %s" . PHP_EOL, SomeMessage_Alignment::name($response->getMapMessageValue()['key']->getAlignment()));
printf("Test printing enum fields of a map value: %s" . PHP_EOL, Alignment::name($response->getMapMessageValue()['key']->getAlignment()));
} finally {
$libraryServiceClient->close();
}
Expand Down Expand Up @@ -1144,7 +1144,7 @@ sampleStreamBooks();
require __DIR__ . '/../../vendor/autoload.php';

use Google\Cloud\Example\Library\V1\LibraryServiceClient;
use Google\Example\Library\V1\Comment_Stage;
use Google\Example\Library\V1\Comment\Stage;
use Google\Example\Library\V1\DiscussBookRequest;

/** Testing calling forms */
Expand All @@ -1166,7 +1166,7 @@ function sampleMonologAboutBook()
$stream->write($request);
}
$response = $stream->readResponse();
printf("The stage of the comment is: %s" . PHP_EOL, Comment_Stage::name($response->getStage()));
printf("The stage of the comment is: %s" . PHP_EOL, Stage::name($response->getStage()));
} finally {
$libraryServiceClient->close();
}
Expand Down Expand Up @@ -1202,7 +1202,7 @@ sampleMonologAboutBook();
require __DIR__ . '/../../vendor/autoload.php';

use Google\Cloud\Example\Library\V1\LibraryServiceClient;
use Google\Example\Library\V1\Comment_Stage;
use Google\Example\Library\V1\Comment\Stage;
use Google\Example\Library\V1\DiscussBookRequest;

/** Testing calling forms */
Expand All @@ -1221,7 +1221,7 @@ function sampleMonologAboutBook()
$requests = [$request];
$stream = $libraryServiceClient->monologAboutBook();
$response = $stream->writeAllAndReadResponse($requests);
printf("The stage of the comment is: %s" . PHP_EOL, Comment_Stage::name($response->getStage()));
printf("The stage of the comment is: %s" . PHP_EOL, Stage::name($response->getStage()));
} finally {
$libraryServiceClient->close();
}
Expand Down Expand Up @@ -1993,13 +1993,13 @@ use Google\Example\Library\V1\StreamShelvesRequest;
use Google\Example\Library\V1\StreamShelvesResponse;
use Google\Example\Library\V1\StringBuilder;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerEnum;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerMessage;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_OptionalMapEntry;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_RequiredMapEntry;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerEnum;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerMessage;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\OptionalMapEntry;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\RequiredMapEntry;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsResponse;
use Google\Example\Library\V1\UpdateBookIndexRequest;
use Google\Example\Library\V1\UpdateBookIndexRequest_IndexMapEntry;
use Google\Example\Library\V1\UpdateBookIndexRequest\IndexMapEntry;
use Google\Example\Library\V1\UpdateBookRequest;
use Google\LongRunning\Operation;
use Google\Protobuf\Any;
Expand Down Expand Up @@ -3128,8 +3128,8 @@ class LibraryServiceGapicClient
* try {
* $formattedName = $libraryServiceClient->bookName('[SHELF_ID]', '[BOOK_ID]');
* $comment = '';
* $stage = Comment_Stage::UNSET;
* $alignment = SomeMessage2_SomeMessage3_Alignment::CHAR;
* $stage = Stage::UNSET;
* $alignment = Alignment::CHAR;
* $commentsElement = new Comment();
* $commentsElement->setComment($comment);
* $commentsElement->setStage($stage);
Expand Down Expand Up @@ -3944,10 +3944,10 @@ class LibraryServiceGapicClient
* $requiredSingularFloat = 0.0;
* $requiredSingularDouble = 0.0;
* $requiredSingularBool = false;
* $requiredSingularEnum = TestOptionalRequiredFlatteningParamsRequest_InnerEnum::ZERO;
* $requiredSingularEnum = InnerEnum::ZERO;
* $requiredSingularString = '';
* $requiredSingularBytes = '';
* $requiredSingularMessage = new TestOptionalRequiredFlatteningParamsRequest_InnerMessage();
* $requiredSingularMessage = new InnerMessage();
* $formattedRequiredSingularResourceName = $libraryServiceClient->bookName('[SHELF_ID]', '[BOOK_ID]');
* $formattedRequiredSingularResourceNameOneof = $libraryServiceClient->bookName('[SHELF_ID]', '[BOOK_ID]');
* $formattedRequiredSingularResourceNameCommon = $libraryServiceClient->projectName('[PROJECT]');
Expand Down Expand Up @@ -3979,10 +3979,10 @@ class LibraryServiceGapicClient
* @param float $requiredSingularFloat
* @param float $requiredSingularDouble
* @param bool $requiredSingularBool
* @param int $requiredSingularEnum For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerEnum}
* @param int $requiredSingularEnum For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerEnum}
* @param string $requiredSingularString
* @param string $requiredSingularBytes
* @param TestOptionalRequiredFlatteningParamsRequest_InnerMessage $requiredSingularMessage
* @param InnerMessage $requiredSingularMessage
* @param string $requiredSingularResourceName
* @param string $requiredSingularResourceNameOneof
* @param string $requiredSingularResourceNameCommon
Expand All @@ -3993,10 +3993,10 @@ class LibraryServiceGapicClient
* @param float[] $requiredRepeatedFloat
* @param float[] $requiredRepeatedDouble
* @param bool[] $requiredRepeatedBool
* @param int[] $requiredRepeatedEnum For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerEnum}
* @param int[] $requiredRepeatedEnum For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerEnum}
* @param string[] $requiredRepeatedString
* @param string[] $requiredRepeatedBytes
* @param TestOptionalRequiredFlatteningParamsRequest_InnerMessage[] $requiredRepeatedMessage
* @param InnerMessage[] $requiredRepeatedMessage
* @param string[] $requiredRepeatedResourceName
* @param string[] $requiredRepeatedResourceNameOneof
* @param string[] $requiredRepeatedResourceNameCommon
Expand All @@ -4011,10 +4011,10 @@ class LibraryServiceGapicClient
* @type float $optionalSingularDouble
* @type bool $optionalSingularBool
* @type int $optionalSingularEnum
* For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerEnum}
* For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerEnum}
* @type string $optionalSingularString
* @type string $optionalSingularBytes
* @type TestOptionalRequiredFlatteningParamsRequest_InnerMessage $optionalSingularMessage
* @type InnerMessage $optionalSingularMessage
* @type string $optionalSingularResourceName
* @type string $optionalSingularResourceNameOneof
* @type string $optionalSingularResourceNameCommon
Expand All @@ -4026,10 +4026,10 @@ class LibraryServiceGapicClient
* @type float[] $optionalRepeatedDouble
* @type bool[] $optionalRepeatedBool
* @type int[] $optionalRepeatedEnum
* For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerEnum}
* For allowed values, use constants defined on {@see \Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerEnum}
* @type string[] $optionalRepeatedString
* @type string[] $optionalRepeatedBytes
* @type TestOptionalRequiredFlatteningParamsRequest_InnerMessage[] $optionalRepeatedMessage
* @type InnerMessage[] $optionalRepeatedMessage
* @type string[] $optionalRepeatedResourceName
* @type string[] $optionalRepeatedResourceNameOneof
* @type string[] $optionalRepeatedResourceNameCommon
Expand Down Expand Up @@ -5358,7 +5358,7 @@ namespace Google\Cloud\Example\Library\Tests\System\V1;
use Google\Cloud\Example\Library\V1\LibraryServiceClient;
use Google\ApiCore\Testing\GeneratedTest;
use Google\Example\Library\V1\Book;
use Google\Example\Library\V1\Book_Rating;
use Google\Example\Library\V1\Book\Rating;
use Google\Example\Library\V1\UpdateBookRequest;
use Google\Protobuf\FieldMask;

Expand All @@ -5380,7 +5380,7 @@ class LibraryServiceSmokeTest extends GeneratedTest

$libraryServiceClient = new LibraryServiceClient();
$formattedName = $libraryServiceClient->bookName('testShelf-'. time(), $projectId);
$rating = Book_Rating::GOOD;
$rating = Rating::GOOD;
$book = new Book();
$book->setRating($rating);
$libraryServiceClient->updateBook($formattedName, $book);
Expand Down Expand Up @@ -5424,7 +5424,7 @@ use Google\Example\Library\V1\Book;
use Google\Example\Library\V1\BookFromAnywhere;
use Google\Example\Library\V1\BookFromArchive;
use Google\Example\Library\V1\Comment;
use Google\Example\Library\V1\Comment_Stage;
use Google\Example\Library\V1\Comment\Stage;
use Google\Example\Library\V1\CreateBookRequest;
use Google\Example\Library\V1\CreateShelfRequest;
use Google\Example\Library\V1\DeleteBookRequest;
Expand All @@ -5450,13 +5450,13 @@ use Google\Example\Library\V1\PublishSeriesRequest;
use Google\Example\Library\V1\PublishSeriesResponse;
use Google\Example\Library\V1\SeriesUuid;
use Google\Example\Library\V1\Shelf;
use Google\Example\Library\V1\SomeMessage2_SomeMessage3_Alignment;
use Google\Example\Library\V1\SomeMessage2\SomeMessage3\Alignment;
use Google\Example\Library\V1\StreamBooksRequest;
use Google\Example\Library\V1\StreamShelvesRequest;
use Google\Example\Library\V1\StreamShelvesResponse;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerEnum;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest_InnerMessage;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerEnum;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsRequest\InnerMessage;
use Google\Example\Library\V1\TestOptionalRequiredFlatteningParamsResponse;
use Google\Example\Library\V1\UpdateBookIndexRequest;
use Google\Example\Library\V1\UpdateBookRequest;
Expand Down Expand Up @@ -6535,8 +6535,8 @@ class LibraryServiceClientTest extends GeneratedTest
// Mock request
$formattedName = $client->bookName('[SHELF_ID]', '[BOOK_ID]');
$comment = '95';
$stage = Comment_Stage::UNSET;
$alignment = SomeMessage2_SomeMessage3_Alignment::CHAR;
$stage = Stage::UNSET;
$alignment = Alignment::CHAR;
$commentsElement = new Comment();
$commentsElement->setComment($comment);
$commentsElement->setStage($stage);
Expand Down Expand Up @@ -6585,8 +6585,8 @@ class LibraryServiceClientTest extends GeneratedTest
// Mock request
$formattedName = $client->bookName('[SHELF_ID]', '[BOOK_ID]');
$comment = '95';
$stage = Comment_Stage::UNSET;
$alignment = SomeMessage2_SomeMessage3_Alignment::CHAR;
$stage = Stage::UNSET;
$alignment = Alignment::CHAR;
$commentsElement = new Comment();
$commentsElement->setComment($comment);
$commentsElement->setStage($stage);
Expand Down Expand Up @@ -7726,10 +7726,10 @@ class LibraryServiceClientTest extends GeneratedTest
$requiredSingularFloat = -7514705.0;
$requiredSingularDouble = 1.9111005E8;
$requiredSingularBool = true;
$requiredSingularEnum = TestOptionalRequiredFlatteningParamsRequest_InnerEnum::ZERO;
$requiredSingularEnum = InnerEnum::ZERO;
$requiredSingularString = 'requiredSingularString-1949894503';
$requiredSingularBytes = '-29';
$requiredSingularMessage = new TestOptionalRequiredFlatteningParamsRequest_InnerMessage();
$requiredSingularMessage = new InnerMessage();
$formattedRequiredSingularResourceName = $client->bookName('[SHELF_ID]', '[BOOK_ID]');
$formattedRequiredSingularResourceNameOneof = $client->bookName('[SHELF_ID]', '[BOOK_ID]');
$formattedRequiredSingularResourceNameCommon = $client->projectName('[PROJECT]');
Expand Down Expand Up @@ -7878,10 +7878,10 @@ class LibraryServiceClientTest extends GeneratedTest
$requiredSingularFloat = -7514705.0;
$requiredSingularDouble = 1.9111005E8;
$requiredSingularBool = true;
$requiredSingularEnum = TestOptionalRequiredFlatteningParamsRequest_InnerEnum::ZERO;
$requiredSingularEnum = InnerEnum::ZERO;
$requiredSingularString = 'requiredSingularString-1949894503';
$requiredSingularBytes = '-29';
$requiredSingularMessage = new TestOptionalRequiredFlatteningParamsRequest_InnerMessage();
$requiredSingularMessage = new InnerMessage();
$formattedRequiredSingularResourceName = $client->bookName('[SHELF_ID]', '[BOOK_ID]');
$formattedRequiredSingularResourceNameOneof = $client->bookName('[SHELF_ID]', '[BOOK_ID]');
$formattedRequiredSingularResourceNameCommon = $client->projectName('[PROJECT]');
Expand Down
Loading