hmis-tools / hmis-api-server

Version 2 of OpenHMIS
Mozilla Public License 2.0
15 stars 11 forks source link

Finish and deploy the mirrorsync-compat branch. #49

Open kfogel opened 8 years ago

kfogel commented 8 years ago

Background: Because the addition of exceptions caused returned objects to be wrapped in an extra layer that MirrorSync isn't expecting, at least for now we're going to deploy a shim server (on some port other than 8080) that is based on pre-exception code, so that it's still compatible with MirrorSync.

The mirrorsync-compat branch is where this is happening. It is based from commit 4dc9c0cb8c (that is, the commit on master right before the feature-search branch was merged into master). We don't want the feature-exceptions code in the branch, and we don't need feature-search either (which is good, because feature-search is tangled up with the exceptions stuff), since the search functionality is for TechBridge's Salesforce application and the mirrorsync-compat branch is just for MirrorSync.

However, we do want all the recent configuration improvements from the feature-configuration-refactor branch to go into mirrorsync-compat, because we're going to have to manage two servers now and it would be nice to use the same configuration file format and placement in both of them. (But we can't merge 'feature-configuration-refactor' into 'mirrorsync-compat', because 'feature-configuration-refactor' was branched from master after a bunch of other changes we don't want -- changes that were deliberately left off the 'mirrorsync-compat' branch. So let's take the careful route, and cherry-pick the relevant commits one at a time in order.)

The commits we want are:

So, starting from the bottom (earliest) commit, I did this on the mirrorsync-compat branch:

$ git cherry-pick 3bf8d411be

Complete success. No problems. Pushed up to origin (GitHub) now.

Next I did:

$ git cherry-pick be5d7f05c
error: could not apply be5d7f0... Hibernate now loads connection settings from a separate configuration
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
$ git status
On branch mirrorsync-compat
You are currently cherry-picking commit be5d7f0.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:

    modified:   .gitignore
    modified:   pom.xml
    modified:   src/main/java/org/openhmis/util/ApplicationPropertyUtil.java
    modified:   src/main/java/org/openhmis/util/Authentication.java
    modified:   src/main/java/org/openhmis/util/HibernateSessionFactory.java
    deleted:    src/main/resources/application.properties.example
    new file:   src/main/resources/dev.properties.example
    renamed:    src/main/resources/hibernate.cfg.xml.example -> src/main/resources/hibernate.cfg.xml

Unmerged paths:
  (use "git add <file>..." to mark resolution)

    both modified:   src/main/java/org/openhmis/webservice/ClientService.java

$ 

The conflict in src/main/java/org/openhmis/webservice/ClientService.java (see below) is interesting. It looks like it might just a spurious indendation change -- in other words, I could choose either side of the conflict and things would be fine, code-wise.

But when I examine the actual commit be5d7f05c diff in ClientService.java, I get more worried. It looks like some other changes got successfully merged in the cherry-pick, without conflict, that depend on the new object-wrapping world order -- i.e., defeating the whole point of the 'mirrosync-compat' branch. I'm talking about this line specifically, the one that now starts with public List<ClientDTO>. And is the same thing going on in some of the other Java files that were changed as part of commit be5d7f05c?

When I mentioned this to Dan in IRC, he was busy with some other OpenHMIS stuff at the time and we agreed to put off the quetsion until later, but he took a quick look and said:

gut feeling is that nothing there looks like a problem

the code that causes the wrapping is the introduction of the
ObjectMapperContextResolver.java in the org.openhmis.provider

the use of generics isn't an issue

So maybe there's no problem. In the meantime, I'm saving everything in this ticket, so I can remember where I was, and so we can take a closer look later. Here's that conflict (but you can get this just by doing git cherry-pick be5d7f05c in the mirrorsync-compat branch right now):

@Path("/clients")
public class ClientService {
    private static final Logger log = Logger.getLogger(ClientService.class);
    private static final ClientManager clientManager = new ClientManager();

    public ClientService() {}

    @GET
    @Path("/")
    @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
    public List<ClientDTO> getClients(@HeaderParam("Authorization") String authorization, @BeanParam ClientSearchDTO searchDTO) throws JsonProcessingException {
        if(!Authentication.googleAuthenticate(authorization))
            throw new Error("You are not authorized to access this content");

<<<<<<< HEAD
        // If the user specified no updatedSince parameter, return everything
        List<ClientDTO> clientDTOs = clientManager.getClients(searchDTO);
        return clientDTOs;
=======
        // Return clients that match the search terms
        List<ClientDTO> clientDTOs = clientManager.getClients(searchDTO);
        return clientDTOs;
>>>>>>> be5d7f0... Hibernate now loads connection settings from a separate configuration
    }

    @POST
    @Path("/")
    @Consumes({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
    @Produces({MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML})
    public ClientDTO createClient(@HeaderParam("Authorization") String authorization, ClientDTO inputVO) throws JsonParseException, JsonMappingException, IOException {
        if(!Authentication.googleAuthenticate(authorization))
            throw new Error("You are not authorized to access this content");
        ClientDTO outputVO = clientManager.addClient(inputVO);
        return outputVO;
    }