Skip to content

DS-1747: Support tags on branches #1696

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

Merged
merged 2 commits into from
Nov 20, 2025
Merged

DS-1747: Support tags on branches #1696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c259fb
DS-1747: Support tags on branches
mujammil10 Oct 31, 2025
5e2789b
Add branchId as a field in Tags dynamo
mujammil10 Nov 19, 2025
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
22 changes: 14 additions & 8 deletions xyz-hub-service/src/main/java/com/here/xyz/hub/config/dynamo/DynamoTagConfigClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.amazonaws.services.dynamodbv2.model.ReturnValue;
import com.here.xyz.XyzSerializable;
import com.here.xyz.hub.config.TagConfigClient;
import com.here.xyz.models.hub.Ref;
import com.here.xyz.models.hub.Tag;
import com.here.xyz.util.service.Core;
import com.here.xyz.util.service.aws.dynamo.DynamoClient;
Expand Down Expand Up @@ -221,14 +222,19 @@ private static List<Tag> tagDataToTags(List<Map<String, AttributeValue>> items)
if (items == null || items.isEmpty())
return Collections.emptyList();

return items.stream().map(tagData -> new Tag()
.withId(tagData.get("id").getS())
.withSpaceId(tagData.get("spaceId").getS())
.withVersion(Long.parseLong(tagData.get("version").getN()))
.withSystem( tagData.get("system") != null ? tagData.get("system").getBOOL() : false )
.withDescription(tagData.get("description") != null ? tagData.get("description").getS() : "")
.withAuthor(tagData.get("author") != null ? tagData.get("author").getS() : "system")
.withCreatedAt(tagData.get("createdAt") != null ? Long.parseLong(tagData.get("createdAt").getN()) : -1)
return items.stream().map(tagData -> {
long version = Long.parseLong(tagData.get("version").getN());
String branchId = tagData.get("branchId") != null ? tagData.get("branchId").getS() : Ref.MAIN;
return new Tag()
.withId(tagData.get("id").getS())
.withSpaceId(tagData.get("spaceId").getS())
.withVersion(version)
.withVersionRef(Ref.fromBranchId(branchId, version))
.withSystem(tagData.get("system") != null ? tagData.get("system").getBOOL() : false)
.withDescription(tagData.get("description") != null ? tagData.get("description").getS() : "")
.withAuthor(tagData.get("author") != null ? tagData.get("author").getS() : "system")
.withCreatedAt(tagData.get("createdAt") != null ? Long.parseLong(tagData.get("createdAt").getN()) : -1);
}
).collect(Collectors.toList());
}

Expand Down
66 changes: 41 additions & 25 deletions xyz-hub-service/src/main/java/com/here/xyz/hub/rest/TagApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package com.here.xyz.hub.rest;

import static com.here.xyz.hub.rest.ApiParam.Path.INCLUDE_SYSTEM_TAGS;
import static com.here.xyz.models.hub.Ref.HEAD;
import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND;

import com.fasterxml.jackson.core.JsonProcessingException;
Expand All @@ -28,6 +29,9 @@
import com.here.xyz.hub.connectors.models.Space;
import com.here.xyz.hub.rest.ApiParam.Path;
import com.here.xyz.hub.rest.ApiParam.Query;
import com.here.xyz.hub.task.BranchHandler;
import com.here.xyz.hub.task.FeatureTask;
import com.here.xyz.models.hub.Ref;
import com.here.xyz.models.hub.Tag;
import com.here.xyz.responses.ChangesetsStatisticsResponse;
import com.here.xyz.util.service.BaseHttpServerVerticle;
Expand Down Expand Up @@ -61,7 +65,7 @@ private void createTag(RoutingContext context) {
getMarker(context),
getSpaceId(context),
tag.getId(),
tag.getVersion(),
tag.getVersionRef(),
tag.isSystem(),
tag.getDescription(),
author,
Expand Down Expand Up @@ -119,19 +123,15 @@ private void updateTag(RoutingContext context) {

String author = BaseHttpServerVerticle.getAuthor(context);
deserializeTag(context.body().asString())
.compose(tag -> updateTag(marker, spaceId, tagId, tag.getVersion(), author, tag.getDescription()))
.compose(tag -> updateTag(marker, spaceId, tagId, tag.getVersionRef(), author, tag.getDescription()))
.onSuccess(tag -> sendResponse(context, HttpResponseStatus.OK.code(), tag))
.onFailure(t -> sendErrorResponse(context, t));
}

public static Future<Tag> updateTag(Marker marker, String spaceId, String tagId, long version, String author, String description) {
public static Future<Tag> updateTag(Marker marker, String spaceId, String tagId, Ref versionRef, String author, String description) {
if (spaceId == null)
return Future.failedFuture(new ValidationException("Invalid parameter"));

//FIXME: Neither -2 nor -1 are valid versions
if (version < -2)
return Future.failedFuture(new ValidationException("Invalid version parameter"));

if (!Tag.isDescriptionValid(description))
return Future.failedFuture(new ValidationException("Invalid description parameter, description must be less than 255 characters"));

Expand All @@ -140,24 +140,24 @@ public static Future<Tag> updateTag(Marker marker, String spaceId, String tagId,
.compose(loadedTag -> loadedTag == null
? Future.failedFuture(new HttpException(NOT_FOUND, "Tag " + tagId + " not found"))
: Future.succeededFuture(loadedTag))
.compose(loadedTag -> Service.tagConfigClient.storeTag(marker, new Tag()
.withId(tagId)
.withSpaceId(spaceId)
.withVersion(version)
.withSystem(loadedTag.isSystem())
.withDescription(description == null ? loadedTag.getDescription() : description))
.map(loadedTag))
.compose(loadedTag -> resolveVersionRef(marker, spaceId, versionRef)
.compose(resolvedVersionRef -> Service.tagConfigClient.storeTag(marker, new Tag()
.withId(tagId)
.withSpaceId(spaceId)
.withVersionRef(resolvedVersionRef)
.withSystem(loadedTag.isSystem())
.withDescription(description == null ? loadedTag.getDescription() : description))
.map(loadedTag.withVersionRef(resolvedVersionRef))))
.map(loadedTag -> loadedTag
.withVersion(version)
.withDescription(description == null ? loadedTag.getDescription() : description));
}

public static Future<Tag> createTag(Marker marker, String spaceId, String tagId, String author) {
return createTag(marker, spaceId, tagId, -2, true, "", author, Core.currentTimeMillis());
return createTag(marker, spaceId, tagId, new Ref(HEAD), true, "", author, Core.currentTimeMillis());
}

// TODO auth
public static Future<Tag> createTag(Marker marker, String spaceId, String tagId, long version, boolean system,
public static Future<Tag> createTag(Marker marker, String spaceId, String tagId, Ref versionRef, boolean system,
String description, String author, long createdAt) {
if (spaceId == null) {
return Future.failedFuture(new ValidationException("Invalid parameter"));
Expand All @@ -173,19 +173,15 @@ public static Future<Tag> createTag(Marker marker, String spaceId, String tagId,
);
}

if (version < -2) {
return Future.failedFuture(new ValidationException("Invalid version parameter"));
}

final Future<Space> spaceFuture = getSpaceIfActive(marker, spaceId);
final Future<ChangesetsStatisticsResponse> changesetFuture = ChangesetApi.getChangesetStatistics(marker, Future::succeededFuture, spaceId);
final Future<Ref> resolveVersionRef = resolveVersionRef(marker, spaceId, versionRef);
final Future<Void> checkExistingAlias = checkExistingAlias(marker, spaceId, tagId);

return Future.all(spaceFuture, changesetFuture, checkExistingAlias).compose(cf -> {
return Future.all(spaceFuture, resolveVersionRef, checkExistingAlias).compose(cf -> {
final Tag tag = new Tag()
.withId(tagId)
.withSpaceId(spaceId)
.withVersion(version == -2 ? changesetFuture.result().getMaxVersion() : version)
.withVersionRef(resolveVersionRef.result())
.withSystem(system)
.withDescription(description)
.withAuthor(author)
Expand All @@ -204,13 +200,18 @@ public static Future<Tag> deleteTag(Marker marker, String spaceId, String tagId)
.compose(none -> Service.tagConfigClient.deleteTag(marker, tagId, spaceId));
}

//TODO: Check why does this return a Future ?
private static Future<Tag> deserializeTag(String body) {
if (StringUtils.isBlank(body)) {
return Future.failedFuture(new ValidationException("Unable to parse body"));
}

try {
return Future.succeededFuture(XyzSerializable.deserialize(body, Tag.class));
Tag tag = XyzSerializable.deserialize(body, Tag.class);
if (tag.getVersionRef() == null) {
tag.setVersionRef(tag.getVersion() == -2 ? new Ref(HEAD) : new Ref(tag.getVersion()));
}
return Future.succeededFuture(tag);
} catch (JsonProcessingException e) {
return Future.failedFuture(new ValidationException("Unable to parse body"));
}
Expand All @@ -226,6 +227,21 @@ private static Future<Space> getSpaceIfActive(Marker marker, String spaceId) {
: Future.succeededFuture(s));
}

private static Future<Ref> resolveVersionRef(Marker marker, String spaceId, Ref versionRef) {
if (versionRef.isTag())
throw new IllegalArgumentException("A tag cannot be used as Ref");

Future<Ref> future = Future.succeededFuture(versionRef);
if (!versionRef.isMainBranch()) {
future = FeatureTask.getReferencedBranch(spaceId, versionRef).map(versionRef);
}

if (versionRef.isHead()) {
future = future.compose(v -> BranchHandler.resolveRefHeadVersion(marker, spaceId, versionRef));
}
return future;
}

private static String getAuthor(Future<Tag> tagFuture, String author) {
return tagFuture.result().getAuthor().isEmpty() ? author : tagFuture.result().getAuthor();
}
Expand Down
2 changes: 1 addition & 1 deletion xyz-hub-service/src/main/java/com/here/xyz/hub/task/BranchHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private static Future<Ref> resolveRef(Marker marker, String spaceId, Ref ref) {
return resolveRefHeadVersion(marker, spaceId, ref);
}

private static Future<Ref> resolveRefHeadVersion(Marker marker, String spaceId, Ref ref) {
public static Future<Ref> resolveRefHeadVersion(Marker marker, String spaceId, Ref ref) {
if (ref.isHead())
return FeatureQueryApi.getStatistics(marker, spaceId, EXTENSION, ref, true, false)
.map(statistics -> new Ref(ref.getBranch() + ":" + statistics.getMaxVersion().getValue()));
Expand Down
71 changes: 31 additions & 40 deletions xyz-hub-service/src/main/java/com/here/xyz/hub/task/FeatureTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import com.here.xyz.hub.auth.Authorization;
import com.here.xyz.hub.auth.FeatureAuthorization;
import com.here.xyz.hub.config.BranchConfigClient;
import com.here.xyz.hub.config.TagConfigClient;
import com.here.xyz.hub.connectors.RpcClient;
import com.here.xyz.hub.connectors.models.Connector;
import com.here.xyz.hub.connectors.models.Space;
Expand Down Expand Up @@ -147,13 +148,25 @@ private FeatureTask(T event, RoutingContext context, ApiResponseType responseTyp
this.requestBodySize = requestBodySize;
}

//TODO: Merge into resolveVersionRef
void resolveBranch(final X task, final Callback<X> callback) {
void resolveVersionRef(final X task, final Callback<X> callback) {
try {
if (task.getEvent() instanceof ContextAwareEvent<?> event) {
resolveBranchFor(event, task.space)
.onFailure(callback::exception)
.onSuccess(v -> callback.call(task));
Future<Void> future = Future.succeededFuture();
if (event.getRef().isTag()) {
future = TagConfigClient.getInstance().getTag(task.getMarker(), event.getRef().getTag(), task.space.getId())
.compose(tag -> {
if (tag == null) {
//It could be a branch
event.setRef(Ref.fromBranchId(event.getRef().getTag()));
} else {
event.setRef(tag.getVersionRef());
}
return Future.succeededFuture();
});
}
future.compose(v -> resolveBranchFor(event, task.space))
.onFailure(callback::exception)
.onSuccess(v -> callback.call(task));
} else
callback.call(task);
} catch (Exception e) {
Expand All @@ -162,27 +175,11 @@ void resolveBranch(final X task, final Callback<X> callback) {
}

public static Future<Void> resolveBranchFor(ContextAwareEvent<?> event, Space space) {
if (event.getRef() == null || (event.getRef().isMainBranch() && !event.getRef().isTag()))
if (event.getRef() == null || event.getRef().isMainBranch())
return Future.succeededFuture();

Ref branchRef = event.getRef().isTag() ? Ref.fromBranchId(event.getRef().getTag()) : event.getRef();
Future<Branch> future = getReferencedBranch(space.getId(), branchRef);
if (event.getRef().isTag()) {
// Check if it's a branch or a tag
future = future.compose(branch -> {
if (branch != null)
event.setRef(branchRef);
return Future.succeededFuture(branch);
})
.recover(t -> {
// If the branch does not exist, then it could be a normal tag
if (t instanceof HttpException e && e.status == NOT_FOUND)
return Future.succeededFuture();
return Future.failedFuture(t);
});
}

return future.compose(referencedBranch -> {
return getReferencedBranch(space.getId(), event.getRef())
.compose(referencedBranch -> {
if (referencedBranch != null) {
LinkedList<Ref> branchPath = new LinkedList<>(referencedBranch.getBranchPath());
event.withNodeId(referencedBranch.getNodeId())
Expand Down Expand Up @@ -211,7 +208,7 @@ public static Future<Branch> getReferencedBranch(String spaceId, Ref ref) {
return BranchConfigClient.getInstance().load(spaceId, ref.getBranch())
.compose(branch -> {
if (branch == null)
return Future.failedFuture(new HttpException(NOT_FOUND, "Branch \"" + ref.getBranch() + "\" was not found on resource \"" + spaceId + "\"."));
return Future.failedFuture(new HttpException(NOT_FOUND, "Tag or Branch \"" + ref.getBranch() + "\" was not found on resource \"" + spaceId + "\"."));
return Future.succeededFuture(branch);
});
}
Expand Down Expand Up @@ -342,8 +339,7 @@ public GeometryQuery(GetFeaturesByGeometryEvent event, RoutingContext context, A
public TaskPipeline<GeometryQuery> createPipeline() {
return TaskPipeline.create(this)
.then(FeatureTaskHandler::resolveSpace)
.then(this::resolveBranch) //TODO: Merge into resolveVersionRef and move back again after checkImmutability
.then(FeatureTaskHandler::resolveVersionRef)
.then(this::resolveVersionRef)
.then(this::resolveRefSpace)
.then(this::resolveRefConnector)
.then(Authorization::authorizeComposite)
Expand Down Expand Up @@ -473,8 +469,7 @@ public BBoxQuery(GetFeaturesByBBoxEvent event, RoutingContext context, ApiRespon
public TaskPipeline<BBoxQuery> createPipeline() {
return TaskPipeline.create(this)
.then(FeatureTaskHandler::resolveSpace)
.then(this::resolveBranch) //TODO: Merge into resolveVersionRef and move back again after checkImmutability
.then(FeatureTaskHandler::resolveVersionRef)
.then(this::resolveVersionRef)
.then(Authorization::authorizeComposite)
.then(FeatureAuthorization::authorize)
.then(FeatureTaskHandler::checkImmutability)
Expand Down Expand Up @@ -504,8 +499,7 @@ public TileQuery(GetFeaturesByTileEvent event, RoutingContext context, ApiRespon
public TaskPipeline<TileQuery> createPipeline() {
return TaskPipeline.create(this)
.then(FeatureTaskHandler::resolveSpace)
.then(this::resolveBranch) //TODO: Merge into resolveVersionRef and move back again after checkImmutability
.then(FeatureTaskHandler::resolveVersionRef)
.then(this::resolveVersionRef)
.then(Authorization::authorizeComposite)
.then(FeatureAuthorization::authorize)
.then(FeatureTaskHandler::checkImmutability)
Expand Down Expand Up @@ -540,8 +534,7 @@ public IdsQuery(GetFeaturesByIdEvent event, RoutingContext context, ApiResponseT
public TaskPipeline<IdsQuery> createPipeline() {
return TaskPipeline.create(this)
.then(FeatureTaskHandler::resolveSpace)
.then(this::resolveBranch) //TODO: Merge into resolveVersionRef and move back again after auth
.then(FeatureTaskHandler::resolveVersionRef)
.then(this::resolveVersionRef)
.then(Authorization::authorizeComposite)
.then(FeatureAuthorization::authorize)
.then(FeatureTaskHandler::checkImmutability)
Expand All @@ -562,8 +555,7 @@ public IterateQuery(IterateFeaturesEvent event, RoutingContext context, ApiRespo
public TaskPipeline<IterateQuery> createPipeline() {
return TaskPipeline.create(this)
.then(FeatureTaskHandler::resolveSpace)
.then(this::resolveBranch) //TODO: Merge into resolveVersionRef and move back again after auth
.then(FeatureTaskHandler::resolveVersionRef)
.then(this::resolveVersionRef)
.then(Authorization::authorizeComposite)
.then(FeatureAuthorization::authorize)
.then(FeatureTaskHandler::checkImmutability)
Expand All @@ -584,8 +576,7 @@ public SearchQuery(SearchForFeaturesEvent<?> event, RoutingContext context, ApiR
public TaskPipeline<SearchQuery> createPipeline() {
return TaskPipeline.create(this)
.then(FeatureTaskHandler::resolveSpace)
.then(this::resolveBranch) //TODO: Merge into resolveVersionRef and move back again after auth
.then(FeatureTaskHandler::resolveVersionRef)
.then(this::resolveVersionRef)
.then(Authorization::authorizeComposite)
.then(FeatureAuthorization::authorize)
.then(FeatureTaskHandler::checkImmutability)
Expand All @@ -610,7 +601,7 @@ public TaskPipeline<GetStatistics> createPipeline() {
.then(FeatureTaskHandler::resolveSpace)
.then(Authorization::authorizeComposite)
.then(FeatureAuthorization::authorize)
.then(this::resolveBranch) //TODO: Merge into resolveVersionRef?
.then(this::resolveVersionRef)
.then(FeatureTaskHandler::readCache)
.then(FeatureTaskHandler::invoke)
.then(FeatureTaskHandler::convertResponse)
Expand Down Expand Up @@ -639,7 +630,7 @@ public void onPreProcessed(ModifySpaceEvent event) {
public TaskPipeline<ModifySpaceQuery> createPipeline() {
return TaskPipeline.create(this)
.then(FeatureTaskHandler::resolveSpace)
.then(this::resolveBranch)
.then(this::resolveVersionRef)
.then(SpaceTaskHandler::invokeConditionally);
}
}
Expand Down Expand Up @@ -699,7 +690,7 @@ public TaskPipeline<ConditionalOperation> createPipeline() {
.then(FeatureTaskHandler::resolveSpace)
.then(FeatureTaskHandler::registerRequestMemory)
.then(FeatureTaskHandler::throttle)
.then(this::resolveBranch)
.then(this::resolveVersionRef)
.then(FeatureTaskHandler::injectSpaceParams)
.then(FeatureTaskHandler::checkPreconditions)
.then(FeatureTaskHandler::prepareModifyFeatureOp)
Expand Down
32 changes: 0 additions & 32 deletions xyz-hub-service/src/main/java/com/here/xyz/hub/task/FeatureTaskHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,38 +767,6 @@ private static <T extends FeatureTask> CompletableFuture<XyzResponse> executePro
return f;
}

static <X extends FeatureTask> void resolveVersionRef(final X task, final Callback<X> callback) {
if (!(task.getEvent() instanceof SelectiveEvent event)) {
callback.call(task);
return;
}

if (event.getRef() == null || !event.getRef().isTag()) {
callback.call(task);
return;
}

TagConfigClient.getInstance().getTag(task.getMarker(), event.getRef().getTag(), task.space.getId())
.compose(tag -> {
if (tag == null) {
return Future.failedFuture(new HttpException(NOT_FOUND, "Version ref not found: " + event.getRef().getTag()));
}

try {
event.setRef(new Ref(tag.getVersion()));
} catch (InvalidRef e) {
return Future.failedFuture(e);
}

return Future.succeededFuture(tag);
})
.onSuccess(tag -> callback.call(task))
.onFailure(t -> {
logger.warn(task.getMarker(), "Unable to resolve version ref.", t);
callback.exception(t);
});
}

private static class RpcContextHolder {
RpcContext rpcContext;
}
Expand Down
Loading
Loading