Uploaded image for project: 'Spring Data REST'
  1. Spring Data REST
  2. DATAREST-1304

PUT and PATCH don't work, when custom entity lookup is configured.

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.11 (Kay SR11)
    • Fix Version/s: 3.2 M2 (Moore)
    • Component/s: Repositories
    • Labels:
      None

      Description

      Starting with version 3.0.0 save does not work correctly anymore , if a custom entity lookup strategy is configured. 

      If we have an EntityLookupConfiguration like

      @Configuration
      public class RepositoryEntityLookupConfiguration extends RepositoryRestConfigurerAdapter {
      
          @Override
          public void configureRepositoryRestConfiguration(RepositoryRestConfiguration configuration) {
              configuration
                      .withEntityLookup()
                      .forRepository(SomeRepository.class, Some::getUid, SomeRepository::findByUid);
          }
      }
      

      assuming some Document like

      @Document
      public class Some {
      
          @Id
          String id;
          @NotBlank(message = "uid may not be null, empty or blank")
          @Indexed(unique = true)
          String uid;
      
          Programs programs;
          Bugs bugs;
      }
      

      then any update operation fails with 409.

      Actually, when the frameworks tries to save (updates the persistent data), the id of the persistence layer (id in our case) is substituted by the lookup key (uid), and then the save operation inserts a new document with a uid forbidden by the unique id.

      The "bug" is in PersistentEntityResourceHandlerMethodArgumentResolver#resolveArgument (lines 139-150), where the persistence-id-property is set.

       

           // here it is checked whether we have an update and the persistence-id-property is set to entity-id
                  PersistentEntity<?, ?> entity = resourceInformation.getPersistentEntity();
                  boolean forUpdate = objectToUpdate.isPresent();
                  Optional<Object> entityIdentifier = objectToUpdate.map(it -> entity.getIdentifierAccessor(it).getIdentifier());
      
                  entityIdentifier.ifPresent(it -> entity.getPropertyAccessor(obj).setProperty(entity.getRequiredIdProperty(),
                          entityIdentifier.orElse(null)));
      
                  // unfortunately, here the correct value is overwritten (possibly with a wrong value, as in case custom lookup configuration)
                  id.ifPresent(it -> {
                      ConvertingPropertyAccessor accessor = new ConvertingPropertyAccessor(entity.getPropertyAccessor(obj),
                              conversionService);
                      accessor.setProperty(entity.getRequiredIdProperty(), it);
                  });
      

      I'm not quite sure under with conditions the requiredIdProperty should be set;
      perhaps only if (!forUpdate) or (!forUpdate && !noCustomLookupConfig) or something similar along the lines.

      The result then may be something like:

      if(!forUpdate) {
                  id.ifPresent(it -> {
                      ConvertingPropertyAccessor accessor = new ConvertingPropertyAccessor(entity.getPropertyAccessor(obj),
                              conversionService);
                      accessor.setProperty(entity.getRequiredIdProperty(), it);
                  });
              }
      

      This is actually equivalent to what was implemented in 2.x.x branches. 

      Note this does still not give a valid solution of the problem in the sense of the requirements in DATA-REST-1050 in case of EntityLookupConfiguration. This is because in case of creating a new resource, a lookup by the same custom key does not find the created resource in general.

      But at least, it allows us to upgrade existing applications to Spring Boot 2.0, since we are not relying on an idempotent PUT anyway. With the current version, we see no way to upgrade to Spring Boot 2 at all.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              olivergierke Oliver Drotbohm
              Reporter:
              Wagener Hubert Wagener
              Last updater:
              Mark Paluch
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: