fix for #102 (356eb78b) · Commits · OSL / code / org.etsi.osl.tmf.api

src/main/java/org/etsi/osl/tmf/rcm634/api/ResourceSpecificationApiRouteBuilder.java

+17 −10

Original line number	Diff line number	Diff line
		@@ -6,10 +6,8 @@ import org.apache.camel.builder.RouteBuilder;
		import org.apache.camel.model.dataformat.JsonLibrary;
		import org.apache.commons.logging.Log;
		import org.apache.commons.logging.LogFactory;
		import org.etsi.osl.tmf.rcm634.model.ResourceSpecification;
		import org.etsi.osl.tmf.rcm634.model.ResourceSpecificationCreate;
		import org.etsi.osl.tmf.rcm634.model.ResourceSpecificationUpdate;
		import org.etsi.osl.tmf.rcm634.reposervices.ResourceSpecificationRepoService;
		import com.fasterxml.jackson.databind.JsonNode;
		import org.springframework.beans.factory.annotation.Autowired;
		import org.springframework.beans.factory.annotation.Value;
		import org.springframework.context.annotation.Configuration;
		@@ -47,27 +45,36 @@ public class ResourceSpecificationApiRouteBuilder extends RouteBuilder {
		@Override
		public void configure() throws Exception {

		/*
		* These three routes unmarshal to JsonNode rather than binding straight to a specification class,
		* because the concrete class depends on the body's own "@type" discriminator. Physical fields
		* (model/part/sku/vendor) are declared only on PhysicalResourceSpecificationUpdate — a sibling of
		* ResourceSpecificationCreate, not a supertype — so binding a physical payload to
		* ResourceSpecificationCreate silently discards them. The repo service picks the payload class and
		* the entity from that one string.
		*/

		from( CATALOG_ADD_RESOURCESPEC )
		.log(LoggingLevel.INFO, log, CATALOG_ADD_RESOURCESPEC + " message received!")
		.to("log:DEBUG?showBody=true&showHeaders=true")
		.unmarshal()
		.json( JsonLibrary.Jackson, ResourceSpecificationCreate .class, true)
		.bean( resourceSpecificationRepoService, "addResourceSpecification(${body})")
		.json( JsonLibrary.Jackson, JsonNode.class )
		.bean( resourceSpecificationRepoService, "addResourceSpecificationFromJson(${body})")
		.marshal().json( JsonLibrary.Jackson)
		.convertBodyTo( String.class );

		from( CATALOG_UPD_RESOURCESPEC )
		.log(LoggingLevel.INFO, log, CATALOG_UPD_RESOURCESPEC + " message received!")
		.to("log:DEBUG?showBody=true&showHeaders=true")
		.unmarshal().json( JsonLibrary.Jackson, ResourceSpecificationUpdate.class, true)
		.bean( resourceSpecificationRepoService, "updateResourceSpecification(${header.resourceSpecId}, ${body} )")
		.unmarshal().json( JsonLibrary.Jackson, JsonNode.class )
		.bean( resourceSpecificationRepoService, "updateResourceSpecificationFromJson(${header.resourceSpecId}, ${body} )")
		.marshal().json( JsonLibrary.Jackson)
		.convertBodyTo( String.class );

		from( CATALOG_UPDADD_RESOURCESPEC )
		.log(LoggingLevel.INFO, log, CATALOG_UPDADD_RESOURCESPEC + " message received!")
		.to("log:DEBUG?showBody=true&showHeaders=true")
		.unmarshal().json( JsonLibrary.Jackson, ResourceSpecificationCreate.class, true)
		.unmarshal().json( JsonLibrary.Jackson, JsonNode.class )
		.bean( resourceSpecificationRepoService, "addOrupdateResourceSpecificationByNameCategoryVersion(${header.aname}, ${header.acategory}, ${header.aversion}, ${body} )")
		.marshal().json( JsonLibrary.Jackson)
		.convertBodyTo( String.class );

src/main/java/org/etsi/osl/tmf/rcm634/reposervices/ResourceSpecificationRepoService.java

+129 −37

Original line number	Diff line number	Diff line
		@@ -36,6 +36,7 @@ import java.util.Optional;
		import java.util.UUID;

		import com.fasterxml.jackson.core.JsonProcessingException;
		import com.fasterxml.jackson.databind.JsonNode;
		import com.fasterxml.jackson.databind.ObjectMapper;

		import org.apache.commons.logging.Log;
		@@ -115,16 +116,75 @@ public class ResourceSpecificationRepoService {

		@Transactional
		public ResourceSpecification addResourceSpecification(@Valid ResourceSpecificationCreate resourceSpecification) {
		return addResourceSpecificationGeneric( newEntityForType( resourceSpecification.getType() ), resourceSpecification);
		}

		ResourceSpecification reSpec ;
		if (resourceSpecification.getType() !=null && ( resourceSpecification.getType().equals( "PhysicalResourceSpecification" ) \|\|
		resourceSpecification.getType().equals( "PhysicalResourceSpecificationCreate" ) )) {
		reSpec = new PhysicalResourceSpecification();
		}else {
		reSpec = new LogicalResourceSpecification();
		/**
		* Creates a resource specification from the raw request body, choosing the payload class from the
		* {@code @type} discriminator.
		*
		* <p>The JMS routes call this rather than binding straight to a class: physical fields
		* (model/part/sku/vendor) are declared only on {@link PhysicalResourceSpecificationUpdate}, so
		* unmarshalling a physical payload into {@link ResourceSpecificationCreate} silently drops them
		* before this service ever sees them. The entity and the payload are chosen from the same string, so
		* they cannot disagree.
		*/
		@Transactional
		public ResourceSpecification addResourceSpecificationFromJson(JsonNode jsonNode) throws JsonProcessingException {
		ResourceSpecificationUpdate payload = toTypedCreatePayload( jsonNode );
		return addResourceSpecificationGeneric( newEntityForType( payload.getType() ), payload );
		}

		/** Updates a resource specification from the raw request body. @type selects the payload class. */
		@Transactional
		public ResourceSpecification updateResourceSpecificationFromJson(String id, JsonNode jsonNode)
		throws JsonProcessingException {
		return updateResourceSpecification( id, toTypedCreatePayload( jsonNode ) );
		}

		/**
		* Binds a request body to the concrete payload class its {@code @type} names. Mirrors the dispatch in
		* {@code ResourceSpecificationApiController.createResourceSpecification} — the two must agree, or the
		* same JSON would mean different things over REST and over JMS.
		*
		* <p>Falls back to {@link ResourceSpecificationCreate} for an absent or unrecognised {@code @type},
		* which is what every logical producer sends.
		*/
		private ResourceSpecificationUpdate toTypedCreatePayload(JsonNode jsonNode) throws JsonProcessingException {
		String atType = jsonNode.path("@type").asText(null);

		if ( isPhysicalType( atType ) ) {
		return objectMapper.treeToValue( jsonNode, PhysicalResourceSpecificationCreate.class );
		}
		if ( isResourceFunctionType( atType ) ) {
		return objectMapper.treeToValue( jsonNode, ResourceFunctionSpecificationCreate.class );
		}
		return objectMapper.treeToValue( jsonNode, ResourceSpecificationCreate.class );
		}

		/**
		* The JPA entity a {@code @type} names. Kept beside {@link #toTypedCreatePayload} deliberately: these
		* two must stay in step, since pairing an entity with a payload that cannot carry its fields is
		* exactly the defect this replaced.
		*/
		private ResourceSpecification newEntityForType(String atType) {
		if ( isPhysicalType( atType ) ) {
		return new PhysicalResourceSpecification();
		}
		if ( isResourceFunctionType( atType ) ) {
		return new ResourceFunctionSpecification();
		}
		return new LogicalResourceSpecification();
		}

		return addResourceSpecificationGeneric(reSpec, resourceSpecification);
		private static boolean isPhysicalType(String atType) {
		return "PhysicalResourceSpecification".equals( atType )
		\|\| "PhysicalResourceSpecificationCreate".equals( atType );
		}

		private static boolean isResourceFunctionType(String atType) {
		return "ResourceFunctionSpecification".equals( atType )
		\|\| "ResourceFunctionSpecificationCreate".equals( atType );
		}

		public LogicalResourceSpecification addLogicalResourceSpecification(@Valid ResourceSpecificationCreate logicalResourceSpec) {
		@@ -323,8 +383,21 @@ public class ResourceSpecificationRepoService {
		}


		/**
		* Upserts a resource specification identified by name+category+version, from the raw request body.
		*
		* <p>Takes the body as a {@link JsonNode} so the {@code @type} discriminator can pick the payload
		* class: physical fields exist only on {@link PhysicalResourceSpecificationUpdate}, and binding a
		* physical payload to {@link ResourceSpecificationCreate} would drop them before this method runs.
		*
		* <p>Note the update branch cannot re-type an existing specification — JPA has no way to change an
		* entity's concrete class in place. One first registered as logical stays logical, and a later
		* physical payload's fields are ignored with a warning.
		*/
		@Transactional
		public ResourceSpecification addOrupdateResourceSpecificationByNameCategoryVersion(String aname, String acategory, String aversion, ResourceSpecificationCreate aesourceCreate) {
		public ResourceSpecification addOrupdateResourceSpecificationByNameCategoryVersion(String aname, String acategory, String aversion, JsonNode jsonNode) throws JsonProcessingException {

		ResourceSpecificationUpdate payload = toTypedCreatePayload( jsonNode );

		List<ResourceSpecification> rspecs = this.resourceSpecificationRepo.findByNameAndCategoryAndVersion(aname, acategory, aversion);
		ResourceSpecification result = null;
		@@ -333,14 +406,13 @@ public class ResourceSpecificationRepoService {
		if ( rspecs.size() >0 ) {
		//perform update to the first one
		String resID = rspecs.get(0).getUuid();
		result = this.updateResourceSpecification(resID, aesourceCreate);
		result = this.updateResourceSpecification(resID, payload);
		} else {
		result = this.addResourceSpecification(aesourceCreate);
		result = addResourceSpecificationGeneric( newEntityForType( payload.getType() ), payload );
		}

		ObjectMapper mapper = new ObjectMapper();
		try {
		String originaServiceAsJson = mapper.writeValueAsString( result );
		String originaServiceAsJson = objectMapper.writeValueAsString( result );
		logger.debug(originaServiceAsJson);
		} catch (JsonProcessingException e) {
		logger.error("cannot umarshall service: " + result.getName() );
		@@ -403,18 +475,6 @@ public class ResourceSpecificationRepoService {

		}

		public LogicalResourceSpecification updateLogicalResourceSpecSpecification(String id,
		@Valid ResourceSpecificationUpdate logicalResourceSpec) {
		return (LogicalResourceSpecification) this.updateResourceSpecification(id, logicalResourceSpec);
		}

		public PhysicalResourceSpecification updatePhysicalResourceSpecSpecification(String id,
		@Valid ResourceSpecificationUpdate physicalResourceSpec) {
		return (PhysicalResourceSpecification) this.updateResourceSpecification(id, physicalResourceSpec);
		}



		private ResourceSpecification updateResourceSpecDataFromAPIcall(
		ResourceSpecification resourceSpec,
		ResourceSpecificationUpdate resSpecUpd )
		@@ -435,12 +495,44 @@ public class ResourceSpecificationRepoService {

		resourceSpec.setLastUpdate( OffsetDateTime.now(ZoneOffset.UTC) );

		if (resourceSpec instanceof PhysicalResourceSpecification){
		((PhysicalResourceSpecification) resourceSpec)
		.model(( (PhysicalResourceSpecificationUpdate) resSpecUpd ).getModel() )
		.part( ( (PhysicalResourceSpecificationUpdate) resSpecUpd ).getPart())
		.sku( ( (PhysicalResourceSpecificationUpdate) resSpecUpd ).getSku())
		.vendor(( (PhysicalResourceSpecificationUpdate) resSpecUpd ).getVendor() );
		/*
		* Physical fields live only on PhysicalResourceSpecificationUpdate, which is a SIBLING of
		* ResourceSpecificationCreate (both extend ResourceSpecificationUpdate) — not a supertype. So the
		* payload must be checked as well as the entity: a physical entity paired with a plain
		* ResourceSpecificationCreate is a normal, expected combination (that is what an @type-tagged
		* message from a JMS producer looks like), and casting on the entity check alone threw
		* ClassCastException on every such call.
		*
		* Each field is copied only when present, matching the name/description/category handling above:
		* a PATCH that does not mention model must leave model alone, not null it.
		*/
		if ( resourceSpec instanceof PhysicalResourceSpecification
		&& resSpecUpd instanceof PhysicalResourceSpecificationUpdate ) {
		PhysicalResourceSpecification prs = (PhysicalResourceSpecification) resourceSpec;
		PhysicalResourceSpecificationUpdate pUpd = (PhysicalResourceSpecificationUpdate) resSpecUpd;

		if ( pUpd.getModel() != null ) {
		prs.setModel( pUpd.getModel() );
		}
		if ( pUpd.getPart() != null ) {
		prs.setPart( pUpd.getPart() );
		}
		if ( pUpd.getSku() != null ) {
		prs.setSku( pUpd.getSku() );
		}
		if ( pUpd.getVendor() != null ) {
		prs.setVendor( pUpd.getVendor() );
		}
		} else if ( isPhysicalType( resSpecUpd.getType() ) && !(resourceSpec instanceof PhysicalResourceSpecification) ) {
		/*
		* The payload claims to be physical but the stored entity is not. JPA cannot re-type an entity in
		* place, so the physical fields are dropped. Say so — a spec registered as logical first stays
		* logical forever, and silence here makes that look like data loss with no cause.
		*/
		logger.warn("Resource specification '" + resourceSpec.getName() + "' (" + resourceSpec.getUuid()
		+ ") is stored as " + resourceSpec.getClass().getSimpleName()
		+ " but the payload declares @type=" + resSpecUpd.getType()
		+ ". Physical fields are ignored; an existing specification cannot be re-typed.");
		}

src/test/java/org/etsi/osl/services/api/rcm634/ResourceSpecificationRepoServiceTest.java

0 → 100644

+253 −0

Original line number	Diff line number	Diff line
		package org.etsi.osl.services.api.rcm634;

		import com.fasterxml.jackson.databind.JsonNode;
		import com.fasterxml.jackson.databind.ObjectMapper;
		import org.etsi.osl.services.api.BaseIT;
		import org.etsi.osl.tmf.rcm634.model.LogicalResourceSpecification;
		import org.etsi.osl.tmf.rcm634.model.PhysicalResourceSpecification;
		import org.etsi.osl.tmf.rcm634.model.ResourceFunctionSpecification;
		import org.etsi.osl.tmf.rcm634.model.ResourceSpecification;
		import org.etsi.osl.tmf.rcm634.model.ResourceSpecificationUpdate;
		import org.etsi.osl.tmf.rcm634.reposervices.ResourceSpecificationRepoService;
		import org.junit.jupiter.api.Test;
		import org.springframework.beans.factory.annotation.Autowired;
		import org.springframework.security.test.context.support.WithMockUser;

		import static org.assertj.core.api.Assertions.assertThat;

		/**
		* Covers the {@code @type}-driven resource specification paths reached over JMS.
		*
		* <p>These exist because a physical specification registered over JMS used to throw
		* {@code ClassCastException: ResourceSpecificationCreate cannot be cast to
		* PhysicalResourceSpecificationUpdate} — the entity was chosen from {@code @type} while the payload
		* stayed a plain create, and nothing checked that the two agreed. It went unnoticed because the only
		* physical path with any coverage was the REST controller, which happens to keep them in step.
		*
		* <p>The Camel routes are bound to real {@code jms:queue:} endpoints, so they cannot be driven here.
		* Each route is a one-line bean expression, so these call the repo methods with the exact
		* {@link JsonNode} the route hands them.
		*
		* <p>Kept in its own class deliberately: sibling suites assert exact specification counts, and
		* {@link BaseIT} is {@code @Transactional} so each method rolls back.
		*/
		@WithMockUser(username = "osadmin", roles = {"ADMIN", "USER"})
		public class ResourceSpecificationRepoServiceTest extends BaseIT {

		@Autowired
		private ResourceSpecificationRepoService specRepoService;

		@Autowired
		private ObjectMapper objectMapper;

		private JsonNode json(String raw) throws Exception {
		return objectMapper.readTree(raw);
		}

		/**
		* The reported crash: a physical specification arriving as a plain create tagged with {@code @type}.
		* Also asserts the physical fields survive — the route used to bind them to a class that cannot hold
		* them, so they were dropped before the service ever ran.
		*/
		@Test
		public void addFromJson_physicalType_buildsPhysicalEntity_andKeepsItsFields() throws Exception {
		ResourceSpecification spec = specRepoService.addResourceSpecificationFromJson(json("""
		{
		"name": "RadioNode",
		"description": "A radio access node",
		"version": "1.0.0",
		"category": "test.category",
		"@type": "PhysicalResourceSpecification",
		"model": "gNB-3000",
		"part": "RU-1",
		"sku": "SKU-42",
		"vendor": "ACME"
		}"""));

		assertThat(spec).isInstanceOf(PhysicalResourceSpecification.class);

		PhysicalResourceSpecification physical = (PhysicalResourceSpecification) spec;
		assertThat(physical.getName()).isEqualTo("RadioNode");
		assertThat(physical.getModel()).isEqualTo("gNB-3000");
		assertThat(physical.getPart()).isEqualTo("RU-1");
		assertThat(physical.getSku()).isEqualTo("SKU-42");
		assertThat(physical.getVendor()).isEqualTo("ACME");
		}

		/** Every existing JMS producer (cridge, giter, ietf.ns, capif.invoker) sends this shape. */
		@Test
		public void addFromJson_noType_buildsLogicalEntity() throws Exception {
		ResourceSpecification spec = specRepoService.addResourceSpecificationFromJson(json("""
		{
		"name": "PlainLogicalSpec",
		"description": "no @type at all",
		"version": "1.0.0",
		"category": "test.category"
		}"""));

		assertThat(spec).isInstanceOf(LogicalResourceSpecification.class);
		assertThat(spec.getName()).isEqualTo("PlainLogicalSpec");
		}

		/**
		* Over REST this @type already yielded a ResourceFunctionSpecification while over JMS it silently
		* became Logical. Same JSON, same meaning, either transport.
		*/
		@Test
		public void addFromJson_resourceFunctionType_buildsResourceFunctionEntity() throws Exception {
		ResourceSpecification spec = specRepoService.addResourceSpecificationFromJson(json("""
		{
		"name": "AResourceFunction",
		"version": "1.0.0",
		"category": "test.category",
		"@type": "ResourceFunctionSpecification"
		}"""));

		assertThat(spec).isInstanceOf(ResourceFunctionSpecification.class);
		}

		/** The upsert's create branch, then its update branch — the path the example controller uses. */
		@Test
		public void addOrUpdate_createsPhysicalThenUpdatesItInPlace() throws Exception {
		String first = """
		{
		"name": "CPE",
		"version": "0.1.0",
		"category": "test.category",
		"@type": "PhysicalResourceSpecification",
		"model": "HGW-1200",
		"vendor": "ACME"
		}""";

		ResourceSpecification created = specRepoService
		.addOrupdateResourceSpecificationByNameCategoryVersion("CPE", "test.category", "0.1.0", json(first));

		assertThat(created).isInstanceOf(PhysicalResourceSpecification.class);
		assertThat(((PhysicalResourceSpecification) created).getModel()).isEqualTo("HGW-1200");
		String uuid = created.getUuid();

		String second = first.replace("HGW-1200", "HGW-2400");
		ResourceSpecification updated = specRepoService
		.addOrupdateResourceSpecificationByNameCategoryVersion("CPE", "test.category", "0.1.0", json(second));

		assertThat(updated.getUuid()).isEqualTo(uuid);
		assertThat(((PhysicalResourceSpecification) updated).getModel()).isEqualTo("HGW-2400");
		}

		/**
		* A physical entity updated by a payload that cannot carry physical fields. This is the combination
		* that used to throw: the entity is physical, so the old code cast the payload unconditionally.
		* Nothing should blow up, and the stored fields must survive rather than be nulled.
		*/
		@Test
		public void addOrUpdate_physicalEntityWithLogicalPayload_doesNotThrow_andKeepsPhysicalFields() throws Exception {
		specRepoService.addOrupdateResourceSpecificationByNameCategoryVersion(
		"MobilePhone", "test.category", "0.1.0", json("""
		{
		"name": "MobilePhone",
		"version": "0.1.0",
		"category": "test.category",
		"@type": "PhysicalResourceSpecification",
		"model": "generic-5g-handset",
		"vendor": "ACME"
		}"""));

		ResourceSpecification updated = specRepoService.addOrupdateResourceSpecificationByNameCategoryVersion(
		"MobilePhone", "test.category", "0.1.0", json("""
		{
		"name": "MobilePhone",
		"description": "now with a description",
		"version": "0.1.0",
		"category": "test.category"
		}"""));

		assertThat(updated).isInstanceOf(PhysicalResourceSpecification.class);
		assertThat(updated.getDescription()).isEqualTo("now with a description");
		assertThat(((PhysicalResourceSpecification) updated).getModel()).isEqualTo("generic-5g-handset");
		assertThat(((PhysicalResourceSpecification) updated).getVendor()).isEqualTo("ACME");
		}

		/**
		* The PATCH path: {@code updateResourceSpecification} takes a bare ResourceSpecificationUpdate
		* straight from the REST controller, so every PATCH of a physical specification hit the same cast.
		* Nothing here mentions physical fields, so nothing physical may change.
		*/
		@Test
		public void updateWithBareUpdatePayload_onPhysicalSpec_doesNotThrow_andPreservesPhysicalFields() throws Exception {
		ResourceSpecification created = specRepoService.addResourceSpecificationFromJson(json("""
		{
		"name": "PatchMe",
		"version": "1.0.0",
		"category": "test.category",
		"@type": "PhysicalResourceSpecification",
		"model": "M-1",
		"vendor": "ACME"
		}"""));

		ResourceSpecificationUpdate patch = new ResourceSpecificationUpdate();
		patch.setDescription("patched description");

		ResourceSpecification patched = specRepoService.updateResourceSpecification(created.getUuid(), patch);

		assertThat(patched).isInstanceOf(PhysicalResourceSpecification.class);
		assertThat(patched.getDescription()).isEqualTo("patched description");
		assertThat(((PhysicalResourceSpecification) patched).getModel()).isEqualTo("M-1");
		assertThat(((PhysicalResourceSpecification) patched).getVendor()).isEqualTo("ACME");
		}

		/**
		* A correctly typed physical payload that simply omits a field must leave it alone. The old code
		* assigned all four unconditionally, so any partial update wiped the rest — a data-loss bug the
		* ClassCastException was masking.
		*/
		@Test
		public void updateWithPartialPhysicalPayload_onlyTouchesTheFieldsItCarries() throws Exception {
		ResourceSpecification created = specRepoService.addResourceSpecificationFromJson(json("""
		{
		"name": "PartialUpdate",
		"version": "1.0.0",
		"category": "test.category",
		"@type": "PhysicalResourceSpecification",
		"model": "M-1",
		"part": "P-1",
		"sku": "S-1",
		"vendor": "ACME"
		}"""));

		ResourceSpecification updated = specRepoService.updateResourceSpecificationFromJson(created.getUuid(), json("""
		{
		"@type": "PhysicalResourceSpecification",
		"vendor": "GLOBEX"
		}"""));

		PhysicalResourceSpecification physical = (PhysicalResourceSpecification) updated;
		assertThat(physical.getVendor()).isEqualTo("GLOBEX");
		assertThat(physical.getModel()).isEqualTo("M-1");
		assertThat(physical.getPart()).isEqualTo("P-1");
		assertThat(physical.getSku()).isEqualTo("S-1");
		}

		/**
		* An existing specification cannot be re-typed — JPA has no way to change an entity's concrete class
		* in place. Pinning the behaviour so the limitation is a decision, not a surprise.
		*/
		@Test
		public void addOrUpdate_cannotRetypeAnExistingLogicalSpecToPhysical() throws Exception {
		specRepoService.addOrupdateResourceSpecificationByNameCategoryVersion(
		"WasLogical", "test.category", "0.1.0", json("""
		{"name": "WasLogical", "version": "0.1.0", "category": "test.category"}"""));

		ResourceSpecification updated = specRepoService.addOrupdateResourceSpecificationByNameCategoryVersion(
		"WasLogical", "test.category", "0.1.0", json("""
		{
		"name": "WasLogical",
		"version": "0.1.0",
		"category": "test.category",
		"@type": "PhysicalResourceSpecification",
		"model": "M-1"
		}"""));

		assertThat(updated).isInstanceOf(LogicalResourceSpecification.class);
		assertThat(updated).isNotInstanceOf(PhysicalResourceSpecification.class);
		}
		}